Skip to content

Import Rust template tooling#32

Merged
leynos merged 30 commits into
mainfrom
rust-project-enhancements
May 25, 2026
Merged

Import Rust template tooling#32
leynos merged 30 commits into
mainfrom
rust-project-enhancements

Conversation

@leynos
Copy link
Copy Markdown
Owner

@leynos leynos commented May 23, 2026

Summary

This branch imports the requested Rust project tooling into the Copier template so generated projects use fast local Rust iteration, stricter linting, release metadata, and template tests that exercise the generated public gates.

Execplan: docs/execplans/rust-project-enhancements.md

Review walkthrough

Validation

  • make test 2>&1 | tee /tmp/test-agent-template-rust-rust-project-enhancements-tooling-import.out: passed, 9 tests.
  • make test 2>&1 | tee /tmp/test-agent-template-rust-rust-project-enhancements-final.out: passed, 9 tests.
  • coderabbit review --agent 2>&1 | tee /tmp/coderabbit-agent-template-rust-rust-project-enhancements-tooling-import.out: actionable findings were addressed. One later recommendation to replace action SHAs with version tags was intentionally not applied because the task explicitly required replacing versions with SHAs sourced through Firecrawl-assisted lookup.

Notes

The root AGENTS.md now states that this repository is a Copier template and documents how to run pytest-copier tests through uvx.

Summary by Sourcery

Import Rust template tooling into the Copier template so generated Rust projects include the requested metadata, tooling contracts, and CI behaviour, and ensure template tests validate these public gates.

New Features:

  • Add Copier prompts and Cargo.toml template fields for crates.io metadata and optional app-only cargo-binstall configuration.
  • Generate per-app GitHub Actions release workflow for cross-compiled binaries with SHA-pinned actions.
  • Configure generated Rust projects with Cranelift debug codegen, optional mold-based Linux linking, nextest-based testing, and Whitaker lint integration via Makefile and toolchain files.
  • Introduce a disposable Rust test stub so freshly generated projects have an initial nextest test target.

Enhancements:

  • Refine generated app and library stubs to align with stricter linting while clearly indicating they are temporary placeholders.
  • Update template CI workflow to pin shared actions by SHA, add Whitaker installation and caching, and configure LLVM-based linking only where coverage requires it.
  • Document repository purpose, validation flow, and pytest-copier usage in a new AGENTS.md and an execplan for Rust project enhancements.
  • Refactor pytest-copier template tests to centralize project rendering, exercise generated make-based quality gates, and assert the presence of the new tooling contracts.

CI:

  • Adjust root CI workflow to delegate formatting and linting to the template and focus on coverage generation for this repository.

leynos added 5 commits May 23, 2026 15:42
Add root guidance that identifies this repository as a Copier template and documents the pytest-copier test setup path. Record the living execution plan for the Rust template tooling import so future work can resume from repository evidence.
Render generated Rust projects with local Cranelift codegen, Linux mold linker configuration, nextest tests, Whitaker linting, crates.io metadata prompts, and cargo-binstall metadata for app projects.

Import the dear-diary CI shape with SHA-pinned actions, Whitaker caching, mold installation, and an llvm-cov lld fallback. Strengthen pytest-copier coverage so rendered app and library projects pass their public make all gate.
Update the living execution plan with the committed tooling milestone, CodeRabbit disposition, and final validation status before the branch is pushed for review.
Capture the final post-milestone template test evidence in the living execution plan before pushing the branch for pull request review.
Update the living execution plan with the pushed branch and draft pull request URL so the repository captures the completed delivery state.
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 23, 2026

Review Change Stack

Overview

This pull request imports Rust project tooling into the Copier template to support faster local Rust iteration, stricter linting, release automation, and comprehensive template validation. The implementation is documented in the new execution plan at docs/execplans/rust-project-enhancements.md.

Key Changes

Configuration & Metadata

  • copier.yaml: Exposes new Copier template variables for crate metadata (package_description, repository_url, homepage_url, package_keywords, package_categories) and a dev_target parameter (defaulting to x86_64-unknown-linux-gnu) for optional Linux target triple configuration. Updates default licensing holder and email values.

  • template/Cargo.toml.jinja: Generates additional package metadata fields (description, license, repository, homepage, keywords, categories, readme). For app flavour projects, conditionally generates a [package.metadata.binstall] section with cargo-binstall install-time configuration.

Build & Development Tooling

  • template/.cargo/config.toml.jinja: Configures Cranelift as the debug codegen backend, enables unstable codegen-backend feature, and conditionally adds Linux-specific linker configuration using mold via -fuse-ld=mold.

  • template/rust-toolchain.toml.jinja: Expands components list to include llvm-tools-preview and rustc-codegen-cranelift-preview alongside existing clippy and rustfmt.

  • template/Makefile.jinja: Adds Whitaker fallback resolution with user-local lookup paths (~/.local/bin/whitaker), updates the lint target to always run Whitaker with fallback support, gates doc-test execution on lib flavour, and integrates lld-based linker flags for coverage generation.

CI/CD & Release

  • .github/workflows/ci.yml (root repository): Pins Whitaker installer version via environment variable with caching, installs mold linker on Linux, sets up uv, installs cargo-nextest via cargo-binstall, and delegates formatting/linting to generated templates. Includes conditional Whitaker installation on cache miss.

  • template/.github/workflows/ci.yml: Parameterises Whitaker installer revision with caching, installs build dependencies (clang, lld, mold), sets up uv, and installs cargo-nextest. Adds linker configuration for clang/lld during coverage generation.

  • template/.github/workflows/{% if flavour == 'app' %}release.yml{% endif %}.jinja (new): Defines a release workflow triggered on semantic version tags (v*.*.*). Implements cross-compilation matrix (Linux x86_64/aarch64, Windows x86_64, macOS x86_64/aarch64, FreeBSD x86_64) using pinned cross tool, generates SHA256 checksums, and publishes binaries to GitHub Releases.

Testing & Validation

  • tests/test_template.py (major rewrite): Replaces simple render-based tests with comprehensive contract validation:

    • Centralised render_project() helper for consistent test project generation
    • Validates public gates (make all) for both app and lib flavours
    • test_makefile_resolves_whitaker_fallback(): Simulates Whitaker availability across system paths and asserts correct fallback behaviour
    • test_generated_tooling_contracts(): Extensive multi-flavour/multi-target validation including Cargo metadata, Makefile structure, CI workflow YAML, toolchain configuration, and app-specific release workflow validation
    • test_generated_structured_file_snapshots(): Snapshot coverage for generated configuration files via syrupy
    • Helper functions for parsing TOML/YAML, structured validation, and consistent error reporting
  • tests/conftest.py (new): Provides copier_template_paths() fixture restricting pytest-copier's template repository to only Copier input paths.

  • template/tests/stub.rs (new): Disposable Rust test stub asserting CARGO_MANIFEST_DIR environment variable; intended for replacement with real tests in generated projects.

Documentation

  • AGENTS.md (new): Copier-template-specific agent guidance documenting template purpose, validation test execution via uvx --with pytest-copier, feature/tooling guidance location, and debugging workflow.

  • docs/developers-guide.md (new): Parent repository developers guide describing the template-focused workflow, test execution, expected tooling requirements (Python, Rust, cargo-nextest, mbake, Whitaker, clang/lld/mold), and design notes on behaviour validation through parent tests.

  • docs/users-guide.md (new): User-facing guide explaining Copier prompts, mapping to generated configuration, Makefile targets, and local Linux dependencies.

  • docs/execplans/rust-project-enhancements.md (new): Execution plan documenting goals, constraints, risks, dated progress milestones, discoveries, decision log, and outcomes for the tooling enhancement.

  • template/README.md.jinja: Adds "Build Linkers" section documenting mold usage for development builds and lld for coverage, with installation instructions.

  • README.md: Expands "What you get" section with new metadata prompt fields and additional generated tooling (Cranelift, mold, nextest, Whitaker, lld-backed coverage). Updates Testing section with user-facing documentation links and parent-template requirements.

Dependencies & Infrastructure

  • Makefile: Test target now includes pyyaml and syrupy dependencies alongside pytest-copier.
  • requirements.txt: Adds syrupy>=4.0 for snapshot assertion testing.
  • tests/__snapshots__/test_template.ambr (new): Snapshot baseline for generated structured files including cargo config, CI workflow, Makefile, and release workflow.

Validation Notes

  • All tests pass (9 tests reported in parent template test suite)
  • Makefile nextest fallback restored per reviewer feedback; TEST_CMD now uses nextest when available with cargo test fallback
  • Test metadata coverage expanded: assertions added for Cargo.toml fields (homepage, keywords, categories) and app-specific binstall metadata
  • Large test refactor completed: test_generated_tooling_contracts() refactored into focused assertion helpers with module docstrings and descriptive error messages
  • CI release workflow comments added explaining +stable override for release builds and linker flag clearing
  • One intentional decision retained: SHA-pinned GitHub action references maintained per task constraints (tag-based references not applied)
  • Outstanding issue #34 opened for focused semantic assertions in snapshot tests

Execution Plan Reference

Detailed project rationale, constraints, decisions, and outcomes are documented in docs/execplans/rust-project-enhancements.md.

Walkthrough

Add Copier inputs for package metadata and dev_target; emit extended Cargo metadata and optional app binstall block; enable Cranelift and conditional Linux mold linking; update Makefile to resolve Whitaker and wire coverage flags; change CI to cache/install Whitaker and run make test; add release workflow template; expand parent tests, docs and snapshots to validate generated projects.

Changes

Rust Template Tooling Enhancements

Layer / File(s) Summary
Template input configuration
copier.yaml
New Copier variables capture crates.io metadata (package_description, repository_url, homepage_url, package_keywords, package_categories) and add dev_target for development build target.
Package metadata and distribution
template/Cargo.toml.jinja
Cargo template now emits description, license, repository, homepage, readme, templated keywords/categories, and conditionally a [package.metadata.binstall] block for app flavour.
Local development tooling and stubs
template/.cargo/config.toml.jinja, template/rust-toolchain.toml.jinja, template/Makefile.jinja, template/src/..., template/tests/stub.rs, template/README.md.jinja
Enable unstable codegen-backend with Cranelift, conditionally emit Linux [target.<dev_target>] with clang+-fuse-ld=mold, expand rust-toolchain components, update source stubs and add test stub validating Cargo env, document linker instructions, add WHITAKER resolution and coverage linker flags, and gate doc tests by flavour.
CI workflow and Whitaker caching
template/.github/workflows/ci.yml, .github/workflows/ci.yml
Add WHITAKER_INSTALLER_REV, pin action SHAs, install clang/lld/mold on Linux, set up uv and install cargo-nextest/mbake, cache Whitaker installer/binaries keyed by OS+revision, conditionally install and run whitaker-installer --cranelift, run make test after Setup Rust, and force clang/lld via env for coverage generation.
Release automation for app flavour
template/.github/workflows/{% if flavour == 'app' %}release.yml{% endif %}.jinja
Add tag-triggered release workflow template that cross-compiles for multiple OS/arch, caches cross, prepares per-target artefacts and checksums, and publishes them to GitHub Releases.
Tests and pytest-copier wiring
tests/test_template.py, tests/conftest.py, tests/__snapshots__/*
Introduce render_project helper and copier_template_paths fixture; refactor tests to run generated make all and make lint as quality gates; add parameterised tooling-contracts and Whitaker-resolution tests; add structured file snapshots and parsing/assertion helpers for TOML/YAML/Makefile/CI outputs.
Contributor guidance and execplan
AGENTS.md, docs/execplans/rust-project-enhancements.md, docs/developers-guide.md, docs/users-guide.md, README.md, requirements.txt, Makefile
Add AGENTS.md with repository-specific agent and test guidance; add execplan documenting goals, constraints, tolerances, risks, progress log, decisions and outcomes; add developer and user guides; update README prompts and testing links; add syrupy requirement; and extend Makefile test environment.

Possibly related issues

Possibly related PRs

Poem

🦀 Cranelift hums as builds align,
Whitaker hunts each linting sign,
Mold links fast where Linux leads,
Nextest rings true for test‑time deeds,
Generated projects cross the lines.

🚥 Pre-merge checks | ✅ 18 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Testing (Property / Proof) ⚠️ Warning Linker and metadata parsing invariants over unbounded inputs (targets, comma-separated strings) tested only with fixed examples; property-based testing needed. Add Hypothesis property-based tests for linker target configuration and CSV metadata parsing across diverse input distributions, including edge cases and non-Linux targets.
Testing (Compile-Time / Ui) ⚠️ Warning The snapshot test (test_generated_structured_file_snapshots) lacks paired semantic assertions, capturing 4 structured outputs as a single dict snapshot without explicit behavioural contracts. Pair the snapshot with focused assertions validating linker config, action SHAs, and Whitaker integration to prevent formatting churn and ensure snapshots encode meaningful behavioural expectations.
✅ Passed checks (18 passed)
Check name Status Explanation
Title check ✅ Passed The pull request title 'Import Rust template tooling' accurately and concisely summarises the main change: importing tooling (Cranelift, mold, nextest, Whitaker, etc.) into the Copier template for generated Rust projects.
Description check ✅ Passed The pull request description is directly related to the changeset, providing a comprehensive summary of the tooling imports, review walkthrough, validation results, and documentation updates aligned with the file-level changes.
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.
Testing (Overall) ✅ Passed Tests render actual projects, run make gates, parse TOML/YAML with value assertions, test failure modes, and snapshot structured output—would fail if implementations removed or inverted.
User-Facing Documentation ✅ Passed New features documented in docs/users-guide.md with distinct docs for template users, generated project users, and template maintainers.
Developer Documentation ✅ Passed Developers guide documents all tooling and architectural boundaries; execplan is complete with all progress items checked and outcomes list all delivered features; design decisions in Decision Log.
Module-Level Documentation ✅ Passed All modules carry module-level docstrings: tests/conftest.py, tests/test_template.py, and template/tests/stub.rs. All 22 functions include docstrings documenting purpose and relationships.
Testing (Unit And Behavioural) ✅ Passed 9 tests render projects, run functional gates via make all, cover flavour/target variants, Whitaker paths, with 37 contract assertions and error path testing.
Unit Architecture ✅ Passed Query and command paths are properly separated, dependencies injected, side-effects visible and isolated, no global state or hard-coded clients detected.
Domain Architecture ✅ Passed This PR modifies a Copier template generator with no core business domain code—only template configuration, test infrastructure, and build tooling. The domain architecture check does not apply.
Observability ✅ Passed Operational changes provide adequate observability: diagnostic logging at decision points, failure boundaries visible in tool output, and test validation assertions confirm observability presence.
Security And Privacy ✅ Passed No credentials, injection risks, or authorisation gaps found. Secrets use GitHub context properly, test data uses fake example.com, minimal job permissions, checkout disables persist-credentials.
Performance And Resource Use ✅ Passed No algorithmic regressions or unbounded resource growth detected. Test suite uses 18 bounded permutations; file parsing is linear; subprocess calls are explicit; no unbounded allocations found.
Concurrency And State ✅ Passed No concurrency patterns. Tests use function-scoped tmp_path fixtures; subprocess.run copies os.environ; CI workflows execute sequentially; generated projects contain no async/threading primitives.
Architectural Complexity And Maintainability ✅ Passed Test helpers have measurable reuse and value; Makefile variables use standard Make builtins; new dependency (syrupy) justified; no circular dependencies; architecture is proportional and documented.
Rust Compiler Lint Integrity ✅ Passed One #[expect(clippy::print_stdout)] on app stub with execplan link, properly documented. No #[allow], dead code, artificial references, or module suppressions. All Rust code is necessary.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch rust-project-enhancements

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

@leynos leynos marked this pull request as ready for review May 23, 2026 15:34
Drop the parent repository CI format and lint steps because the parent Makefile only exposes the supported template test target. Generated-project format and lint validation remains covered by pytest-copier rendering and make all.
@sourcery-ai
Copy link
Copy Markdown

sourcery-ai Bot commented May 23, 2026

Reviewer's Guide

Imports Rust tooling and metadata from an existing project into the Copier template so generated Rust apps/libs have Cranelift/mold setup, stricter linting, SHA-pinned CI, release workflows, and pytest-copier tests that exercise the generated public gates.

Flow diagram for generated project tooling and gates (Cranelift, mold, nextest, Whitaker)

flowchart LR
  Dev[Developer] -->|copier copy| CopierTemplate[Copier_template]
  CopierTemplate -->|render| GenProj[Generated_Rust_project]

  Dev -->|make lint| LintTarget[Makefile_lint]
  LintTarget --> CargoClippy[cargo_clippy]
  LintTarget --> Whitaker[whitaker_lints]

  Dev -->|make test| TestTarget[Makefile_test]
  TestTarget --> Nextest[cargo_nextest_run]

  GenProj -->|local_build| CargoConfig[.cargo_config_cranelift_mold]

  CI[GitHub_Actions_CI] -->|run make all| GenProj
Loading

File-Level Changes

Change Details Files
Add crates.io-style metadata prompts and cargo-binstall metadata to generated Cargo.toml for app and lib flavours.
  • Extend copier.yaml with prompts for description, repository URL, homepage URL, keywords, categories, and a dev_target for linker config.
  • Populate description, license, repository, homepage, readme, keywords, and categories fields in template/Cargo.toml.jinja.
  • Add [package.metadata.binstall] configuration for app flavour with a URL pattern suitable for cargo-binstall.
copier.yaml
template/Cargo.toml.jinja
Configure generated projects for Cranelift debug codegen and mold-based Linux linking, with LLVM-based linking reserved for CI coverage.
  • Introduce template/.cargo/config.toml.jinja enabling unstable codegen-backend, using Cranelift in dev profile, and optionally configuring a Linux dev_target linker to clang+mold.
  • Add rust-toolchain.toml.jinja components for clippy, llvm-tools-preview, rustc-codegen-cranelift-preview, and rustfmt.
  • Document the linker/Cranelift decisions and constraints in docs/execplans/rust-project-enhancements.md.
template/.cargo/config.toml.jinja
template/rust-toolchain.toml.jinja
docs/execplans/rust-project-enhancements.md
Tighten and centralize generated Makefile quality gates around nextest, Whitaker, and Clippy.
  • Update template/Makefile.jinja to prefer cargo-nextest for tests (with separate doc-tests for libs), and to standardize CARGO_FLAGS/CLIPPY_FLAGS usage.
  • Make Whitaker a first-class lint step by resolving it via PATH or a user-local path and running it with the same CARGO_FLAGS, ensuring PATH includes common user bin locations.
  • Introduce helper variables for user-local Whitaker and bin path handling.
template/Makefile.jinja
Update CI workflow to use SHA-pinned actions, Whitaker caching, mold/LLVM-aware coverage, and uv-based tooling setup.
  • Pin actions/checkout, leynos/shared-actions, DavidAnson/markdownlint-cli2-action, astral-sh/setup-uv, and actions/cache to specific SHAs in template/.github/workflows/ci.yml.
  • Install mold/LLD toolchain on Linux runners, cache Whitaker installation keyed by installer revision, and run Whitaker via a cached installer with Cranelift.
  • Configure the coverage step to use clang+lld via CARGO_TARGET_X86_64_UNKNOWN_LINUX_GNU_LINKER and RUSTFLAGS, matching llvm-cov expectations while keeping mold for local builds.
template/.github/workflows/ci.yml
Add a reusable GitHub Actions release workflow template for app flavour binaries using cross and pinned actions.
  • Introduce template/.github/workflows/{% if flavour == 'app' %}release.yml{% endif %}.jinja as a raw GHA definition that builds a matrix of OS/arch targets via cross and uploads artifacts per platform.
  • Cache the cross binary and Cargo registry using SHA-pinned actions/cache and ensure RUSTFLAGS are cleared when installing cross to avoid mold-specific flags.
  • Add a release job that downloads all artifacts and publishes them via softprops/action-gh-release pinned to a specific SHA.
template/.github/workflows/{% if flavour == 'app' %}release.yml{% endif %}.jinja
Strengthen pytest-copier tests to exercise generated projects’ public gates and assert tooling contracts.
  • Refactor tests/test_template.py to introduce helpers (render_project, run_quality_gates, read_generated_file) that render projects with publishable metadata and run make all.
  • Update existing tests to use render_project, ensure app/lib flavours include or omit release workflows appropriately, and switch Clippy tests to invoke make lint.
  • Add a new parametrized test that checks the presence and content of Cargo.toml metadata, Makefile nextest/Whitaker commands, .cargo/config.toml cranelift/mold config, ci.yml Whitaker cache/linker fallback, rust-toolchain components, and the disposable tests/stub.rs; for app flavour, also assert binstall metadata and release workflow pins.
tests/test_template.py
template/tests/stub.rs
Clarify template vs generated-project responsibilities and how to run tests via uv/pytest-copier.
  • Add a root AGENTS.md describing the repository as a Copier template, documenting pytest-copier usage via uvx and the expected make test entrypoint.
  • Provide guidance on keeping tests’ dependencies explicit via uvx --with and on using generated projects’ public gates for validation.
AGENTS.md
Polish generated Rust source templates to align with Clippy/rustdoc expectations and disposable stub guidance.
  • Adjust template/src main.rs/lib.rs stubs to emphasize their disposable nature and to use #[expect] for clippy::print_stdout rather than #[allow].
  • Condense the lib greet function into a single-line const fn while preserving documentation, and normalize Jinja conditionals for flavour selection.
template/src/{% if flavour == 'app' %}main.rs{% else %}lib.rs{% endif %}.jinja

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

sourcery-ai[bot]

This comment was marked as resolved.

@coderabbitai coderabbitai Bot added the Roadmap label May 23, 2026
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 3b94065abc

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 10

🤖 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 `@AGENTS.md`:
- Line 55: In AGENTS.md update the example command that uses an invalid git
option: replace the substring "git branch --show" with "git branch
--show-current" in the tee pipeline so the documented command becomes runnable;
ensure the rest of the line (the make test and tee usage) remains unchanged and
the updated example appears where the original command is shown.

In `@docs/execplans/rust-project-enhancements.md`:
- Around line 120-121: Remove or update the stale statement "The branch
currently points at the same commit as `main`" so it no longer contradicts later
progress entries; replace it with a short line that reflects the actual
committed/pushed state recorded later in the document (e.g., "Branch advanced
and changes committed/pushed as recorded below") ensuring the execution log
remains temporally consistent with subsequent entries and preserving the
original tone and formatting.

In `@template/.github/workflows/ci.yml`:
- Around line 21-27: The CI installs mold in the "Install mold linker" step but
the coverage job explicitly forces clang to use lld via -fuse-ld=lld, which is
confusing; update the repo to explicitly state and enforce this behavior: add a
short comment in the CI step clarifying that mold is installed for faster dev
builds while coverage uses lld for llvm-tools compatibility, ensure both mold
and lld remain installed in that step, and update the Makefile (e.g., the
default build target and the coverage target) so the default uses mold but the
coverage target sets RTFLAGS/CFLAGS/LDFLAGS to -fuse-ld=lld; also update the
README/docs to document that dev builds use mold and coverage generation
requires lld.

In `@template/Cargo.toml.jinja`:
- Around line 15-21: Add a short comment above the [package.metadata.binstall]
block in Cargo.toml.jinja explaining that the pkg-url string intentionally mixes
Jinja2 ({{ repository_url }}, {{ package_name }}) rendered at project generation
with single-brace placeholders ({ version }, { target }, { binary-ext }) which
are cargo-binstall variables substituted at install time; reference the pkg-url
key and the package.metadata.binstall section so future maintainers understand
the dual-template semantics.

In `@tests/test_template.py`:
- Around line 83-84: Update the two bare assertions in the test that check for
the created files—specifically the checks asserting (project / "src" /
"lib.rs").exists() and asserting not (project / ".github" / "workflows" /
"release.yml").exists()—to include descriptive assertion messages; replace each
bare assert with assert <condition>, "<clear message describing expected
condition and context>" so failures show which file existence check failed and
why (e.g., "expected lib.rs to exist in project src" and "expected release.yml
workflow not to be present").
- Around line 69-70: The two bare assertions in tests/test_template.py should
include descriptive failure messages: replace the assertions that check (project
/ "src" / "main.rs").exists() and (project / ".github" / "workflows" /
"release.yml").exists() with assertions that provide clear messages (e.g.,
indicating the missing file path or that project scaffolding failed) so test
failures print helpful context; update the two asserts to use the form assert
<condition>, "<descriptive message>" referring to those exact expressions.
- Line 96: Update the bare assertion in tests/test_template.py that currently
reads assert (project / "Makefile").exists() to include a descriptive message;
change the assertion for the expression (project / "Makefile").exists() to use
the assert ... , "message" form (e.g., include which project path or that the
Makefile is missing) so failures show a clear diagnostic instead of a silent
bare-assert failure.
- Around line 55-56: Replace the two bare asserts in tests/test_template.py with
assertions that include descriptive messages: for the Cargo manifest check,
update the assertion that calls (project / "Cargo.toml").exists() to include a
message like "expected Cargo.toml to exist in project root", and for the library
source check, update the assertion that calls (project / "src" /
f"{LIB}.rs").exists() to include a message like "expected src/{LIB}.rs to
exist"; locate and modify the two assertions referencing (project /
"Cargo.toml").exists() and (project / "src" / f"{LIB}.rs").exists() to add the
respective ", \"message\"" argument.
- Around line 1-13: Add a module-level triple-quoted docstring immediately after
the `from __future__ import annotations` line that explains this test suite's
purpose (validating the Copier Rust template with pytest-copier), how to use it
(e.g., run tests with `uvx pytest`), and notes about the key fixtures/variables
used (`CopierFixture`, `CopierProject`, `TEMPLATE_PATH`, `APP`, `LIB`) so future
readers understand intent and usage.
- Around line 146-168: Update every bare assert in tests/test_template.py (the
ones referencing cargo_toml, makefile, cargo_config, rust_toolchain,
ci_workflow, test_stub, and the APP/flavour conditional with release_workflow)
to include a descriptive message using the assert <condition>, "message" form;
for example describe what each contract checks like "Cargo.toml must include
description", "Makefile must invoke nextest", "Cargo config must enable
cranelift", "Rust toolchain must include llvm-tools-preview", "CI workflow must
cache Whitaker", "Test stub must include deletion note", and for the APP branch
messages such as "Binstall metadata must be present in Cargo.toml" and "Release
workflow must pin upload-artifact action". Ensure every assert in that block
gets a clear one-line message.
🪄 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: 61e25630-f08f-4c54-bb74-5b01163db70f

📥 Commits

Reviewing files that changed from the base of the PR and between 09056c7 and ca44b42.

📒 Files selected for processing (14)
  • .github/workflows/ci.yml
  • AGENTS.md
  • copier.yaml
  • docs/execplans/rust-project-enhancements.md
  • template/.cargo/config.toml.jinja
  • template/.github/workflows/ci.yml
  • template/.github/workflows/{% if flavour == 'app' %}release.yml{% endif %}.jinja
  • template/.github/workflows/{% if flavour == APP %}release.yml{% endif %}.jinja
  • template/Cargo.toml.jinja
  • template/Makefile.jinja
  • template/rust-toolchain.toml.jinja
  • template/src/{% if flavour == 'app' %}main.rs{% else %}lib.rs{% endif %}.jinja
  • template/tests/stub.rs
  • tests/test_template.py
💤 Files with no reviewable changes (2)
  • template/.github/workflows/{% if flavour == APP %}release.yml{% endif %}.jinja
  • .github/workflows/ci.yml

Comment thread AGENTS.md Outdated
Comment thread docs/execplans/rust-project-enhancements.md Outdated
Comment thread template/.github/workflows/ci.yml
Comment thread template/Cargo.toml.jinja
Comment thread tests/test_template.py
Comment thread tests/test_template.py Outdated
Comment thread tests/test_template.py Outdated
Comment thread tests/test_template.py Outdated
Comment thread tests/test_template.py Outdated
Comment thread tests/test_template.py Outdated
Replace the shared coverage action in the parent repository workflow with the supported make test target. Remove the dependent CodeScene upload because the parent template test target does not produce lcov output.
@coderabbitai coderabbitai Bot added Issue and removed Roadmap labels May 24, 2026
Limit pytest-copier's temporary template repository to the actual Copier inputs so CI does not copy the pull request checkout's .git directory. This avoids detached merge metadata making the fixture's synthetic git commit a no-op.
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 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 `@tests/conftest.py`:
- Around line 1-4: Add a module-level triple-quoted docstring immediately after
the "from __future__ import annotations" line that explains that this
tests.conftest module provides pytest fixtures used to configure pytest-copier's
template input selection during test runs, describes its purpose and basic usage
(how the fixtures affect template selection), and gives a short example of how a
test should reference those fixtures; ensure the docstring is placed at the top
of the module before any other imports or code.
🪄 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: 41c56e28-0587-450d-ad2d-2934ac4b4c89

📥 Commits

Reviewing files that changed from the base of the PR and between a16f3e2 and 6bbe943.

📒 Files selected for processing (1)
  • tests/conftest.py

Comment thread tests/conftest.py
Add a module docstring to tests.conftest explaining why pytest-copier only copies the actual Copier template inputs and how tests consume that fixture through the normal copier fixture.
@coderabbitai coderabbitai Bot added Roadmap and removed Issue labels May 24, 2026
Install mold, nextest, mbake, and Whitaker in the parent CI workflow so pytest-copier can run generated make all gates on GitHub runners. Also install nextest in rendered CI and fix the generated lib Makefile doctest recipe indentation.
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
tests/conftest.py (1)

18-21: 🧹 Nitpick | 🔵 Trivial | 💤 Low value

Expand fixture docstring to full numpy style.

The guideline requires full structured documentation for all public interfaces. A pytest fixture in conftest.py is a public interface within the test suite. Add a Returns section following numpy style.

📚 Proposed enhancement to use numpy-style docstring
 `@pytest.fixture`(scope="session")
 def copier_template_paths() -> list[str]:
-    """Copy only template inputs into pytest-copier's temporary Git repo."""
+    """
+    Copy only template inputs into pytest-copier's temporary Git repo.
+
+    Returns
+    -------
+    list[str]
+        File paths to include in the temporary repository, relative to
+        the repository root.
+    """
     return ["copier.yaml", "template"]

As per coding guidelines: "Use a single-line summary for private functions and methods, and full structured docs for all public interfaces."

🤖 Prompt for 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.

In `@tests/conftest.py` around lines 18 - 21, Expand the docstring for the public
pytest fixture copier_template_paths to a full numpy-style docstring: include a
short summary line, a blank line, a Parameters section (if applicable) and a
Returns section that documents the return type as list[str] and describes that
it returns the file/directory names to copy into pytest-copier's temporary Git
repo; update the triple-quoted string above the function copier_template_paths
accordingly.
🤖 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.

Outside diff comments:
In `@tests/conftest.py`:
- Around line 18-21: Expand the docstring for the public pytest fixture
copier_template_paths to a full numpy-style docstring: include a short summary
line, a blank line, a Parameters section (if applicable) and a Returns section
that documents the return type as list[str] and describes that it returns the
file/directory names to copy into pytest-copier's temporary Git repo; update the
triple-quoted string above the function copier_template_paths accordingly.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 860a930e-558c-4aaa-bce3-68a38516706e

📥 Commits

Reviewing files that changed from the base of the PR and between 6bbe943 and 6ccafb8.

📒 Files selected for processing (1)
  • tests/conftest.py

coderabbitai[bot]

This comment was marked as resolved.

Pin parent workflow actions to full commit SHAs and use the requested
shared-actions revision for the parent Rust setup action.

Document the template test module and helper contracts, then add explicit
failure messages to generated-project assertions.
coderabbitai[bot]

This comment was marked as resolved.

Restore the generated Makefile fallback to `cargo test` when
`cargo-nextest` is unavailable, and assert the fallback contract in the
template tests.

Parse generated `Cargo.toml` as TOML so metadata prompts and app
binstall fields are covered by exact structured assertions.
@leynos
Copy link
Copy Markdown
Owner Author

leynos commented May 24, 2026

@coderabbitai pause

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 24, 2026

✅ Actions performed

Reviews paused.

Explain that the generated `pkg-url` combines Copier-rendered Jinja2
values with cargo-binstall placeholders expanded later during install.
coderabbitai[bot]

This comment was marked as resolved.

Resolve the generated Makefile dry-run through an absolute make path, construct synthetic PATH values portably, and split the large tooling contract test into focused assertion helpers. Also fix the README article before lld-backed.
@leynos
Copy link
Copy Markdown
Owner Author

leynos commented May 24, 2026

@coderabbitai Have the following now been resolved?

tests/test_template.py (1)

238-433: 🛠️ Refactor suggestion | 🟠 Major | 🏗️ Heavy lift
Split the monolithic tooling-contract test into focused assertion helpers.
Extract domain-specific assertion blocks (Cargo metadata, Makefile contracts, CI workflow contracts, release workflow contracts) into helper functions. Keep orchestration in test_generated_tooling_contracts only. This restores maintainability and keeps complexity inside the repository limit.

♻️ Refactor outline
+def assert_cargo_contracts(package: dict[str, object], metadata: dict[str, object], flavour: str) -> None:
+    ...
+
+def assert_makefile_contracts(makefile: str) -> None:
+    ...
+
+def assert_ci_contracts(ci_workflow: str, parsed_ci_workflow: dict[str, object]) -> None:
+    ...
+
+def assert_toolchain_contracts(cargo_config: str, rust_toolchain: str, dev_target: str) -> None:
+    ...
+
+def assert_release_contracts(project: CopierProject, metadata: dict[str, object], flavour: str) -> None:
+    ...
+
 def test_generated_tooling_contracts(...):
-    # 70+ statements and mixed responsibilities
+    project = render_project(...)
+    ...
+    assert_cargo_contracts(package, metadata, flavour)
+    assert_makefile_contracts(makefile)
+    assert_ci_contracts(ci_workflow, parsed_ci_workflow)
+    assert_toolchain_contracts(cargo_config, rust_toolchain, dev_target)
+    assert_release_contracts(project, metadata, flavour)

As per coding guidelines: "Keep C90 / mccabe complexity ≤ 9" and "Follow single responsibility and CQRS (command/query segregation)".

🤖 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 @tests/test_template.py around lines 238 - 433, The
test_generated_tooling_contracts function is too large; refactor by extracting
related assertion groups into focused helper functions (e.g.
assert_cargo_contracts(cargo, package, metadata),
assert_makefile_contracts(makefile),
assert_ci_workflow_contracts(parsed_ci_workflow, ci_workflow), and
assert_release_workflow_contracts(release_workflow) ), move the corresponding
assertions from the body into those helpers, and replace the assertion blocks in
test_generated_tooling_contracts with calls to these helpers, passing only the
variables they need (cargo/package/metadata, makefile,
parsed_ci_workflow/ci_workflow, and release_workflow). Ensure each helper
contains only its domain assertions and keep test_generated_tooling_contracts as
orchestration that builds project artifacts and calls the helpers.

♻️ Duplicate comments (1)

README.md (1)

26-26: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win
Fix the indefinite article before "lld-backed".
Replace "an lld-backed coverage target" with "a lld-backed coverage target". The abbreviation "lld" is pronounced with a consonant sound (ell-ell-dee), so it requires "a" rather than "an".
Triage: [type:grammar]

📝 Proposed fix
-  fallback, Whitaker linting, and an lld-backed coverage target.
+  fallback, Whitaker linting, and a lld-backed coverage target.
🤖 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 @README.md at line 26, Change the indefinite article before "lld-backed" in
the README by replacing the phrase "an lld-backed coverage target" with "a
lld-backed coverage target" so the article matches the consonant pronunciation
of "lld".

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 24, 2026

Tip

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

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
echo "=== tests/test_template.py: helper function definitions ==="
grep -n "^def assert_cargo\|^def assert_makefile\|^def assert_ci\|^def assert_toolchain\|^def assert_release\|^def assert_readme\|^def assert_test_stub" tests/test_template.py

echo ""
echo "=== tests/test_template.py: test_generated_tooling_contracts body (lines 230-270) ==="
sed -n '230,270p' tests/test_template.py

echo ""
echo "=== README.md line 24-28 ==="
sed -n '24,28p' README.md

Length of output: 2589


@leynos — both items are resolved.


tests/test_template.py (238–433) — Split into focused assertion helpers ✅ Resolved

All required helpers are present and test_generated_tooling_contracts delegates to them:

Helper Line
assert_cargo_contracts 289
assert_makefile_contracts 330
assert_cargo_config_contracts 358
assert_toolchain_contracts 380
assert_ci_workflow_contracts 390
assert_readme_contracts 433
assert_test_stub_contracts 443
assert_release_workflow_contracts 450

The orchestrating function now calls these helpers directly after reading the generated files.


README.md line 26 — Indefinite article before "lld-backed" ✅ Resolved

The line now reads:

fallback, Whitaker linting, and a lld-backed coverage target.

leynos added 3 commits May 25, 2026 00:55
Add explicit parsing context for generated files, exercise Whitaker fallback behaviour through real Makefile execution, and snapshot generated structured tooling files. Log Whitaker and coverage linker diagnostics in generated workflows and Makefiles, and document the new snapshot test dependency.
Explain why the generated release workflow clears repository Rust flags before installing cross and why release builds explicitly use the stable toolchain.
coderabbitai[bot]

This comment was marked as resolved.

leynos added 2 commits May 25, 2026 01:11
Format the syrupy dependency name in the developer guide, make the lint subprocess non-raising behaviour explicit, and add a diagnostic message to the generated structured file snapshot assertion.
Link the app stub lint expectation to the tracked tooling plan, add root CI Whitaker diagnostics, and consolidate generated tooling contract assertions into one validator.
coderabbitai[bot]

This comment was marked as resolved.

Wrap PyYAML and Whitaker in inline code formatting so the tooling list is consistent with the surrounding entries.
@leynos
Copy link
Copy Markdown
Owner Author

leynos commented May 24, 2026

@coderabbitai please create a new GitHub issue for the following:

Testing (Compile-Time / Ui) | ⚠️ Warning | Snapshot test lacks semantic assertions. Snapshot comparison is bare without focused assertions on generated-file contracts (Makefile targets, CI structure, Cargo metadata). | Add focused assertions around snapshot comparison in test_generated_structured_file_snapshots to pair snapshot verification with explicit behavioural contracts.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 24, 2026

Tip

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

🧩 Analysis chain

@leynos — the issue has been created and assigned to you. It covers the requirement to pair the syrupy snapshot comparison in test_generated_structured_file_snapshots with focused assertions on Makefile targets, CI structure, and Cargo metadata, and calls out that the existing helpers (assert_makefile_contracts, assert_ci_workflow_contracts, assert_cargo_contracts) should be reused.


🏁 Script executed:

#!/bin/bash
gh issue create \
  --repo leynos/agent-template-rust \
  --title "test: add semantic assertions to test_generated_structured_file_snapshots" \
  --assignee leynos \
  --body "## Summary

The \`test_generated_structured_file_snapshots\` test in \`tests/test_template.py\` relies solely on syrupy snapshot comparison. Snapshot comparison is bare — it verifies that output has not changed but does not assert specific behavioural contracts on the generated files (e.g. Makefile targets, CI workflow structure, Cargo metadata fields).

## Problem

If a snapshot is regenerated (e.g. after an intentional template change), there is no safety net of focused assertions to confirm that the contracts that matter — Makefile phony targets, CI checkout/nextest/coverage steps, Cargo \`[package]\` and \`[package.metadata.binstall]\` fields — are still correct. A snapshot regeneration could silently swallow a regression.

## Proposed Action

Add focused assertions inside \`test_generated_structured_file_snapshots\`, paired with the snapshot comparison, to exercise:

- **Makefile contracts** — presence of expected phony targets (e.g. \`all\`, \`test\`, \`lint\`, \`coverage\`)
- **CI workflow contracts** — expected job steps (e.g. checkout, nextest install, Whitaker cache, coverage upload)
- **Cargo metadata contracts** — \`[package]\` fields (description, repository, homepage, keywords, categories) and, for the app flavour, \`[package.metadata.binstall]\` fields

These are the same contract categories exercised by \`assert_makefile_contracts\`, \`assert_ci_workflow_contracts\`, and \`assert_cargo_contracts\` in \`test_generated_tooling_contracts\`. Reuse or call those helpers directly where appropriate.

## References

- PR: `#32`
- Raised during code review of \`tests/test_template.py\`
- Requested by `@leynos`"

Length of output: 56


@leynos — issue #34 has been created: #34

Extract the GitHub Actions checkout step selection into a helper and reuse it for generated CI and release workflow contract checks.
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
tests/test_template.py (1)

381-435: 🧹 Nitpick | 🔵 Trivial | ⚖️ Poor tradeoff

Split this validator into thematic sub-helpers to reduce statement density.

The static analysis correctly flags 64 statements exceeding the 50-statement threshold. Extract themed assertion groups into private helpers:

  • _assert_cargo_package_contracts(...) for lines 397–435
  • _assert_makefile_contracts(...) for lines 437–466
  • _assert_cargo_config_contracts(...) for lines 468–485
  • _assert_ci_workflow_contracts(...) for lines 494–538
  • _assert_release_workflow_contracts(...) for lines 551–577

Each sub-helper receives only the parameters it requires, improving testability and localising future changes.

🤖 Prompt for 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.

In `@tests/test_template.py` around lines 381 - 435, The
assert_generated_tooling_contracts function is too large; split its assertion
blocks into focused private helpers and call them from
assert_generated_tooling_contracts. Create
_assert_cargo_package_contracts(package, metadata, flavour),
_assert_makefile_contracts(makefile),
_assert_cargo_config_contracts(cargo_config),
_assert_ci_workflow_contracts(parsed_ci_workflow, ci_workflow, test_stub), and
_assert_release_workflow_contracts(release_workflow) and move the corresponding
themed assertions into each helper (retaining checks for binstall inside
_assert_cargo_package_contracts using flavour), then update
assert_generated_tooling_contracts to call these helpers with only the
parameters they need.
🤖 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.

Outside diff comments:
In `@tests/test_template.py`:
- Around line 381-435: The assert_generated_tooling_contracts function is too
large; split its assertion blocks into focused private helpers and call them
from assert_generated_tooling_contracts. Create
_assert_cargo_package_contracts(package, metadata, flavour),
_assert_makefile_contracts(makefile),
_assert_cargo_config_contracts(cargo_config),
_assert_ci_workflow_contracts(parsed_ci_workflow, ci_workflow, test_stub), and
_assert_release_workflow_contracts(release_workflow) and move the corresponding
themed assertions into each helper (retaining checks for binstall inside
_assert_cargo_package_contracts using flavour), then update
assert_generated_tooling_contracts to call these helpers with only the
parameters they need.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 44ccbd69-3af3-4261-bdc9-dbf9a9bf47b2

📥 Commits

Reviewing files that changed from the base of the PR and between 7388745 and e5722cd.

📒 Files selected for processing (4)
  • .github/workflows/ci.yml
  • docs/developers-guide.md
  • template/src/{% if flavour == 'app' %}main.rs{% else %}lib.rs{% endif %}.jinja
  • tests/test_template.py

@leynos
Copy link
Copy Markdown
Owner Author

leynos commented May 24, 2026

@coderabbitai Have the following now been resolved?

tests/test_template.py (1)

563-575: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
Extract the checkout-step extraction logic into a reusable helper.
The list comprehension that extracts checkout steps from workflow jobs (lines 563–570) duplicates the logic at lines 489–496 in assert_ci_workflow_contracts. Extract this to a shared helper to avoid maintaining the same pattern in two places.

♻️ Proposed extraction
def extract_checkout_steps(jobs: dict[str, Any]) -> list[dict[str, Any]]:
    """Extract checkout steps from workflow jobs."""
    return [
        step
        for job in jobs.values()
        if isinstance(job, dict)
        for step in job.get("steps", [])
        if isinstance(step, dict)
        and step.get("uses", "").startswith("actions/checkout@")
    ]
🤖 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 @tests/test_template.py around lines 563 - 575, Extract the checkout-step
selection into a shared helper named extract_checkout_steps(jobs: dict[str,
Any]) that returns the list of checkout step dicts (the comprehension currently
assigned to release_checkout_steps and duplicated in
assert_ci_workflow_contracts); replace both occurrences by calling
extract_checkout_steps(jobs) and use its result in the existing assertions (keep
assert names and messages the same), ensuring the helper checks isinstance(job,
dict) and isinstance(step, dict) and uses step.get("uses",
"").startswith("actions/checkout@") so behavior is unchanged.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 24, 2026

Tip

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

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
echo "=== extract_checkout_steps definition ==="
grep -n "def extract_checkout_steps\|extract_checkout_steps" tests/test_template.py

echo ""
echo "=== lines 480-510 (assert_ci_workflow_contracts) ==="
sed -n '480,510p' tests/test_template.py

echo ""
echo "=== lines 555-585 ==="
sed -n '555,585p' tests/test_template.py

Length of output: 3134


@leynos — yes, this is resolved. ✅

extract_checkout_steps is defined at line 580 and both former duplication sites now call it:

  • Line 495 — assert_ci_workflow_contracts: checkout_steps = extract_checkout_steps(jobs)
  • Line 556 — assert_release_workflow_contracts: release_checkout_steps = extract_checkout_steps(jobs)

The raw list comprehension no longer appears in either location.

Move the dense generated tooling validator into focused private assertion helpers while preserving the existing checks and failure messages.
@leynos
Copy link
Copy Markdown
Owner Author

leynos commented May 24, 2026

@coderabbitai Have the following now been resolved?

tests/test_template.py (1)

15-23: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win
Convert public helper docstrings to full NumPy-style structured docstrings.
Replace the single-line docstrings for render_project, run_quality_gates, and read_generated_file with structured NumPy-style docs that include parameters and returns.
As per coding guidelines, "Docstrings must follow the numpy style guide... full structured docs for all public interfaces."
Also applies to: 40-47

🤖 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 @tests/test_template.py around lines 15 - 23, Update the single-line
docstrings for the public helpers render_project, run_quality_gates, and
read_generated_file to full NumPy-style structured docstrings: add a short
summary line, a Parameters section listing each parameter name, type and brief
description (e.g., tmp_path: Path, copier: CopierFixture, project_name: str,
package_name: str, flavour: str = LIB), and a Returns section describing the
return type (e.g., CopierProject or appropriate type) and meaning; ensure each
docstring follows numpy conventions (summary, blank line, "Parameters" header,
indented parameter entries, blank line, "Returns" header) and replace the
existing single-line docstrings in the functions render_project,
run_quality_gates, and read_generated_file accordingly.

tests/test_template.py (2)

55-56: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win
Add descriptive failure messages to every bare assert.
Attach a clear message to each bare assertion in these ranges so failures identify the broken contract immediately.
As per coding guidelines, "Use assert …, \"message\" over bare asserts".
Also applies to: 69-70, 83-84, 96-96, 146-170

🤖 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 @tests/test_template.py around lines 55 - 56, Update every bare assert in
tests/test_template.py (including the occurrences checking (project /
"Cargo.toml").exists(), (project / "src" / f"{LIB}.rs").exists(), and the other
ranges noted: lines around 69-70, 83-84, 96, and 146-170) to include a
descriptive failure message using the "assert , "message"" form; for
each assertion, craft a concise message identifying the expected file or
condition (e.g., "expected Cargo.toml to exist in generated project", "expected
src/{LIB}.rs to exist", or "expected to hold for ") so a failed test immediately states which contract broke and why.

1-3: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Add the required module docstring at the top of the file.
Insert a triple-quoted module docstring immediately after Line 1 describing purpose, utility, and usage of this test module.
As per coding guidelines, "Every module must begin with a triple-quoted docstring explaining 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 @tests/test_template.py around lines 1 - 3, Add a triple-quoted module
docstring immediately after the first line (right after "from future import
annotations") in tests/test_template.py that states the purpose of this test
module, what it verifies (utility), how to run it or any fixtures it expects
(usage), and a brief example of running the tests or example call; ensure the
docstring is the first top-level string in the file and includes enough context
for future contributors and CI consumers.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 24, 2026

Tip

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

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
echo "=== Lines 1-60 of tests/test_template.py ==="
sed -n '1,60p' tests/test_template.py

echo ""
echo "=== render_project definition and docstring ==="
awk '/^def render_project/,/^def [a-zA-Z_]/' tests/test_template.py | head -50

echo ""
echo "=== run_quality_gates and read_generated_file definitions ==="
grep -n "def run_quality_gates\|def read_generated_file\|def read_generated_text\|def parse_toml_file\|def parse_yaml_mapping\|def require_mapping\|def require_optional_mapping\|def assert_generated_tooling\|def extract_checkout" tests/test_template.py

echo ""
echo "=== All bare assert statements (without message) ==="
grep -n "^\s*assert " tests/test_template.py | grep -v ', "' | grep -v ", '" | head -40

echo ""
echo "=== Lines 50-110 ==="
sed -n '50,110p' tests/test_template.py

echo ""
echo "=== Lines 135-175 ==="
sed -n '135,175p' tests/test_template.py

Length of output: 8440


🏁 Script executed:

#!/bin/bash
echo "=== Lines 318-335 (assert { context) ==="
sed -n '318,335p' tests/test_template.py

echo ""
echo "=== Docstrings for read_generated_text, parse_toml_file, parse_yaml_mapping ==="
awk '/^def read_generated_text/,/^def [a-zA-Z_]/' tests/test_template.py | head -30

echo ""
echo "=== Docstrings for require_mapping, require_optional_mapping ==="
awk '/^def require_mapping/,/^def [a-zA-Z_]/' tests/test_template.py | head -30

echo ""
echo "=== Lines 606-630 (extract_checkout_steps) ==="
sed -n '606,630p' tests/test_template.py

echo ""
echo "=== Check for any assert without a message string (no comma after) ==="
python3 - <<'EOF'
import ast, sys

with open("tests/test_template.py") as f:
    src = f.read()

tree = ast.parse(src)
lines = src.splitlines()

for node in ast.walk(tree):
    if isinstance(node, ast.Assert) and node.msg is None:
        print(f"Line {node.lineno}: {lines[node.lineno - 1].strip()}")
EOF

Length of output: 1397


@leynos — all three items are resolved. ✅


Module docstring (1-3) ✅ Resolved
Lines 1–12 contain a full triple-quoted module docstring covering purpose, utility, usage, and an example make test invocation.


NumPy-style structured docstrings (15-23, 40-47) ✅ Resolved
render_project carries a complete NumPy-style docstring with a Parameters section (all six parameters typed and described) and a Returns section. The two originally named helpers run_quality_gates and read_generated_file no longer exist in the file, so those requirements are moot.


Descriptive failure messages on bare assert (55-56, 69-70, 83-84, 96, 146-170) ✅ Resolved
An AST-level parse of the file confirms zero bare assert statements without a message — every assertion now carries an explicit failure string.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
tests/test_template.py (1)

411-579: 🛠️ Refactor suggestion | 🟠 Major | 🏗️ Heavy lift

Split this test module to enforce the 400-line limit.

Move parsing helpers and tooling-contract assertion helpers into dedicated support modules and keep this file focused on scenario-level tests. The current module exceeds the maximum logical line count and bundles too many responsibilities in one place.

As per coding guidelines, "Files must not exceed 400 logical lines: Decompose large modules into subpackages".

🤖 Prompt for 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.

In `@tests/test_template.py` around lines 411 - 579, This test module exceeds the
400-line limit; extract the parsing helpers and tooling-assertion helpers into
dedicated support modules and keep only scenario-level tests here. Create a new
support module (e.g., tests/support/tooling_asserts.py) and move helper
functions such as _assert_cargo_package_contracts, _assert_makefile_contracts,
_assert_cargo_config_contracts, _assert_ci_workflow_contracts and any parsing
helpers they depend on into it, update imports in tests/test_template.py to
import those helpers, and ensure all references (function names and any utility
helpers like require_mapping or extract_checkout_steps) are updated and tests
still import only scenario-level fixtures and calls. Ensure
tests/test_template.py remains focused on high-level scenarios and falls below
400 logical lines after relocation.
🤖 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.

Outside diff comments:
In `@tests/test_template.py`:
- Around line 411-579: This test module exceeds the 400-line limit; extract the
parsing helpers and tooling-assertion helpers into dedicated support modules and
keep only scenario-level tests here. Create a new support module (e.g.,
tests/support/tooling_asserts.py) and move helper functions such as
_assert_cargo_package_contracts, _assert_makefile_contracts,
_assert_cargo_config_contracts, _assert_ci_workflow_contracts and any parsing
helpers they depend on into it, update imports in tests/test_template.py to
import those helpers, and ensure all references (function names and any utility
helpers like require_mapping or extract_checkout_steps) are updated and tests
still import only scenario-level fixtures and calls. Ensure
tests/test_template.py remains focused on high-level scenarios and falls below
400 logical lines after relocation.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: c9bb09fe-f009-4659-8f54-4ab42723f3a5

📥 Commits

Reviewing files that changed from the base of the PR and between e5722cd and f788402.

📒 Files selected for processing (1)
  • tests/test_template.py

@leynos
Copy link
Copy Markdown
Owner Author

leynos commented May 25, 2026

@coderabbitai please create a new GitHub issue for the following:

Please address the following concerns and ensure all commit gates succeed:

⚠️ Outside diff range comments (1)

tests/test_template.py (1)

411-579: 🛠️ Refactor suggestion | 🟠 Major | 🏗️ Heavy lift
Split this test module to enforce the 400-line limit.
Move parsing helpers and tooling-contract assertion helpers into dedicated support modules and keep this file focused on scenario-level tests. The current module exceeds the maximum logical line count and bundles too many responsibilities in one place.
As per coding guidelines, "Files must not exceed 400 logical lines: Decompose large modules into subpackages".

🤖 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 @tests/test_template.py around lines 411 - 579, This test module exceeds the
400-line limit; extract the parsing helpers and tooling-assertion helpers into
dedicated support modules and keep only scenario-level tests here. Create a new
support module (e.g., tests/support/tooling_asserts.py) and move helper
functions such as _assert_cargo_package_contracts, _assert_makefile_contracts,
_assert_cargo_config_contracts, _assert_ci_workflow_contracts and any parsing
helpers they depend on into it, update imports in tests/test_template.py to
import those helpers, and ensure all references (function names and any utility
helpers like require_mapping or extract_checkout_steps) are updated and tests
still import only scenario-level fixtures and calls. Ensure
tests/test_template.py remains focused on high-level scenarios and falls below
400 logical lines after relocation.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 25, 2026

Tip

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

🧩 Analysis chain

@leynos — the issue has been created and assigned to you. It covers extracting the parsing helpers and tooling-contract assertion helpers from tests/test_template.py into tests/support/tooling_asserts.py, with the goal of keeping the top-level test module below 400 logical lines and focused solely on scenario-level tests.


🏁 Script executed:

#!/bin/bash
gh issue create \
  --repo leynos/agent-template-rust \
  --assignee leynos \
  --title "refactor(tests): split test_template.py to enforce 400-line module limit" \
  --body "## Summary

\`tests/test_template.py\` currently exceeds 400 logical lines, violating the repository guideline that files must not exceed 400 logical lines and should be decomposed into subpackages when they do.

## Problem

The module bundles three distinct concerns in one file:

- **Parsing helpers** — \`read_generated_text\`, \`parse_toml_file\`, \`parse_yaml_mapping\`, \`require_mapping\`, \`require_optional_mapping\`
- **Tooling-contract assertion helpers** — \`assert_cargo_contracts\`, \`assert_makefile_contracts\`, \`assert_cargo_config_contracts\`, \`assert_ci_workflow_contracts\`, \`assert_toolchain_contracts\`, \`assert_readme_contracts\`, \`assert_test_stub_contracts\`, \`assert_release_workflow_contracts\`, \`extract_checkout_steps\`
- **Scenario-level tests** — \`test_template_renders\`, \`test_generated_tooling_contracts\`, \`test_generated_structured_file_snapshots\`, etc.

Mixing these in one file makes it harder to locate failures, increases cyclomatic complexity, and violates the single-responsibility guideline.

## Proposed Action

1. Create \`tests/support/__init__.py\` (empty or with a brief docstring).
2. Create \`tests/support/tooling_asserts.py\` and move all tooling-contract assertion helpers into it, along with any parsing utilities they depend on (\`require_mapping\`, \`extract_checkout_steps\`, \`parse_toml_file\`, \`parse_yaml_mapping\`, \`read_generated_text\`).
3. Update \`tests/test_template.py\` to import from \`tests.support.tooling_asserts\` instead of defining these locally.
4. Ensure \`tests/test_template.py\` falls below 400 logical lines and contains only scenario-level fixtures and test functions.
5. Verify all tests pass (\`make test\`) after the relocation.

## References

- PR: `#32`
- Flagged during code review of \`tests/test_template.py\` lines 411–579
- Requested by `@leynos`"

Length of output: 56


@leynos — issue #36 has been created: #36

It covers splitting tests/test_template.py into a scenario-level test file and a tests/support/tooling_asserts.py support module, bringing the top-level file below the 400-logical-line limit.

@leynos leynos merged commit ffe151a into main May 25, 2026
3 checks passed
@leynos leynos deleted the rust-project-enhancements branch May 25, 2026 00:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant