Separate Ninja generation steps from runner reporting (#343)

[leynos]

Summary

Closes #343

Splits generation into composable query-style steps with reporting kept in a thin runner wrapper, as proposed.

Changes

• src/runner/generation.rs (new): pure steps load_manifest (with an optional StageObserver callback), build_graph, and ninja_text — no StatusReporter anywhere; localised error contexts preserved.

• src/runner/mod.rs: generate_ninja now interleaves report_pipeline_stage calls between the pure steps (identical stage sequence); stage_reporting_callback maps manifest stages to reporter updates; load_manifest_with_stage_reporting becomes a thin wrapper.

• src/runner/graph.rs: shares generation::build_graph instead of duplicating context wrapping.

Testing

• New generation_steps_run_without_reporter unit test composes the pure pipeline end to end (manifest file → graph → Ninja text) with no reporter.

• All existing graph/build/manifest tests unchanged and green.

Validation

• make check-fmt / make lint / make test — pass (37 suites)

🤖 Generated with Claude Code

Summary by Sourcery

Separate reusable Ninja generation steps from runner reporting while preserving command-stage progress and generation behavior.

Enhancements:

• Introduce a reusable runner generation pipeline that separates manifest loading, graph construction, and Ninja synthesis from status reporting and command execution.
• Provide distinct read-only and effectful manifest-loading paths with preserved error context and capability boundaries.
• Reuse the shared generation graph-building path across generation, graph rendering, and help queries.

Documentation:

• Document the runner generation module, its reuse boundary, and composition rules.

Tests:

• Add unit and integration coverage for reporter-free generation, effectful-helper restrictions, error-context preservation, stage ordering, and isolated packaging subprocesses.

Enhancements:

• Introduce a dedicated generation module with pure manifest loading, build graph construction, and Ninja text synthesis steps usable without a status reporter.

• Refactor runner manifest loading to delegate to the generation pipeline while mapping manifest load stages into pipeline status updates.

• Reuse the shared generation-based graph-building path in the graph handler to avoid duplicated context wrapping.

Tests:

• Add an end-to-end test that exercises the generation pipeline (manifest → graph → Ninja) without any status reporter.

References

• Lody session

[coderabbitai]

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

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 0dade265-1427-4f61-a5fe-1a4213a52caf

📥 Commits

Reviewing files that changed from the base of the PR and between d533911 and 4b3d4be.

📒 Files selected for processing (4)

• src/runner/generation.rs
• src/runner/graph.rs
• src/runner/mod.rs
• src/runner/tests.rs

🔗 Linked repositories identified

CodeRabbit considers these linked repositories for cross-repo context during reviews:

• leynos/monotony (auto-detected)
• leynos/rstest-bdd (auto-detected)
• leynos/ortho-config (auto-detected)
• leynos/whitaker (auto-detected)
• leynos/shared-actions (auto-detected)

Included review availability: 1 review is currently available. Your included PR review attempts over the past 7 days set your current allowance at 4 reviews per hour.

---

Summary

• Separate Ninja generation from runner status reporting.
• Add composable helpers for manifest loading, graph construction, and Ninja text generation.
• Keep manifest stage observation optional and remove StatusReporter dependencies from generation.
• Keep pipeline stage reporting in the runner orchestration layer.
• Reuse shared graph construction and preserve the existing runner stage sequence.
• Add an end-to-end test for manifest-to-graph-to-Ninja generation without a reporter.
• Address issue #343 and pass formatting, linting, and test checks across 37 suites.

Walkthrough

The runner now delegates manifest loading, graph construction, and Ninja generation to reusable helpers. Stage reporting remains in the runner through an optional callback. A test verifies generation without a status reporter.

Changes

Generation and runner orchestration

Layer / File(s)	Summary
Add pure generation helpers src/runner/generation.rs	Add helpers for manifest loading, graph construction, and Ninja text generation. Support optional stage observation and contextual errors.
Wire generation into runner flows src/runner/mod.rs, src/runner/graph.rs	Delegate generation work to the private generation module. Preserve pipeline reporting and telemetry.
Validate reporter-free generation src/runner/tests.rs	Run generation without a reporter and verify the expected hello build edge in the Ninja output.

Suggested labels: Issue

Poem

Load the manifest, shape the graph,
Let Ninja trace the chosen path.
Report each stage, keep work pure,
Test the edge and make it sure.
Three small steps now work as one.

---

Caution

Pre-merge checks failed

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

• Ignore

❌ Failed checks (2 errors, 2 warnings)

Check name	Status	Explanation	Resolution
Testing (Overall)	❌ Error	The added test covers only one happy path and one build hello: substring; it never checks StageObserver forwarding or the runner's complete stage sequence.	Add tests that record all manifest stages through generation::load_manifest, assert exact 1–6 runner order, and cover load, graph, and Ninja error propagation.
Unit Architecture	❌ Error	The new query-style generation::load_manifest calls full-policy manifest loading, which registers fetch and shell helpers; templates can therefore make network calls and side effects behind a pure-...	Separate the read-only query loader from full manifest evaluation, or expose the latter as effectful and inject its environment, network, and filesystem dependencies; test the boundary.
Developer Documentation	⚠️ Warning	The PR adds the internal generation API and pure-generation/reporting boundary, but changes no documentation and the developer guide has no references to them.	Update docs/developers-guide.md with the three-step pipeline, StageObserver, and the runner reporting boundary; add or update an ADR or design record if required.
Testing (Unit And Behavioural)	⚠️ Warning	The sole added test calls private generation helpers directly, so it is not an end-to-end runner test; it also omits error paths and stage-observer reporting. Public generate tests were unchanged.	Add a public run or CLI test for manifest-to-Ninja generation and stage sequencing. Add focused tests for StageObserver callbacks and load or graph error contexts.

✅ Passed checks (16 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	The changes satisfy the composable generation, thin reporting wrapper, stage preservation, reuse, and test objectives in [#343].
Out of Scope Changes check	✅ Passed	All changes support the generation and reporting separation described in [#343]; no unrelated changes are present.
Docstring Coverage	✅ Passed	Docstring coverage is 90.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 10 functions across 4 files.
User-Facing Documentation	✅ Passed	Pass: the diff is an internal refactor; new helpers are pub(super), CLI behaviour and stage sequence remain unchanged, and no user-facing functionality requires guide updates.
Module-Level Documentation	✅ Passed	Accept the module documentation: new generation.rs states its purpose, three-step utility, and relationship to runner reporting; touched modules also retain clear //! summaries.
Testing (Property / Proof)	✅ Passed	Pass this check: the diff only extracts and delegates existing loader, graph, and Ninja operations; it introduces no invariant over varied inputs or states, and no proof assumption requiring propte...
Testing (Compile-Time / Ui)	✅ Passed	Accept this check: the PR refactors existing generation only; its build-edge assertion and existing Ninja snapshots cover output, with no new compile-time contract.
Domain Architecture	✅ Passed	The diff only refactors runner orchestration. generation delegates filesystem loading and existing IR/Ninja APIs; no adapter concerns enter ast or ir domain code.
Observability	✅ Passed	Accept this change: production paths retain the existing stage sequence, contextual errors, and Ninja telemetry; the new private generation steps add no operational path or unobserved failure mode.
Security And Privacy	✅ Passed	The diff only extracts existing manifest, graph, and Ninja calls; it adds no secrets, credentials, permissions, logging, process execution, or new trust-boundary handling.
Performance And Resource Use	✅ Passed	The diff only extracts existing manifest, graph, and Ninja calls; it adds no loops, collections, cloning, repeated I/O, retries, or blocking work.
Concurrency And State	✅ Passed	The diff adds synchronous, local generation steps; they borrow immutable inputs, and the observer is a caller-owned synchronous FnMut. No new tasks, locks, global state, or parallel execution appea...
Architectural Complexity And Maintainability	✅ Passed	The PR adds one private, small generation module with three explicit wrappers and an optional existing callback shape; runner and graph use it, with no new dependencies, traits, registries, global...
Rust Compiler Lint Integrity	✅ Passed	The diff adds no lint suppressions or clones. New generation items and the reporting callback have real callers, while obsolete direct generation paths are replaced.
Title check	✅ Passed	The title accurately summarises separating Ninja generation from runner reporting and references the linked issue (#343).
Description check	✅ Passed	The description directly explains the generation pipeline refactor, reporting separation, shared graph path, tests, and validation.

✨ 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 issue-343-pure-ninja-generation

Warning

Your free Security trial is over. An organization admin can activate billing to continue.

---

_{Comment @coderabbitai help to get the list of available commands.}

[sourcery-ai]

Reviewer's Guide

Refactors the Ninja generation pipeline into pure, composable steps in a new generation module, with runner-level reporting layered on top, and updates graph handling and tests to use the shared pipeline.

Sequence diagram for generate_ninja with separated generation steps

sequenceDiagram
    participant Runner
    participant Reporter
    participant Generation
    participant ManifestModule as manifest
    participant BuildGraphType as BuildGraph
    participant NinjaGen as ninja_gen

    Runner->>Reporter: report_pipeline_stage(PipelineStage::IrGenerationValidation)
    Runner->>Generation: generate_ninja
    activate Generation
    Generation->>BuildGraphType: build_graph(&manifest)
    deactivate Generation

    Runner->>Reporter: report_pipeline_stage(PipelineStage::NinjaSynthesisAndExecution)
    Runner->>Generation: ninja_text(&graph)
    activate Generation
    Generation->>NinjaGen: generate(&graph)
    deactivate Generation

    Generation-->>Runner: NinjaContent

Loading

Flow diagram for manifest loading and reporting separation

flowchart TD
    A[load_manifest_with_stage_reporting]
    B[stage_reporting_callback]
    C[report_pipeline_stage]
    D[generation::load_manifest]
    E[manifest::from_path_with_policy]

    A --> B
    B --> C
    A --> D
    D --> E

Loading

File-Level Changes

Change	Details	Files
Extract pure, composable Ninja generation steps into a new generation module and wire them into the runner.	Introduce generation::load_manifest, generation::build_graph, and generation::ninja_text as pure functions without StatusReporter dependencies Keep localized error context when delegating from previous inlined logic to the new generation functions Update generate_ninja to call the new generation steps while interleaving report_pipeline_stage calls to preserve the existing reporting sequence	src/runner/generation.rs src/runner/mod.rs
Decouple manifest stage reporting from loading logic via a reusable callback wrapper.	Add stage_reporting_callback helper that maps ManifestLoadStage values to PipelineStage values and emits report_pipeline_stage calls Refactor load_manifest_with_stage_reporting into a thin wrapper over generation::load_manifest using the new callback	src/runner/mod.rs
Have graph handling reuse the shared generation pipeline instead of duplicating graph construction.	Change graph handling to call generation::build_graph for BuildGraph construction Remove duplicated context-wrapping logic around BuildGraph::from_manifest in favor of the shared generation step	src/runner/graph.rs
Add an end-to-end test for the pure generation pipeline without any status reporter.	Create generation_steps_run_without_reporter test to run manifest → graph → Ninja text using the new generation functions Assert that the generated Ninja includes the expected build edge to validate end-to-end behavior	src/runner/tests.rs

Assessment against linked issues

Issue	Objective	Addressed	Explanation
#343	Create a pure (or mostly pure) generation path composed of reusable steps: load manifest, build graph, and generate Ninja text, without mandatory StatusReporter side effects.	✅
#343	Ensure runner commands still emit the same pipeline stage reporting sequence while delegating core work to the pure generation steps.	✅
#343	Keep existing graph/build/manifest tests passing and add coverage to confirm the pure generation pipeline works end-to-end (and passes formatting/lint/test checks).	✅

Possibly linked issues

• Separate Ninja generation steps from runner reporting #343: PR creates pure load_manifest/build_graph/ninja_text steps and keeps pipeline stage reporting in runner, matching issue goals.

---

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

• Contact our support team for questions or feedback.
• Visit our documentation for detailed guides and information.
• Keep in touch with the Sourcery team by following us on X/Twitter, LinkedIn or GitHub.

[coderabbitai]

Warning

Your free Security trial is over. An organization admin can activate billing to continue.

[sourcery-ai]

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

Please try again later or upgrade to continue using Sourcery

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 4b3d4be64b

ℹ️ 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".

[leynos]

@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 o11y, code safety, documentation and validation coverage as in scope).

❌ Failed checks (2 errors, 2 warnings)

Check name	Status	Explanation	Resolution
Testing (Overall)	❌ Error	The added test covers only one happy path and one build hello: substring; it never checks StageObserver forwarding or the runner's complete stage sequence.	Add tests that record all manifest stages through generation::load_manifest, assert exact 1–6 runner order, and cover load, graph, and Ninja error propagation.
Unit Architecture	❌ Error	The new query-style generation::load_manifest calls full-policy manifest loading, which registers fetch and shell helpers; templates can therefore make network calls and side effects behind a pure-...	Separate the read-only query loader from full manifest evaluation, or expose the latter as effectful and inject its environment, network, and filesystem dependencies; test the boundary.
Developer Documentation	⚠️ Warning	The PR adds the internal generation API and pure-generation/reporting boundary, but changes no documentation and the developer guide has no references to them.	Update docs/developers-guide.md with the three-step pipeline, StageObserver, and the runner reporting boundary; add or update an ADR or design record if required.
Testing (Unit And Behavioural)	⚠️ Warning	The sole added test calls private generation helpers directly, so it is not an end-to-end runner test; it also omits error paths and stage-observer reporting. Public generate tests were unchanged.	Add a public run or CLI test for manifest-to-Ninja generation and stage sequencing. Add focused tests for StageObserver callbacks and load or graph error contexts.

[leynos]

@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 o11y, code safety, documentation and validation coverage as in scope).

❌ Failed checks (2 errors, 2 warnings)

Check name	Status	Explanation	Resolution
Testing (Overall)	❌ Error	The added test covers only one happy path and one build hello: substring; it never checks StageObserver forwarding or the runner's complete stage sequence.	Add tests that record all manifest stages through generation::load_manifest, assert exact 1–6 runner order, and cover load, graph, and Ninja error propagation.
Unit Architecture	❌ Error	The new query-style generation::load_manifest calls full-policy manifest loading, which registers fetch and shell helpers; templates can therefore make network calls and side effects behind a pure-...	Separate the read-only query loader from full manifest evaluation, or expose the latter as effectful and inject its environment, network, and filesystem dependencies; test the boundary.
Developer Documentation	⚠️ Warning	The PR adds the internal generation API and pure-generation/reporting boundary, but changes no documentation and the developer guide has no references to them.	Update docs/developers-guide.md with the three-step pipeline, StageObserver, and the runner reporting boundary; add or update an ADR or design record if required.
Testing (Unit And Behavioural)	⚠️ Warning	The sole added test calls private generation helpers directly, so it is not an end-to-end runner test; it also omits error paths and stage-observer reporting. Public generate tests were unchanged.	Add a public run or CLI test for manifest-to-Ninja generation and stage sequencing. Add focused tests for StageObserver callbacks and load or graph error contexts.

[codescene-access]

No quality gates enabled for this code.