Skip to content

Split generated runtime fallback#17

Open
leynos wants to merge 7 commits into
mainfrom
sync-prosidy-darn-init-runtime-template
Open

Split generated runtime fallback#17
leynos wants to merge 7 commits into
mainfrom
sync-prosidy-darn-init-runtime-template

Conversation

@leynos

@leynos leynos commented Jun 14, 2026

Copy link
Copy Markdown
Owner

Summary

This branch splits generated package backend selection out of the public package initializer so rendered Python/Rust projects keep a small __init__ surface while retaining the optional Rust-extension fallback. It also adds generated public API and runtime fallback regression coverage, and tightens the branch's documentation changes after rebase.

No linked issue, roadmap task, or execplan was found for this branch.

Review walkthrough

Validation

  • mdtablefix --wrap --renumber --breaks --ellipsis --fences --in-place template/docs/complexity-antipatterns-and-refactoring-strategies.md: passed
  • markdownlint-cli2 --fix template/docs/complexity-antipatterns-and-refactoring-strategies.md: passed with 0 errors
  • markdownlint-cli2 template/docs/complexity-antipatterns-and-refactoring-strategies.md: passed with 0 errors
  • ruff format tests template/tests: passed; 1 file reformatted, then make check-fmt verified the result
  • uvx --with pytest-copier --with pyyaml --with syrupy --with make-parser pytest tests/test_template.py::test_runtime_module_documents_and_limits_fallback tests/test_template.py::test_python_only_template tests/test_template.py::test_rust_template -q: passed; 3 passed
  • git diff --check: passed
  • make check-fmt: passed; 18 files already formatted
  • make test: passed; 33 passed, 2 skipped, 3 snapshots passed
  • make typecheck: passed
  • make lint: passed

Notes

mdformat-all is treated as the canonical Markdown formatter, but the installed wrapper does not expose a scope flag and always walks every Markdown file. Running it across the whole repository would reformat the large copied Copier reference documents under docs/configuring-a-copier-template.md and docs/creating-a-copier-template.md, which already have unrelated Markdown lint failures. This branch therefore applies the same canonical mdtablefix plus markdownlint-cli2 --fix pipeline only to the Markdown files touched by the branch.

The installed ruff format command does not support Markdown as Markdown in this checkout. Forcing .md through --extension md:python makes Ruff parse the entire document as Python, so this branch only runs Ruff formatting on the Python test/template-support targets.

leynos added 3 commits June 14, 2026 04:29
Move generated package initialisation to a re-export-only `__init__` and place
the optional Rust-extension fallback in a private runtime module. Add the pure
module docstring and generated public API test so new projects satisfy the
current lint and test gates.

Refresh Ruff-formatted rule and scripting guidance, and update the generated
packaging settings so Python-only and Rust-backed projects install under custom
package names on current Python versions.
Add the root `markdownlint-cli2` config so parent-level Markdown checks use
the same rules as rendered projects.

Remove the incidental `.rules` formatting churn from the branch and repair
the duplicated fenced block in `scripting-standards.md`. Keep the scoped
Markdown formatter output for the docs that remain changed by this branch.

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry @leynos, you have reached your weekly rate limit of 500000 diff characters.

Please try again later or upgrade to continue using Sourcery

@coderabbitai

coderabbitai Bot commented Jun 14, 2026

Copy link
Copy Markdown

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

Walkthrough

A new _runtime.py.jinja template module is created to centralise conditional Rust/pure-Python hello dispatch; __init__.py.jinja is simplified to a single import from it. A generated test template and a comprehensive runtime fallback regression test are introduced. Separately, a .markdownlint-cli2.jsonc configuration is added and template documentation files are reflowed to comply with it, with one citation reference corrected.

Changes

Runtime dispatch refactor

Layer / File(s) Summary
_runtime module and __init__ simplification
template/{{ package_name }}/_runtime.py.jinja, template/{{ package_name }}/__init__.py.jinja
_runtime.py.jinja is created with PACKAGE_NAME and derived Rust module-name constants; conditional Rust import logic guarded by ModuleNotFoundError matching; __init__.py.jinja is reduced to from ._runtime import hello, removing all prior importlib/typing scaffolding and fallback logic.
Public-API test template and runtime fallback regression testing
template/tests/test_public_api.py.jinja, tests/test_template.py
test_public_api.py.jinja is introduced with test_hello_uses_configured_backend, asserting hello() returns the correct greeting per use_rust; tests/test_template.py gains os/sys imports and adds test_runtime_module_documents_and_limits_fallback, a comprehensive regression test verifying the _runtime module's fallback behaviour and error preservation; test_python_only_template is extended with a filesystem assertion for docs/documentation-style-guide.md.

Markdownlint config and doc reformats

Layer / File(s) Summary
Markdownlint configuration
.markdownlint-cli2.jsonc
Introduces markdownlint-cli2 configuration with MD004/MD010/MD013/MD029 rule tuning and an ignores list for build/cache directories and CRUSH.md.
Doc line-wrap reformats and citation fix
template/docs/complexity-antipatterns-and-refactoring-strategies.md, template/docs/scripting-standards.md
Multiple sections in complexity-antipatterns-and-refactoring-strategies.md (Cyclomatic Complexity, Bumpy Road, Separation of Concerns, CQRS discussion) are reflowed to comply with line-length rules; footnote [^26] is corrected to [^34] in the declarative SQL example, with a new [^34] citation entry added. scripting-standards.md has the Cyclopts CLI pattern signature, Catalogue concurrency safety guidance (with new "Concurrent testing patterns with cmd-mox" subsection), and atomic write NamedTemporaryFile example expanded into multi-line forms.

Possibly related PRs

  • leynos/agent-template-python#12: Overlapping changes to template/docs/scripting-standards.md around cmd-mox testing and concurrent execution guidance.
  • leynos/episodic#132: Introduces markdownlint-cli2 configuration and matching doc reformatting, directly paralleling this PR's new .markdownlint-cli2.jsonc and doc reflowing.
  • leynos/prosidy-darn#13: Overlapping markdownlint configuration and reformat of template/docs/complexity-antipatterns-and-refactoring-strategies.md.

Poem

A _runtime module steps into the light,
__init__ grows lean — just one import, just right.
Footnote [^26] bows out for [^34],
And lint rules now govern each markdown shore.
The catalogue stands safe, the docs neatly spaced —
Clean code and clean prose, nothing left misplaced. 📦


Caution

Pre-merge checks failed

Please resolve all errors before merging. Addressing warnings is optional.

  • Ignore

❌ Failed checks (3 errors, 5 warnings)

Check name Status Explanation Resolution
Testing (Overall) ❌ Error Generated test is vacuous (tautologically passes). String assertions check implementation details rather than behaviour. Five assertions lack required messages. Review comments about docstring and... Rewrite test_hello_uses_configured_backend to have fixed expectations (not templated). Replace five implementation-detail string assertions with behavioural tests. Add messages to all assertions per coding guidelines.
Module-Level Documentation ❌ Error Module template/{{ package_name }}/_runtime.py.jinja has a docstring after the __future__ import, rendering it unrecognised as the module's __doc__; move the docstring before all imports. Move the docstring in _runtime.py.jinja to line 1, before from __future__ import annotations, so it becomes the proper module-level documentation.
Unit Architecture ❌ Error PR demonstrates sound unit architecture (clear query/command separation, explicit fallibility, no mutable globals, dependency injection), but fails to address three review comments: footnote number... Address all three review comments: renumber footnote to [^33] for sequential order, change test docstring verb, and attach descriptive messages to each assertion per coding guidelines.
Developer Documentation ⚠️ Warning The PR refactors generated package structure by moving backend selection to a private _runtime module, but does not document this architectural change in docs/developers-guide.md. Document the new _runtime.py module and backend-selection architecture in the "Rust Integration" section of docs/developers-guide.md, explaining how the public package maintains a stable __init__.py whilst optional Rust fallback ha...
Testing (Unit And Behavioural) ⚠️ Warning Three review comments remain unaddressed: (1) test_public_api.py.jinja docstring still uses "Return" instead of "Verify"; (2) Five string-matching assertions in test_runtime_module_documents_and_li... Address all three review comments: update test docstring to use action verb, add descriptive messages to each of the five bare assertions, and verify footnote numbering is sequential.
Testing (Compile-Time / Ui) ⚠️ Warning Generated structured output (_runtime.py) lacks snapshot test coverage. Pattern assertions check only specific code fragments rather than encoding meaningful comprehensive behavioural expectations... Add snapshot test for generated _runtime.py module (parameterised for use_rust variants) to ensure generated template changes don't silently alter module structure or re-raise/fallback logic implementation details.
Domain Architecture ⚠️ Warning Domain architecture is clean and properly segregated (domain in pure.py, infrastructure in _runtime.py, API in init.py), but three review comments remain unaddressed: footnote numbering [^34]→[... Address the three review comments: renumber footnote to [^33], update docstring verb to "Verify", add assertion messages to all five runtime module checks.
Architectural Complexity And Maintainability ⚠️ Warning The _runtime.py module introduces justified architectural separation that reduces public API complexity. However, three review comments remain unaddressed: footnote numbering should use [^33] n... Address three outstanding review comments: renumber footnotes to [^33], update test docstring verb, add assertion messages explaining each _runtime validation point.
✅ Passed checks (12 passed)
Check name Status Explanation
Title check ✅ Passed The title 'Split generated runtime fallback' directly describes the main refactoring: moving backend selection logic from init.py to _runtime.py.
Description check ✅ Passed The description comprehensively covers the changeset, detailing the refactoring rationale, file-by-file review guidance, validation steps, and technical notes.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
User-Facing Documentation ✅ Passed PR performs internal template refactoring (moving backend selection from init.py to _runtime.py) without user-facing API changes; generated projects still export identical hello() function with...
Testing (Property / Proof) ✅ Passed PR introduces invariants (backend selection, exception filtering, module name derivation) but tests them with fixed inputs only; property-based testing is not necessary given the deterministic natu...
Observability ✅ Passed This PR modifies template scaffolding, documentation, and test code—no operational behaviour is introduced in the template repository itself. The check is inapplicable to template configuration and...
Security And Privacy ✅ Passed PR contains only template configurations, documentation updates, and test coverage—no secrets, authentication bypasses, injection risks, or data exposure introduced.
Performance And Resource Use ✅ Passed PR contains no avoidable regressions, unbounded resource growth, or unnecessary I/O. Module initialisation uses importlib once per load (cold path), memory allocations are bounded, and test subproc...
Concurrency And State ✅ Passed PR does not introduce concurrency, shared state, async execution, locks, or related concerns—it merely reorganises synchronous module-level import logic.
Rust Compiler Lint Integrity ✅ Passed PR preserves Rust compiler lint integrity. No #[allow(...)] suppressions added; existing #[expect(...)] in lib.rs.jinja is justified for PyO3 framework constraint. New clippy.toml configuration app...
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch sync-prosidy-darn-init-runtime-template

Comment @coderabbitai help to get the list of available commands and usage tips.

@leynos leynos marked this pull request as ready for review June 14, 2026 04:48

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@template/`{{ package_name }}/_runtime.py.jinja:
- Line 1: The _runtime.py.jinja template file is missing a module-level
docstring. Add a triple-quoted docstring immediately after the `from __future__
import annotations` import statement that documents the purpose of this runtime
module, explains its utility, describes how it should be used, and includes
example usage calls where appropriate, following the coding guideline that every
module must begin with such documentation.
- Around line 11-15: The exception handler in the try-except block is catching
all ModuleNotFoundError exceptions, which can mask real packaging and runtime
faults. Instead of catching all ModuleNotFoundError instances, capture the
exception as a variable and check if it's specifically for the missing Rust
module (_{PACKAGE_NAME}_rs). Construct the expected module name as a string
combining the package name, then compare it against the caught exception's name
attribute. Only fall back to the pure Python implementation when the exception
is specifically about the missing Rust module; if the exception's name does not
match the expected Rust module name, re-raise the exception so that other
ModuleNotFoundError cases are not silently masked.

In `@template/docs/complexity-antipatterns-and-refactoring-strategies.md`:
- Around line 655-657: The footnote reference [^34] in the declarative
programming examples section is not defined in the works-cited section, creating
a dangling citation. Either replace the [^34] reference with [^26] if that
existing footnote contains the appropriate source material, or add a new [^34]
footnote entry to the works-cited section with a matching citation. Ensure the
footnote number you use in the text has a corresponding definition elsewhere in
the document.

In `@template/tests/test_public_api.py.jinja`:
- Line 10: The assert statement in the test_public_api.py.jinja file is missing
an assertion message on the equality check of {{ package_name }}.hello(). Add a
descriptive message as the second argument to the assert statement following the
format assert condition, "message" to provide clear context when the test fails,
helping developers understand what the assertion was checking.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 8d8fc41c-12c4-419a-8ff6-b76ea5bab49d

📥 Commits

Reviewing files that changed from the base of the PR and between 0f62dca and 0259872.

📒 Files selected for processing (7)
  • .markdownlint-cli2.jsonc
  • template/docs/complexity-antipatterns-and-refactoring-strategies.md
  • template/docs/scripting-standards.md
  • template/tests/test_public_api.py.jinja
  • template/{{ package_name }}/__init__.py.jinja
  • template/{{ package_name }}/_runtime.py.jinja
  • tests/test_template.py

Comment thread template/{{ package_name }}/_runtime.py.jinja
Comment thread template/{{ package_name }}/_runtime.py.jinja
Comment thread template/tests/test_public_api.py.jinja Outdated
Document the generated runtime selector and narrow the optional Rust fallback
so only the missing extension module falls back to the pure Python backend.
Other missing-module failures now propagate.

Add generated-template coverage for the fallback and re-raise paths, define
the dangling declarative-programming citation, and add context to the public
API assertion.
@pandalump

Copy link
Copy Markdown
Collaborator

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jun 14, 2026

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

coderabbitai[bot]

This comment was marked as resolved.

Expand the generated package entry-point documentation so users can see the
public import path and a concrete `hello()` example.

Keep generated tests easier to diagnose by describing the public API check and
adding assertion messages to the runtime source-shape checks. Renumber the SQL
citation to keep the Markdown references sequential.
@leynos

leynos commented Jun 14, 2026

Copy link
Copy Markdown
Owner Author

@coderabbitai Have the following now been resolved?

template/{{ package_name }}/init.py.jinja (1)

1-1: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
Expand the public package docstring with usage guidance.
The module docstring contains only the project name. For a public package entry point, include purpose, basic usage, and an example.

📚 Proposed enhancement
-"""{{ project_name }} package."""
+"""{{ project_name }} package.
+
+This package provides a greeting function that demonstrates backend selection
+between {% if use_rust %}Rust and {% endif %}Python implementations.
+
+Examples
+--------
+>>> import {{ package_name }}
+>>> {{ package_name }}.hello()
+"{% if use_rust %}hello from Rust{% else %}hello from Python{% endif %}"
+"""

As per coding guidelines, every module must explain its purpose, utility, and usage, including example calls if appropriate.

🤖 Detailed instructions

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @template/{{ package_name }}/init.py.jinja at line 1, Expand the
docstring at the beginning of the init.py.jinja file to go beyond the basic
project name declaration. The docstring should now include a description of the
package's purpose, explain what main functionality it provides, include basic
usage guidance on how to import and use key components, and provide a concrete
example demonstrating typical usage patterns. This will serve as comprehensive
documentation for users of the public package entry point.

Source: Coding guidelines

@coderabbitai

coderabbitai Bot commented Jun 14, 2026

Copy link
Copy Markdown

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check the current content of the __init__.py.jinja file
fd --full-path '__init__.py.jinja' --exec cat {}

Length of output: 723



@leynos Yes, this has been fully resolved. The docstring in template/{{ package_name }}/__init__.py.jinja now includes:

  • A purpose statement explaining it is the stable public entry point for generated projects
  • Usage guidance noting that application code should import from this module
  • An Examples section with a concrete doctest-style demonstration, including the conditional {% if use_rust %} rendering

The current content goes beyond the minimal proposed enhancement and satisfies the coding guideline requirement.

@leynos

leynos commented Jun 14, 2026

Copy link
Copy Markdown
Owner Author

@coderabbitai Please investigate the cause of the following issue using codegraph exploration and research, identify a fix and provide an AI coding agent prompt for the fix:

Run make test WITH_ACT=1
  make test WITH_ACT=1
  shell: /usr/bin/bash -e {0}
  env:
    ACT_VERSION: v0.2.84
    MARKDOWNLINT_CLI2_VERSION: 0.22.1
    MBAKE_VERSION: 1.4.6
    pythonLocation: /opt/hostedtoolcache/Python/3.13.13/x64
    PKG_CONFIG_PATH: /opt/hostedtoolcache/Python/3.13.13/x64/lib/pkgconfig
    Python_ROOT_DIR: /opt/hostedtoolcache/Python/3.13.13/x64
    Python2_ROOT_DIR: /opt/hostedtoolcache/Python/3.13.13/x64
    Python3_ROOT_DIR: /opt/hostedtoolcache/Python/3.13.13/x64
    LD_LIBRARY_PATH: /opt/hostedtoolcache/Python/3.13.13/x64/lib
    UV_CACHE_DIR: /home/runner/work/_temp/setup-uv-cache
    CARGO_INCREMENTAL: 0
    CARGO_PROFILE_DEV_DEBUG: 0
    CARGO_TERM_COLOR: always
    RUST_BACKTRACE: short
    RUSTFLAGS: -D warnings
    CARGO_UNSTABLE_SPARSE_REGISTRY: true
    CARGO_REGISTRIES_CRATES_IO_PROTOCOL: sparse
    UV_PYTHON_INSTALL_DIR: /home/runner/work/_temp/uv-python-dir
    SCCACHE_PATH: /opt/hostedtoolcache/sccache/0.15.0/x64/sccache
    ACTIONS_CACHE_SERVICE_V2: on
    ACTIONS_RESULTS_URL: https://results-receiver.actions.githubusercontent.com/
    ACTIONS_RUNTIME_TOKEN: ***
    ACT_GITHUB_TOKEN: ***
RUN_ACT_VALIDATION=1 /opt/hostedtoolcache/uv/0.11.21/x86_64/uvx --with pytest-copier --with pyyaml --with syrupy --with make-parser pytest tests/
Downloading pydantic-core (2.0MiB)
 Downloaded pydantic-core
Installed 30 packages in 20ms
============================= test session starts ==============================
platform linux -- Python 3.13.13, pytest-9.1.0, pluggy-1.6.0
rootdir: /home/runner/work/agent-template-python/agent-template-python
configfile: pytest.ini
plugins: dir-equal-0.2.1, copier-0.4.1, gitconfig-0.9.0, syrupy-5.3.2
collected 35 items

tests/test_audit.py ..                                                   [  5%]
tests/test_github_actions_integration.py ..xF                            [ 17%]
tests/test_helpers.py ................                                   [ 62%]
tests/test_parent_ci.py .                                                [ 65%]
tests/test_template.py ............                                      [100%]

=================================== FAILURES ===================================
_ test_generated_workflow_runs_with_shared_coverage_action[ActRust-act_rust-True-rust-artifacts] _

act_ready = None
copier = CopierFixture(template=PosixPath('/tmp/pytest-of-runner/pytest-0/src'), defaults={}, monkeypatch=<_pytest.monkeypatch.MonkeyPatch object at 0x7f35aa486350>)
tmp_path = PosixPath('/tmp/pytest-of-runner/pytest-0/test_generated_workflow_runs_w1')
name = 'ActRust', package = 'act_rust', use_rust = True
artifact_dir = 'rust-artifacts'

    @pytest.mark.parametrize(
        ("name", "package", "use_rust", "artifact_dir"),
        [
            ("ActPure", "act_pure", False, "pure-artifacts"),
            ("ActRust", "act_rust", True, "rust-artifacts"),
        ],
    )
    @pytest.mark.act
    def test_generated_workflow_runs_with_shared_coverage_action(
        act_ready: None,
        copier: CopierFixture,
        tmp_path: Path,
        name: str,
        package: str,
        use_rust: bool,
        artifact_dir: str,
    ) -> None:
        """Validate a generated CI workflow through act.
    
        Parameters
        ----------
        act_ready
            Fixture that skips the test unless act validation is explicitly enabled
            and a compatible runtime is reachable.
        copier
            ``pytest-copier`` fixture used to render the template.
        tmp_path
            Temporary directory for the rendered project and act artifacts.
        name
            Project name passed to the Copier template.
        package
            Package name passed to the Copier template.
        use_rust
            Whether to render the Rust-extension variant.
        artifact_dir
            Directory name, under ``tmp_path``, used for act artifacts.
    
        Returns
        -------
        None
            The test passes after the workflow runs and the expected generated
            workflow contract is observed.
    
        Raises
        ------
        AssertionError
            Raised when generated workflow content or act observations are missing.
    
        Examples
        --------
        Run this parametrized test with local act validation enabled::
    
            RUN_ACT_VALIDATION=1 python -m pytest -m act -v
        """
        project = copier.copy(
            tmp_path / package,
            project_name=name,
            package_name=package,
            use_rust=use_rust,
        )
        prepare_git_repository(project)
    
        code, logs = run_act(project, artifact_dir=tmp_path / artifact_dir)
    
        workflow = (project / ".github" / "workflows" / "ci.yml").read_text(
            encoding="utf-8"
        )
        assert "leynos/shared-actions/.github/actions/generate-coverage" in workflow, (
            "Generated workflow should use the shared generate-coverage action"
        )
        if use_rust:
            assert "cargo-manifest: rust_extension/Cargo.toml" in workflow, (
                "Rust workflow should pass the Rust extension manifest to coverage"
            )
>       assert_act_result(project, code, logs, use_rust=use_rust)

tests/test_github_actions_integration.py:460: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
tests/test_github_actions_integration.py:371: in assert_act_result
    assert_ci_exercised_expected_steps(logs, use_rust=use_rust)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

logs = '{"dryrun":false,"job":"CI/lint-test","jobID":"lint-test","level":"info","matrix":{},"msg":"⭐ Run Set up job","step":"...nfo","msg":"Start server on http://10.1.0.226:34567","time":"2026-06-14T14:41:40Z"}\nError: Job \'lint-test\' failed\n'
use_rust = True

    def assert_ci_exercised_expected_steps(logs: str, *, use_rust: bool) -> None:
        """Assert that act logs include the expected test and coverage steps.
    
        Parameters
        ----------
        logs
            Combined act output from ``run_act``.
        use_rust
            Whether the rendered project should include Rust-extension workflow
            steps.
    
        Returns
        -------
        None
            The helper returns after all expected workflow observations are present.
    
        Raises
        ------
        AssertionError
            Raised when coverage, Python tests, or required Rust checks are missing
            from the act logs.
    
        Examples
        --------
        Verify the observed steps after an act run::
    
            assert_ci_exercised_expected_steps(logs, use_rust=True)
        """
        saw_coverage = False
        saw_python = False
        saw_rust = not use_rust
        for event in iter_json_log_events(logs):
            output = str(event_text(event, "Output", "output", "message", "msg"))
            step = str(event_text(event, "name", "step_name", "Step", "step"))
            in_coverage_step = GENERATE_COVERAGE_STEP in step
            saw_coverage = saw_coverage or (
                in_coverage_step
                and ("coverage.xml" in output or "Current coverage" in output)
            )
            saw_python = saw_python or (
                in_coverage_step and ("run_python.py" in output or "pytest -v" in output)
            )
            saw_rust = saw_rust or (
                in_coverage_step
                and (
                    "run_rust.py" in output
                    or "cargo nextest" in output
                    or "cargo llvm-cov" in output
                )
            )
    
>       assert saw_coverage, f"coverage action step was not observed:\n{logs}"

https://github.com/leynos/agent-template-python/actions/runs/27502113599/job/81286742745?pr=17

@coderabbitai

This comment was marked as resolved.

leynos added 2 commits June 14, 2026 18:01
Move the generated `_runtime.py` documentation string before the future import
so Python exposes it through `module.__doc__`.

Extend the rendered-template regression check to require the generated file to
start with the documentation string and to prove the imported runtime module
keeps a non-empty `__doc__` value.
Bump the Rust-extension template to PyO3 0.29.0 so generated
Rust-enabled projects no longer fail `cargo audit` before the shared
coverage action can run.
@leynos

leynos commented Jun 15, 2026

Copy link
Copy Markdown
Owner Author

@coderabbitai Have the following failed checks now been resolved?

If further work is required, please provide an AI agent prompt for the remaining work to be done to address these failures.

Do not treat warnings as optional or aspirational. Where a change is out of scope for this PR, propose a GitHub issue unless one exists already. (Treat documentation and validation coverage as in scope).

❌ Failed checks (3 errors, 5 warnings)

Check name Status Explanation Resolution
Testing (Overall) ❌ Error Generated test is vacuous (tautologically passes). String assertions check implementation details rather than behaviour. Five assertions lack required messages. Review comments about docstring and... Rewrite test_hello_uses_configured_backend to have fixed expectations (not templated). Replace five implementation-detail string assertions with behavioural tests. Add messages to all assertions per coding guidelines.
Module-Level Documentation ❌ Error Module template/{{ package_name }}/_runtime.py.jinja has a docstring after the __future__ import, rendering it unrecognised as the module's __doc__; move the docstring before all imports. Move the docstring in _runtime.py.jinja to line 1, before from __future__ import annotations, so it becomes the proper module-level documentation.
Unit Architecture ❌ Error PR demonstrates sound unit architecture (clear query/command separation, explicit fallibility, no mutable globals, dependency injection), but fails to address three review comments: footnote number... Address all three review comments: renumber footnote to [^33] for sequential order, change test docstring verb, and attach descriptive messages to each assertion per coding guidelines.
Developer Documentation ⚠️ Warning The PR refactors generated package structure by moving backend selection to a private _runtime module, but does not document this architectural change in docs/developers-guide.md. Document the new _runtime.py module and backend-selection architecture in the "Rust Integration" section of docs/developers-guide.md, explaining how the public package maintains a stable __init__.py whilst optional Rust fallback ha...
Testing (Unit And Behavioural) ⚠️ Warning Three review comments remain unaddressed: (1) test_public_api.py.jinja docstring still uses "Return" instead of "Verify"; (2) Five string-matching assertions in test_runtime_module_documents_and_li... Address all three review comments: update test docstring to use action verb, add descriptive messages to each of the five bare assertions, and verify footnote numbering is sequential.
Testing (Compile-Time / Ui) ⚠️ Warning Generated structured output (_runtime.py) lacks snapshot test coverage. Pattern assertions check only specific code fragments rather than encoding meaningful comprehensive behavioural expectations... Add snapshot test for generated _runtime.py module (parameterised for use_rust variants) to ensure generated template changes don't silently alter module structure or re-raise/fallback logic implementation details.
Domain Architecture ⚠️ Warning Domain architecture is clean and properly segregated (domain in pure.py, infrastructure in _runtime.py, API in init.py), but three review comments remain unaddressed: footnote numbering [^34]→[... Address the three review comments: renumber footnote to [^33], update docstring verb to "Verify", add assertion messages to all five runtime module checks.
Architectural Complexity And Maintainability ⚠️ Warning The _runtime.py module introduces justified architectural separation that reduces public API complexity. However, three review comments remain unaddressed: footnote numbering should use [^33] n... Address three outstanding review comments: renumber footnotes to [^33], update test docstring verb, add assertion messages explaining each _runtime validation point.

@coderabbitai

coderabbitai Bot commented Jun 15, 2026

Copy link
Copy Markdown

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check current state of _runtime.py.jinja
fd '_runtime.py.jinja' --exec cat {}
echo "---"
# Check test_public_api.py.jinja
fd 'test_public_api.py.jinja' --exec cat {}
echo "---"
# Check test_template.py (runtime fallback test and assertions)
grep -n 'assert\|def test_runtime\|def test_python_only\|noqa' tests/test_template.py
echo "---"
# Check complexity-antipatterns doc for footnote numbering
grep -n '\[\^3[0-9]\]' "template/docs/complexity-antipatterns-and-refactoring-strategies.md" | tail -20
echo "---"
# Check docs/developers-guide.md exists and its Rust section
fd 'developers-guide.md' --exec grep -n 'rust\|_runtime\|backend' -i {}
echo "---"
# Check for snapshot tests
fd 'snapshots' -t d
fd '__snapshots__' -t d
rg 'snapshot\|syrupy\|assert_match_snapshot' tests/test_template.py

Length of output: 8178


🏁 Script executed:

#!/bin/bash
# Get the full test_runtime_module_documents_and_limits_fallback function
sed -n '162,245p' tests/test_template.py
echo "==="
# Read the Rust Integration section of developers-guide.md
fd 'developers-guide.md' --exec cat {}
echo "==="
# List snapshot directory contents
ls tests/__snapshots__/
echo "==="
# Check test_template.py for any _runtime snapshot test
grep -n 'runtime\|_runtime\|snapshot' tests/test_template.py

Length of output: 15902



@leynos: Five of the eight checks are now resolved. Here is the full status assessment followed by AI agent prompts for the three remaining items.


Status

Check Status Detail
Module-Level Documentation ✅ Resolved Docstring is now the first statement in _runtime.py.jinja
Unit Architecture ✅ Resolved Footnote [^34][^33] done; "Verify" verb done; all 5 assertion messages present
Testing (Unit And Behavioural) ✅ Resolved Same three sub-items all done
Domain Architecture ✅ Resolved Same three sub-items all done
Architectural Complexity And Maintainability ✅ Resolved Same three sub-items all done
Testing (Overall) Still open test_public_api.py.jinja uses the same Jinja conditional for expected_greeting and the implementation — tautological. Five string-pattern assertions in test_runtime_module_documents_and_limits_fallback check source structure rather than behaviour; a snapshot test would supersede them
Developer Documentation ⚠️ Still open docs/developers-guide.md Rust Integration section says nothing about _runtime.py, backend-selection rationale, or the stable __init__.py re-export contract
Testing (Compile-Time / UI) ⚠️ Still open tests/__snapshots__/test_template.ambr has snapshots for make help output and pure.py but none for the generated _runtime.py

AI agent prompts

1 — Fix tautological test_public_api.py.jinja and supersede implementation-detail string assertions with a snapshot test

## Task

Two related problems exist in the test suite for `leynos/agent-template-python`.

### Problem A — Tautological generated test

`template/tests/test_public_api.py.jinja` currently writes:

```python
expected_greeting = "{% if use_rust %}hello from Rust{% else %}hello from Python{% endif %}"
assert {{ package_name }}.hello() == expected_greeting, (
    "expected public hello() to return the configured backend greeting"
)
```

Because the same Jinja conditional drives both `expected_greeting` and the
`hello()` implementation in `_runtime.py.jinja`, a rendering bug that swaps the
condition in both files simultaneously would leave the test passing. Rewrite the
template so the expected value is a Jinja block literal rather than an inline
expression — this makes the expected string visible as a raw constant in the
rendered file and clarifies intent:

```jinja
{% if use_rust -%}
    expected_greeting = "hello from Rust"
{%- else -%}
    expected_greeting = "hello from Python"
{%- endif %}
```

### Problem B — Implementation-detail string assertions

`tests/test_template.py::test_runtime_module_documents_and_limits_fallback`
contains five `assert "..." in runtime_module` checks that verify source-code
fragments exist in the generated `_runtime.py`. These will become redundant once
a snapshot test is added (see the companion snapshot task). Replace them with a
single snapshot assertion so the entire generated module is pinned, rather than
spot-checking individual strings.

Concretely:

1. Add `snapshot: SnapshotAssertion` to the parameter list of
   `test_runtime_module_documents_and_limits_fallback`.
2. Replace the five `assert "..." in runtime_module` assertions with
   `assert runtime_module == snapshot`.
3. Run `pytest tests/test_template.py::test_runtime_module_documents_and_limits_fallback --snapshot-update`
   to capture the initial snapshot.
4. Remove the `# noqa` comments or any suppressions that were compensating for
   those five assertions if any are present.
5. Keep all assertions that verify *behavioural* outcomes (i.e. the two
   `subprocess.run` blocks and their `.returncode`/`.stderr` checks) — do not
   remove those.

### Verification

- `ruff check tests/test_template.py` — no errors.
- `ruff check template/tests/test_public_api.py.jinja` — no errors (render a
  concrete project first if needed).
- `pytest tests/test_template.py::test_runtime_module_documents_and_limits_fallback`
  passes with the updated snapshot.
- `pytest tests/test_template.py` — all tests pass.

2 — Document _runtime.py backend-selection architecture in docs/developers-guide.md

## Task

`docs/developers-guide.md` contains a "Rust Integration" section that describes
`rust_extension/` and maturin but says nothing about how the generated Python
package selects its backend at import time.

Extend the "Rust Integration" section to document the `_runtime.py` module and
its role. The addition must cover:

1. **Stable public entry point** — `__init__.py` is a pure re-export of `hello`
   from `._runtime`; application code should always import from the package
   root.
2. **Backend-selection module** — `_runtime.py` is the private module that
   decides at import time whether to use the compiled Rust extension or the
   pure-Python fallback. It is not part of the public API.
3. **Fallback logic** — when `use_rust` is enabled, `_runtime.py` uses
   `importlib.import_module` to attempt loading the Rust extension. A
   `ModuleNotFoundError` is caught only if `exc.name` matches
   `EXPECTED_RUST_MODULE_NAME`; any other missing-module failure is re-raised
   so transitive dependency errors are not silently swallowed.
4. **Pure-Python path** — when `use_rust` is disabled, `_runtime.py` imports
   `hello` directly from `.pure`.

Write in the same terse, imperative style as the existing section. Prefer short
declarative sentences. Do not introduce sub-headings — extend the existing
"Rust Integration" section with one or two paragraphs.

File to edit: `docs/developers-guide.md`

### Verification

- `markdownlint-cli2 docs/developers-guide.md` — no errors.
- `mdtablefix docs/developers-guide.md` — no diff (content is already tidy).
- Confirm the section mentions `_runtime.py`, `EXPECTED_RUST_MODULE_NAME`, and
  the `__init__.py` re-export contract.

3 — Add snapshot test for generated _runtime.py

## Task

`tests/__snapshots__/test_template.ambr` already holds snapshots for
`make help` output and the generated `pure.py` module. There is no snapshot for
the generated `_runtime.py`. Add a parameterised snapshot test that pins the
rendered `_runtime.py` for both `use_rust=True` and `use_rust=False`.

### Steps

1. Add a new test function `test_runtime_module_snapshot` to
   `tests/test_template.py`, following the same pattern as
   `test_pure_module_snapshot` (lines 99–159).

2. Parameterise it with `use_rust` in `[True, False]`. For each variant:
   - Render the project with `copier.copy(...)` supplying
     `project_name`, `package_name`, and `use_rust`.
   - Read `<package>/_runtime.py` using `read_generated_file`.
   - Assert `runtime_module == snapshot`.

3. Run `pytest tests/test_template.py::test_runtime_module_snapshot --snapshot-update`
   to write the two initial snapshots into `tests/__snapshots__/test_template.ambr`.

4. Confirm the snapshot file now contains two new entries — one for the
   `use_rust=True` variant (containing `importlib`, `EXPECTED_RUST_MODULE_NAME`,
   etc.) and one for `use_rust=False` (containing only `from .pure import hello`).

### Docstring requirements

Follow coding guidelines: NumPy-style docstring, action verb ("Verify") in the
summary line, Parameters / Returns / Raises / Examples sections, and no bare
assertions without messages.

### Verification

- `pytest tests/test_template.py::test_runtime_module_snapshot` — passes
  (two parametrised cases, both green).
- `pytest tests/test_template.py` — full suite passes.
- `ruff check tests/test_template.py` — no errors.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants