Conversation
Reviewer's GuideThis PR revises the Netsuke documentation to introduce a six-stage YAML-first build pipeline with deferred Jinja rendering, clarifies templating and iteration semantics (including prohibition of structural Jinja), and updates the roadmap to reflect the new data-first parsing approach. Flow diagram for the six-stage YAML-first build pipelineflowchart TD
A[Stage 1: Manifest Ingestion]
B[Stage 2: Initial YAML Parsing]
C[Stage 3: Template Expansion]
D[Stage 4: Deserialisation & Final Rendering]
E[Stage 5: IR Generation & Validation]
F[Stage 6: Ninja Synthesis & Execution]
A --> B --> C --> D --> E --> F
Flow diagram for foreach/when expansion and Jinja rendering restrictionsflowchart TD
A[Initial YAML Value]
B[Expand foreach/when keys with Jinja]
C[Preserve iteration context]
D[Deserialise to Rust structs]
E[Render Jinja only in string fields]
F[Prohibit structural Jinja]
A --> B --> C --> D --> E
B --> F
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. Warning Rate limit exceeded@leynos has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 5 minutes and 44 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (7)
Summary by CodeRabbit
WalkthroughSummarise the docs and script edits that switch Netsuke to a six-stage, YAML-first pipeline: parse manifest to an untyped YAML Value, expand foreach/when and macros over the Value, then deserialize to typed structs and render string fields; add glob() semantics and normalise spelling to "artifact." Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Netsuke
participant YAML as "YAML Parser"
participant Tmpl as "Template Expander"
participant Deser as "Deserialiser"
participant IR as "IR Validator"
participant Ninja as "Ninja Synth/Exec"
User->>Netsuke: Provide manifest
Netsuke->>YAML: Parse to serde_yaml::Value
YAML-->>Netsuke: Untyped YAML Value
Netsuke->>Tmpl: Expand foreach/when and macros (Value walk)
Tmpl-->>Netsuke: Expanded Value
Netsuke->>Deser: Deserialize into typed structs
Deser-->>Netsuke: Typed model (strings still templated)
Netsuke->>IR: Generate and validate IR/graph
IR-->>Netsuke: Valid IR
Netsuke->>Ninja: Synthesize build.ninja and execute
sequenceDiagram
participant Value as "YAML Value"
participant Exp as "Template Expander"
participant Env as "Jinja Env"
Value->>Exp: Encounter foreach: glob("assets/svg/*.svg")
Exp->>Env: Evaluate glob(), when filters
Env-->>Exp: Return sorted paths
loop For each item
Exp->>Exp: Preserve per-iteration environment
Exp-->>Value: Insert expanded entries into Value tree
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~18 minutes Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey there - I've reviewed your changes - here's some feedback:
- Update the CLI summary under “Netsuke build” to list all six pipeline stages (using the new stage names) instead of referencing the old five-stage flow.
- Unify spelling conventions across the document (e.g. artifact vs artefact, synthesize vs synthesise) for consistency.
- Refresh the pipeline diagram to include the sixth stage and resolve the nixie rendering error so the visualization matches the documented flow.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Update the CLI summary under “Netsuke build” to list all six pipeline stages (using the new stage names) instead of referencing the old five-stage flow.
- Unify spelling conventions across the document (e.g. artifact vs artefact, synthesize vs synthesise) for consistency.
- Refresh the pipeline diagram to include the sixth stage and resolve the nixie rendering error so the visualization matches the documented flow.
## Individual Comments
### Comment 1
<location> `docs/netsuke-design.md:37` </location>
<code_context>
The process of transforming a user's `Netsukefile` manifest into a completed
-build artifact follows a distinct, five-stage pipeline. This multi-stage data
-flow ensures that dynamic configurations are fully resolved into a static plan
+build artefact now follows a six-stage pipeline. This data flow validates the
+manifest as YAML first, then resolves all dynamic logic into a static plan
before execution, a critical requirement for compatibility with Ninja.
</code_context>
<issue_to_address>
'artefact' should be spelled 'artifact' per en-oxendic spelling.
Please change 'artefact' to 'artifact' to conform to en-oxendic spelling standards.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
![codescene-delta-analysis[bot]](https://avatars.githubusercontent.com/in/53921?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🔭 Outside diff range comments (3)
docs/netsuke-design.md (3)
66-76: Stage 5 text reads well; ensure “validation” is consistently referenced earlier.Keep “IR Generation & Validation” wording consistent in earlier mentions and diagrams.
867-873: Remove structural Jinja example; it violates the new prohibition.This example uses
{% if %}blocks. The updated design forbids structural Jinja. Replace with a string-only conditional example or move it to a “prior model (deprecated)” note.Suggest replacing with a string-level conditional example:
{{ ("config.yaml" | contents) if ("config.yaml" is file and "config.yaml" is readable) else "" }}
1078-1114: Fix malformed Ninja code fences in docs/netsuke-design.mdPinpoint and correct broken fences at lines 1078–1114:
Replace the opening and closing ```````ninja
fences around therule cc` snippet with a single matching pair:# Generated from an ir::Action rule cc command = gcc -c -o $out $in description = CC $out depfile = $out.d deps = gccWrap the
ir::BuildEdgeexample in its own properly closed fence and inline the implicit dependency:# Generated from an ir::BuildEdge build foo.o: cc foo.c build bar.o: cc bar.c build my_app: link foo.o bar.o | lib_dependency.aRemove stray backticks and ensure each code block opens with
ninja and closes withbefore the “Write Build Edges” heading.Verify with:
- make markdownlint
- make nixie
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
💡 Knowledge Base configuration:
- Jira integration is disabled
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
docs/netsuke-design.md(5 hunks)docs/roadmap.md(2 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.md
📄 CodeRabbit Inference Engine (AGENTS.md)
**/*.md: Markdown files must use en-GB-oxendict ("-ize" / "-yse" / "-our") spelling and grammar. (EXCEPTION: the naming of the "LICENSE" file, which is to be left unchanged for community consistency.)
Validate Markdown files using make markdownlint.
Run make fmt after any documentation changes to format all Markdown files and fix table markup.
Validate Mermaid diagrams in Markdown files by running make nixie.
Markdown paragraphs and bullet points must be wrapped at 80 columns.
Code blocks must be wrapped at 120 columns.
Tables and headings must not be wrapped.
Use dashes (-) for list bullets.
Use GitHub-flavoured Markdown footnotes ([^1]) for references and footnotes.
Files:
docs/roadmap.mddocs/netsuke-design.md
⚙️ CodeRabbit Configuration File
**/*.md: * Avoid 2nd person or 1st person pronouns ("I", "you", "we")
- Use en-GB-oxendict (-ize / -our) spelling and grammar
- Headings must not be wrapped.
- Documents must start with a level 1 heading
- Headings must correctly increase or decrease by no more than one level at a time
- Use GitHub-flavoured Markdown style for footnotes and endnotes.
- Numbered footnotes must be numbered by order of appearance in the document.
Files:
docs/roadmap.mddocs/netsuke-design.md
docs/**/*.md
📄 CodeRabbit Inference Engine (AGENTS.md)
docs/**/*.md: Reference: Use the markdown files within the docs/ directory as a knowledge base and source of truth for project requirements, dependency choices, and architectural decisions.
Update: When new decisions are made, requirements change, libraries are added/removed, or architectural patterns evolve, proactively update the relevant file(s) in the docs/ directory to reflect the latest state. Ensure the documentation remains accurate and current.
Files:
docs/roadmap.mddocs/netsuke-design.md
🔍 Remote MCP (Deepwiki)
Here are a few concrete facts that will help when reviewing this PR:
-
Existing pipeline (pre-PR #116) is a five-stage flow in docs/netsuke-design.md:
- Stage 1: Manifest Ingestion
- Stage 2: Jinja Evaluation
- Stage 3: YAML Parsing & Deserialization
- Stage 4: IR Generation & Validation
- Stage 5: Ninja Synthesis & Execution
-
That file also includes a Mermaid sequence diagram block showing these five stages, e.g.:
LoadingsequenceDiagram participant USER as "User" participant JINJA as "minijinja::Environment" … Note over JINJA: "Stage 2: Template rendering with glob(), env() functions" … Note over NINJA_GEN: "Stage 5: Ninja synthesis & execution"
This will need updating to reflect the six-stage pipeline.
-
The repository’s Makefile defines a
nixietarget (and AGENTS.md recommendsmake nixie) to validate Mermaid diagrams in Markdown. The reported “make nixie” failure means the updated diagram isn’t rendering correctly undernixie.
With this context, you can ensure that the new six-stage pipeline descriptions, the Mermaid diagram, and the nixie validation steps are all updated and in sync.
⏰ Context from checks skipped due to timeout of 120000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build-test
🔇 Additional comments (4)
docs/netsuke-design.md (4)
34-40: Good: Six-stage pipeline introduction landed cleanly.The six-stage framing and “artefact”/“synthesises” en-GB spelling are correct.
59-66: Correct: Stage 4 is string-only rendering and forbids structural tags.The restriction to string fields and prohibition of
{% %}blocks aligns with the YAML-first model.
77-84: LGTM: Stage 6 wording uses en-GB (“synthesises”).No changes required.
356-360: Reinforce prohibition: avoid implying structural templating.Keep emphasising that only YAML values carry templating; the structure must remain static.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
There was a problem hiding this comment.
Actionable comments posted: 6
🔭 Outside diff range comments (2)
docs/behavioural-testing-in-rust-with-cucumber.md (1)
21-26: Correct British spelling of “artefact” in narrative textReplace “artifact” with “artefact” in Markdown narrative (do not alter code literals such as
actions/upload-artifact). Wrap lines at 80 columns.
- docs/behavioural-testing-in-rust-with-cucumber.md (line 21)
-The tangible artifact of these conversations is a set of specifications written +The tangible artefact of these conversations is a set of specifications written- docs/netsuke-design.md (line 37)
-build artifact now follows a six-stage pipeline. These data flow validates the +build artefact now follows a six-stage pipeline. These data flow validates the- docs/netsuke-design.md (line 1570)
-build caching, Netsuke could implement a higher-level artifact caching layer. +build caching, Netsuke could implement a higher-level artefact caching layer.docs/netsuke-design.md (1)
34-97: Align Jinja documentation with the six-stage YAML-first pipelineRemove two-pass rendering references and structural Jinja examples; rename and rewrite the data-flow sections:
Section 3.3 (≈ lines 535–539):
• Rename to “Three-Phase Data Flow”
• Replace content with:
- Parse raw YAML into
serde_yml::Value- Expand templates on the Value (foreach/when + string expressions only)
- Deserialize into the Rust AST and perform final string rendering
Section 4.2 (≈ lines 663–668):
• Delete the “lenient first render” paragraph
• Forbid structural{% %}blocks; state that all templating occurs on YAML values and string fields onlyJinja structural example (≈ lines 859–863):
• Remove the{% if … %}/{% endif %}snippetIntegration instructions (≈ lines 1510–1513 & 1518–1520):
• Update “two-pass parsing” to describe a single, YAML-first template-expansion pass on theValuefollowed by strict deserialization
• Remove references to parsing-stage control-flow blocksThis aligns every reference to the deterministic, byte-for-byte reproducible six-stage pipeline.
♻️ Duplicate comments (4)
docs/roadmap.md (4)
82-86: Adopt en‑GB spelling and label the six‑stage pipeline phases explicitly.Use “deserialise” and embed Stage names to align with the six‑stage model.
- - [x] Implement data-first parsing: parse the manifest into a - `serde_yml::Value`, expand `foreach` and `when` entries with a Jinja - environment, then deserialize the expanded tree into the typed AST and - render remaining string fields. + - [x] Implement data-first parsing: parse the manifest into a + `serde_yml::Value` (Stage 2: Initial YAML Parsing), expand `foreach` + and `when` entries with a Jinja environment (Stage 3: Template Expansion), + then deserialise the expanded tree into the typed AST and render remaining + string fields (Stage 4: Deserialisation & Final Rendering).
92-97: Wrap at 80 columns and keep the structural templating prohibition crisp.Retain the rule; enforce wrapping and “deserialise” spelling elsewhere.
- - [x] Evaluate Jinja expressions only within string values, forbidding - structural tags such as `{% if %}` and `{% for %}`. + - [x] Evaluate Jinja expressions only within string values, forbidding + structural tags such as `{% if %}` and `{% for %}`.Note: Content is correct; run formatters to enforce wrapping.
#!/bin/bash set -euo pipefail make fmt make markdownlint
95-97: Document the per‑iteration context contract for foreach/when.Specify bound names and merge order to avoid ambiguity.
- [ ] Implement the `foreach` and `when` keys for target generation, carrying - the iteration context into subsequent rendering phases. + the iteration context into subsequent rendering phases. + Per‑iteration context: + - item: current element + - index: 0‑based index (optional) + - vars: resolved in order globals < target.vars < per‑iteration locals
109-113: Align success‑criteria language with en‑GB and Stage naming.Switch to “deserialise” and ensure the criteria reference the six‑stage names consistently.
- - [ ] Netsuke can successfully build a manifest that uses variables, - conditional logic within string values, the `foreach` and `when` keys, - custom macros, and the `glob()` function to discover and operate on source - files. + - [ ] Netsuke can successfully build a manifest that uses variables, + conditional logic within string values, the `foreach` and `when` keys, + custom macros, and the `glob()` function to discover and operate on source + files.Note: Content is fine; enforce en‑GB spelling (“deserialise”) elsewhere in this file.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
💡 Knowledge Base configuration:
- Jira integration is disabled
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (5)
docs/behavioural-testing-in-rust-with-cucumber.md(1 hunks)docs/netsuke-design.md(8 hunks)docs/roadmap.md(3 hunks)scripts/assert-file-absent.sh(2 hunks)scripts/assert-file-exists.sh(2 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.md
📄 CodeRabbit Inference Engine (AGENTS.md)
**/*.md: Markdown files must use en-GB-oxendict ("-ize" / "-yse" / "-our") spelling and grammar. (EXCEPTION: the naming of the "LICENSE" file, which is to be left unchanged for community consistency.)
Validate Markdown files using make markdownlint.
Run make fmt after any documentation changes to format all Markdown files and fix table markup.
Validate Mermaid diagrams in Markdown files by running make nixie.
Markdown paragraphs and bullet points must be wrapped at 80 columns.
Code blocks must be wrapped at 120 columns.
Tables and headings must not be wrapped.
Use dashes (-) for list bullets.
Use GitHub-flavoured Markdown footnotes ([^1]) for references and footnotes.
Files:
docs/behavioural-testing-in-rust-with-cucumber.mddocs/roadmap.mddocs/netsuke-design.md
⚙️ CodeRabbit Configuration File
**/*.md: * Avoid 2nd person or 1st person pronouns ("I", "you", "we")
- Use en-GB-oxendict (-ize / -our) spelling and grammar
- Headings must not be wrapped.
- Documents must start with a level 1 heading
- Headings must correctly increase or decrease by no more than one level at a time
- Use GitHub-flavoured Markdown style for footnotes and endnotes.
- Numbered footnotes must be numbered by order of appearance in the document.
Files:
docs/behavioural-testing-in-rust-with-cucumber.mddocs/roadmap.mddocs/netsuke-design.md
docs/**/*.md
📄 CodeRabbit Inference Engine (AGENTS.md)
docs/**/*.md: Reference: Use the markdown files within the docs/ directory as a knowledge base and source of truth for project requirements, dependency choices, and architectural decisions.
Update: When new decisions are made, requirements change, libraries are added/removed, or architectural patterns evolve, proactively update the relevant file(s) in the docs/ directory to reflect the latest state. Ensure the documentation remains accurate and current.
Files:
docs/behavioural-testing-in-rust-with-cucumber.mddocs/roadmap.mddocs/netsuke-design.md
🔍 Remote MCP (Deepwiki)
Relevant context from the existing repository that will inform your review:
-
Current docs describe a five-stage pipeline (no code enum exists for stages)
- Stage 1: Manifest Ingestion
- Stage 2: Jinja Evaluation
- Stage 3: YAML Parsing & Deserialization
- Stage 4: IR Generation & Validation
- Stage 5: Ninja Synthesis & Execution
(These are purely conceptual and not backed by a Rustenum.)
-
All structural Jinja constructs (e.g.
{% if %},{% for %}) are explicitly allowed in the existing design doc—Netsuke uses Minijinja to fully render control structures before YAML parsing. -
There is no in-code enum or single type representing “pipeline stages”; the five-stage model is documented only in Markdown.
Implications for PR #116:
- Ensure that references to a “six-stage” pipeline don’t leave behind calls to a missing stage‐enum in code or tests.
- Verify that code/tests don’t rely on structural Jinja tags being applied after YAML parsing (since the PR now prohibits structural templating outside string fields).
- Update any CI/docs scripts or examples (e.g., Mermaid/flowchart generation) to reflect the new stage count and templating restrictions.
⏰ Context from checks skipped due to timeout of 120000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build-test
🔇 Additional comments (4)
scripts/assert-file-absent.sh (1)
2-3: Spelling normalisation to “artifact” is fine in scripts; no functional change.Retain this in shell scripts; en‑GB spelling rules apply to Markdown, not scripts. Logic and diagnostics remain correct.
Also applies to: 15-15
scripts/assert-file-exists.sh (1)
2-3: Non-functional spelling tweak acknowledged.Keep “artifact” here for consistency with other scripts. Behaviour, exit codes, and manifest dump remain correct.
Also applies to: 15-15
docs/netsuke-design.md (2)
46-50: Keep YAML‑first phrasing precise.The phrasing is correct. No action needed beyond enforcing en‑GB spelling consistently elsewhere.
90-97: Fix Mermaid diagram rendering; remove escaped newlines in labelsRemove all
\nescapes from the node labels in docs/netsuke-design.md and switch to single-line labels to stabilise the Mermaid renderer.Pinpoint location:
- docs/netsuke-design.md (around lines 90–97)
Proposed diff:
```mermaid -flowchart TD - A[Stage 1:\nManifest Ingestion] --> B[Stage 2:\nInitial YAML Parsing] - B --> C[Stage 3:\nTemplate Expansion] - C --> D[Stage 4:\nDeserialisation & Final Rendering] - D --> E[Stage 5:\nIR Generation & Validation] - E --> F[Stage 6:\nNinja Synthesis & Execution] +flowchart TD + A[Stage 1: Manifest Ingestion] --> B[Stage 2: Initial YAML Parsing] + B --> C[Stage 3: Template Expansion] + C --> D[Stage 4: Deserialisation & Final Rendering] + D --> E[Stage 5: IR Generation & Validation] + E --> F[Stage 6: Ninja Synthesis & Execution]Manually preview the diagram in your local or Live Editor to confirm labels render correctly.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
1 participant
Summary
Testing
make fmtmake lintmake testmake markdownlintmake nixie(fails: Runtime error while rendering diagram)https://chatgpt.com/codex/tasks/task_e_689e835a8678832297752b53416887f0
Summary by Sourcery
Document the transition to a six-stage YAML-first templating pipeline, clarify Jinja usage constraints, and update the development roadmap accordingly.
New Features:
Enhancements:
Documentation: