Conversation
Reviewer's GuideUpdate the netsuke design document to cover IR decoupling from Ninja and deterministic build graph generation, refine placeholder handling and action hashing, clarify interpreter and Windows script behavior, tighten manifest validation, and document new options and security notes alongside naming and typo corrections. Class diagram for updated NetsukeManifest structureclassDiagram
class NetsukeManifest {
+String netsuke_version
+HashMap<String, serde_yaml::Value> vars
...
}
Class diagram for IR Action deduplication and hashingclassDiagram
class Action {
+String command_text
+String interpreter
+HashMap<String, Value> local_vars
+Option<Depfile> depfile
+hash(): String
}
Action : hash() uses command_text, interpreter, local_vars, depfile
Class diagram for Recipe enum validationclassDiagram
class Recipe {
<<enum>>
+Rule
+Command
+Script
}
class RecipeConflictError {
+String message
}
Recipe <|-- RecipeConflictError : emits on mutual exclusivity violation
Class diagram for placeholder expansion and escapingclassDiagram
class CommandTemplate {
+expand_ins_outs()
+shell_escape()
}
CommandTemplate : expand_ins_outs() expands {{ins}} and {{outs}} to shell-escaped lists
CommandTemplate : shell_escape() applies to all other interpolations
Class diagram for script interpreter selection and Windows behaviorclassDiagram
class Script {
+String content
+Option<String> interpreter
+bool has_shebang()
+get_interpreter()
}
Script : get_interpreter() returns shebang, manifest interpreter, or default
Script : On Windows, default is powershell -Command
Class diagram for glob function determinismclassDiagram
class GlobFunction {
+glob(pattern: &str): Result<Vec<String>, Error>
}
GlobFunction : returns lexicographically sorted results
GlobFunction : follows symlinks
Class diagram for shell() filter and caching behaviorclassDiagram
class ShellFilter {
+apply()
+impure: bool
}
ShellFilter : impure disables template caching between stages
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Warning Rate limit exceeded@leynos has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 3 minutes and 21 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 (1)
Summary by CodeRabbit
Summary by CodeRabbit
WalkthroughUpdate the Netsuke design documentation to clarify and expand details on manifest schema, IR generation, command and script handling, error reporting, CLI behaviour, and determinism. Correct field names, enforce recipe field exclusivity, standardise naming conventions, and describe new CLI features and output formats. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Netsuke CLI
participant Manifest Parser
participant IR Generator
participant Ninja Synthesiser
participant Ninja (Subprocess)
User->>Netsuke CLI: Invoke build/graph/other subcommand
Netsuke CLI->>Manifest Parser: Parse manifest (validate fields, enforce exclusivity)
Manifest Parser->>IR Generator: Produce backend-agnostic IR (expand commands, resolve scripts)
IR Generator->>Ninja Synthesiser: Generate build.ninja (replace macros, shell-escape paths)
Ninja Synthesiser->>Ninja (Subprocess): Invoke ninja for actual build
Ninja (Subprocess)-->>User: Output build results
alt Error occurs
Netsuke CLI-->>User: Emit error (optionally as JSON with --diag-json)
end
Possibly related PRs
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. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
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 @leynos - I've reviewed your changes and they look great!
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location> `docs/netsuke-design.md:1189` </location>
<code_context>
- using tools like Graphviz. This allows users to visually inspect their build
- dependencies, which is invaluable for understanding and debugging complex
- projects.
+ with the graph tool, `ninja -t graph`.3 This outputs the complete build
+ dependency graph in the DOT language. The result can be piped through `dot
+ -Tsvg` or displayed via `netsuke graph --html` using an embedded Dagre.js
</code_context>
<issue_to_address>
Possible stray '.3' after 'ninja -t graph'; clarify or remove.
'.3' seems to be an unintended typo; please clarify its purpose or remove it.
</issue_to_address>
<suggested_fix>
<<<<<<< SEARCH
run the Netsuke pipeline up to Stage 4 (IR Generation) and then invoke Ninja
with the graph tool, `ninja -t graph`.3 This outputs the complete build
dependency graph in the DOT language. The result can be piped through `dot
-Tsvg` or displayed via `netsuke graph --html` using an embedded Dagre.js
=======
run the Netsuke pipeline up to Stage 4 (IR Generation) and then invoke Ninja
with the graph tool, `ninja -t graph`. This outputs the complete build
dependency graph in the DOT language. The result can be piped through `dot
-Tsvg` or displayed via `netsuke graph --html` using an embedded Dagre.js
>>>>>>> REPLACE
</suggested_fix>
### Comment 2
<location> `docs/netsuke-design.md:83` </location>
<code_context>
+ Netsuke's pipeline is **deterministic**. Given the same `Netsukefile` and
+ environment variables, the generated `build.ninja` will be byte-for-byte
+ identical. This property is essential for reproducible builds and makes the
+ output suitable for caching or source control.
+
### 1.3 The Static Graph Mandate
</code_context>
<issue_to_address>
Paragraph exceeds 80 columns; please wrap to 80 columns as per guidelines.
The added paragraph about determinism is not wrapped to 80 columns. Please reformat it so that no line exceeds 80 characters, in accordance with the documentation style guide.
</issue_to_address>
### Comment 3
<location> `docs/netsuke-design.md:674` </location>
<code_context>
| `snake_case` / `camel_case` / `kebab-case` | Rename helpers |
-| `shell_escape` | POSIX-quote for safe `$()` usage |
+
+All built-in filters use `snake_case`. The `camel_case` helper is provided in
+place of `camelCase` so naming remains consistent with `snake_case` and `kebab-
+case`.
</code_context>
<issue_to_address>
Paragraph exceeds 80 columns; please wrap to 80 columns.
The paragraph about built-in filters and naming conventions is not wrapped to 80 columns. Please adjust the formatting.
</issue_to_address>
### Comment 4
<location> `docs/netsuke-design.md:699` </location>
<code_context>
| `grep`, `sed`, `awk`, `cut`, `wc`, `tr` | filters | Canonical wrappers implemented via `shell()` for convenience |
+Using `shell()` marks the template as *impure* and disables caching of the
+rendered YAML between Stage 2 and Stage 3. This avoids accidental
+reuse of results that depend on external commands.
+
</code_context>
<issue_to_address>
Line exceeds 80 columns; please wrap to 80 columns.
The line about rendered YAML and caching is longer than 80 columns. Please wrap it to comply with the documentation guidelines.
</issue_to_address>
### Comment 5
<location> `docs/netsuke-design.md:865` </location>
<code_context>
- Ninja's variables (`$in`, `$out`).
+ `ir::Action`, write a corresponding Ninja `rule` statement. The input and
+ output lists stored in the action replace the `ins` and `outs` placeholders.
+ These lists are then rewritten as Ninja's `$in` and `$out`
When an action's `recipe` is a script, the generated rule wraps the script
</code_context>
<issue_to_address>
Line exceeds 80 columns; please wrap to 80 columns.
The line about rewriting lists as Ninja's variables is longer than 80 columns. Please wrap it.
</issue_to_address>
### Comment 6
<location> `docs/netsuke-design.md:978` </location>
<code_context>
+and build the final command string step by step. The placeholders `{{ ins }}
+` and `{{ outs }}` are expanded to space-separated lists of file paths within
+Netsuke itself. Each path is shell escaped using the `shell-quote` API. When the
+command is written to `build.ninja`, these lists are replaced with Ninja's `$in`
+and `$out` macros. After substitution the command is validated with [`shlex`]
+(<https://docs.rs/shlex/latest/shlex/>) to ensure it parses correctly. This
</code_context>
<issue_to_address>
Line exceeds 80 columns; please wrap to 80 columns.
The line about writing to `build.ninja` is longer than 80 columns. Please wrap it.
</issue_to_address>
### Comment 7
<location> `docs/netsuke-design.md:1190` </location>
<code_context>
- dependencies, which is invaluable for understanding and debugging complex
- projects.
+ with the graph tool, `ninja -t graph`.3 This outputs the complete build
+ dependency graph in the DOT language. The result can be piped through `dot
+ -Tsvg` or displayed via `netsuke graph --html` using an embedded Dagre.js
+ viewer. Visualising the graph is invaluable for understanding and debugging
</code_context>
<issue_to_address>
Line exceeds 80 columns; please wrap to 80 columns.
The line about the DOT language and piping is not wrapped to 80 columns. Please reformat it.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
docs/netsuke-design.md(16 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
docs/**/*.md
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- AGENTS.md
**/*.md
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- AGENTS.md
⚙️ CodeRabbit Configuration File
🧠 Learnings (1)
docs/netsuke-design.md (1)
Learnt from: CR
PR: leynos/netsuke#0
File: docs/netsuke-design.md:0-0
Timestamp: 2025-07-12T13:36:08.546Z
Learning: Applies to docs/src/ninja_gen.rs : All command variable substitutions in the Ninja file generator must use the 'shell-quote' crate for shell escaping; do not use simple string formatting (like format!) for command construction.
🪛 LanguageTool
docs/netsuke-design.md
[uncategorized] ~176-~176: Loose punctuation mark.
Context: ...ng identifier for the rule. - command: A single command string to be executed....
(UNLIKELY_OPENING_PUNCTUATION)
[style] ~182-~182: Consider using a more formal/concise alternative here.
Context: ...x/latest/> shlex/). Any interpolation other than ins or outs is automatically shell ...
(OTHER_THAN)
[uncategorized] ~185-~185: Loose punctuation mark.
Context: ...tomatically shell escaped. - script: A multi-line script declared with the Y...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~186-~186: Possible missing comma found.
Context: ...n interpreter. If the first line begins with #! Netsuke executes the script ver...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~187-~187: A comma may be missing after the conjunctive/linking adverb ‘Otherwise’.
Context: ...cript verbatim, respecting the shebang. Otherwise the block is wrapped in the interpret...
(SENT_START_CONJUNCTIVE_LINKING_ADVERB_COMMA)
[style] ~233-~233: Would you like to use the Oxford spelling “deserialization”? The spelling ‘deserialisation’ is also correct.
Context: ...arser validates this exclusivity during deserialisation. If multiple fields are provided, Netsu...
(OXFORD_SPELLING_Z_NOT_S)
[grammar] ~865-~865: Please add a punctuation mark at the end of paragraph.
Context: ...sts are then rewritten as Ninja's $in and $out When an action's recipe is...
(PUNCTUATION_PARAGRAPH_END)
[style] ~942-~942: Would you like to use the Oxford spelling “minimize”? The spelling ‘minimise’ is also correct.
Context: ...oot. This early acknowledgement aims to minimise surprises on CI systems. ### 6.2 The C...
(OXFORD_SPELLING_Z_NOT_S)
[uncategorized] ~979-~979: Possible missing comma found.
Context: ... Ninja's $in and $out macros. After substitution the command is validated with [shlex]...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~1090-~1090: Possible missing comma found.
Context: ...to the specific. For automation use cases Netsuke will support a --diag-json fl...
(AI_HYDRA_LEO_MISSING_COMMA)
[style] ~1091-~1091: Would you like to use the Oxford spelling “serialized”? The spelling ‘serialised’ is also correct.
Context: ...When enabled, the entire error chain is serialised to JSON, allowing editors and CI tools ...
(OXFORD_SPELLING_Z_NOT_S)
[style] ~1192-~1192: Would you like to use the Oxford spelling “Visualizing”? The spelling ‘Visualising’ is also correct.
Context: ...l` using an embedded Dagre.js viewer. Visualising the graph is invaluable for understandi...
(OXFORD_SPELLING_Z_NOT_S)
|
@sourcery-ai resolve |
|
@sourcery-ai review |
There was a problem hiding this comment.
Hey @leynos - I've reviewed your changes - here's some feedback:
- Move the Windows PowerShell default into the interpreter section so that all script interpreter behavior (including shebang and OS-specific defaults) is documented together.
- Use a single placeholder notation (
{{ ins }}/{{ outs }}) consistently rather than mixing in{ins}forms to avoid confusion. - Standardize on either American or British spelling (e.g. “synthesizes” vs “synthesises”) throughout the document for a consistent style.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Move the Windows PowerShell default into the interpreter section so that all script interpreter behavior (including shebang and OS-specific defaults) is documented together.
- Use a single placeholder notation (`{{ ins }}`/`{{ outs }}`) consistently rather than mixing in `{ins}` forms to avoid confusion.
- Standardize on either American or British spelling (e.g. “synthesizes” vs “synthesises”) throughout the document for a consistent style.
## Individual Comments
### Comment 1
<location> `docs/netsuke-design.md:75` </location>
<code_context>
The final, validated IR is traversed by a code generator. This generator
- synthesizes the content of a build.ninja file, translating the IR's nodes
+ synthesises the content of a `build.ninja` file, translating the IR's nodes
and edges into corresponding Ninja rule and build statements. Once the file
- is written, Netsuke invokes the ninja executable as a subprocess, passing
</code_context>
<issue_to_address>
Inconsistent spelling: 'synthesises' vs. 'synthesizes'.
Since the rest of the document uses American English, please use 'synthesizes' for consistency.
</issue_to_address>
<suggested_fix>
<<<<<<< SEARCH
synthesises the content of a `build.ninja` file, translating the IR's nodes
=======
synthesizes the content of a `build.ninja` file, translating the IR's nodes
>>>>>>> REPLACE
</suggested_fix>
### Comment 2
<location> `docs/netsuke-design.md:232` </location>
<code_context>
- Only one of `rule`, `command`, or `script` may be specified. The parser
- validates this exclusivity during deserialisation.
+Only one of `rule`, `command`, or `script` may be specified. The parser
+validates this exclusivity during deserialisation. If multiple fields are
+provided, Netsuke emits a `RecipeConflict` error with the message "rule, command
+and script are mutually exclusive".
</code_context>
<issue_to_address>
Inconsistent spelling: 'deserialisation' vs. 'deserialization'.
Use 'deserialization' to match the rest of the document's American English spelling.
Suggested implementation:
```
Only one of `rule`, `command`, or `script` may be specified. The parser
validates this exclusivity during deserialization. If multiple fields are
provided, Netsuke emits a `RecipeConflict` error with the message "rule, command
and script are mutually exclusive".
```
```
This union deserializes into the same `Recipe` enum used for rules. The parser
enforces that only one variant is present, maintaining backward compatibility
```
</issue_to_address>
### Comment 3
<location> `docs/netsuke-design.md:1090` </location>
<code_context>
the specific.
+For automation use cases, Netsuke will support a `--diag-json` flag. When
+enabled, the entire error chain is serialised to JSON, allowing editors and CI
+tools to annotate failures inline.
+
</code_context>
<issue_to_address>
Inconsistent spelling: 'serialised' vs. 'serialized'.
Please update to 'serialized' to match American English usage throughout the document.
</issue_to_address>
<suggested_fix>
<<<<<<< SEARCH
For automation use cases, Netsuke will support a `--diag-json` flag. When
enabled, the entire error chain is serialised to JSON, allowing editors and CI
tools to annotate failures inline.
=======
For automation use cases, Netsuke will support a `--diag-json` flag. When
enabled, the entire error chain is serialized to JSON, allowing editors and CI
tools to annotate failures inline.
>>>>>>> REPLACE
</suggested_fix>
### Comment 4
<location> `docs/netsuke-design.md:75` </location>
<code_context>
The final, validated IR is traversed by a code generator. This generator
- synthesizes the content of a build.ninja file, translating the IR's nodes
+ synthesises the content of a `build.ninja` file, translating the IR's nodes
and edges into corresponding Ninja rule and build statements. Once the file
- is written, Netsuke invokes the ninja executable as a subprocess, passing
</code_context>
<issue_to_address>
"synthesises" uses en-gb spelling; en-oxendic (Oxford English) prefers "synthesizes".
Oxford English (en-oxendic) spelling prefers "-ize" endings, so "synthesizes" should be used instead of "synthesises".
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
|
||
| The final, validated IR is traversed by a code generator. This generator | ||
| synthesizes the content of a build.ninja file, translating the IR's nodes | ||
| synthesises the content of a `build.ninja` file, translating the IR's nodes |
There was a problem hiding this comment.
suggestion (typo): Inconsistent spelling: 'synthesises' vs. 'synthesizes'.
Since the rest of the document uses American English, please use 'synthesizes' for consistency.
| synthesises the content of a `build.ninja` file, translating the IR's nodes | |
| synthesizes the content of a `build.ninja` file, translating the IR's nodes |
| Only one of `rule`, `command`, or `script` may be specified. The parser | ||
| validates this exclusivity during deserialisation. | ||
| Only one of `rule`, `command`, or `script` may be specified. The parser | ||
| validates this exclusivity during deserialisation. If multiple fields are |
There was a problem hiding this comment.
suggestion (typo): Inconsistent spelling: 'deserialisation' vs. 'deserialization'.
Use 'deserialization' to match the rest of the document's American English spelling.
Suggested implementation:
Only one of `rule`, `command`, or `script` may be specified. The parser
validates this exclusivity during deserialization. If multiple fields are
provided, Netsuke emits a `RecipeConflict` error with the message "rule, command
and script are mutually exclusive".
This union deserializes into the same `Recipe` enum used for rules. The parser
enforces that only one variant is present, maintaining backward compatibility
| For automation use cases, Netsuke will support a `--diag-json` flag. When | ||
| enabled, the entire error chain is serialised to JSON, allowing editors and CI | ||
| tools to annotate failures inline. |
There was a problem hiding this comment.
suggestion (typo): Inconsistent spelling: 'serialised' vs. 'serialized'.
Please update to 'serialized' to match American English usage throughout the document.
| For automation use cases, Netsuke will support a `--diag-json` flag. When | |
| enabled, the entire error chain is serialised to JSON, allowing editors and CI | |
| tools to annotate failures inline. | |
| For automation use cases, Netsuke will support a `--diag-json` flag. When | |
| enabled, the entire error chain is serialized to JSON, allowing editors and CI | |
| tools to annotate failures inline. |
|
|
||
| The final, validated IR is traversed by a code generator. This generator | ||
| synthesizes the content of a build.ninja file, translating the IR's nodes | ||
| synthesises the content of a `build.ninja` file, translating the IR's nodes |
There was a problem hiding this comment.
suggestion (review_instructions): "synthesises" uses en-gb spelling; en-oxendic (Oxford English) prefers "synthesizes".
Oxford English (en-oxendic) spelling prefers "-ize" endings, so "synthesizes" should be used instead of "synthesises".
Review instructions:
Path patterns: **/*.md
Instructions:
Use en-oxendic spelling and grammar.
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (8)
docs/netsuke-design.md (8)
75-83: Wrap Stage 5 paragraph to ≤ 80 columnsLines 75 – 83 still exceed the mandated 80-column limit. Hard-wrap them without altering content.
575-578: Wrapglobdeterminism sentenceThese lines remain over 80 columns; fold them.
673-675: Wrap filter-naming explanationLine length still violates MD013. Re-wrap.
697-700: Wrapshell()impurity noteLines exceed 80 columns; break lines accordingly.
831-835: Wrap action-dedup bulletStill over 80 columns; re-flow.
973-980: Re-wrap command-generation guidanceLines exceed 80 columns; fold them.
1090-1092: Limit--diag-jsonexplanation to 80 columnsLine length still violates style guide.
1187-1192: Wrap graph command descriptionLines are wider than allowed; re-wrap.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
docs/netsuke-design.md(16 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
docs/**/*.md
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- AGENTS.md
**/*.md
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- AGENTS.md
⚙️ CodeRabbit Configuration File
🧠 Learnings (1)
docs/netsuke-design.md (1)
Learnt from: CR
PR: leynos/netsuke#0
File: docs/netsuke-design.md:0-0
Timestamp: 2025-07-12T13:36:08.546Z
Learning: Applies to docs/src/ninja_gen.rs : All command variable substitutions in the Ninja file generator must use the 'shell-quote' crate for shell escaping; do not use simple string formatting (like format!) for command construction.
🪛 LanguageTool
docs/netsuke-design.md
[style] ~75-~75: Would you like to use the Oxford spelling “synthesizes”? The spelling ‘synthesises’ is also correct.
Context: ... by a code generator. This generator synthesises the content of a build.ninja file, tr...
(OXFORD_SPELLING_Z_NOT_S)
[uncategorized] ~176-~176: Loose punctuation mark.
Context: ...ng identifier for the rule. - command: A single command string to be executed....
(UNLIKELY_OPENING_PUNCTUATION)
[style] ~182-~182: Consider using a more formal/concise alternative here.
Context: ...lex/latest/shlex/). Any interpolation other than ins or outs is automatically shell-...
(OTHER_THAN)
[uncategorized] ~184-~184: Loose punctuation mark.
Context: ...automatically shell-escaped. - script: A multi-line script declared with the Y...
(UNLIKELY_OPENING_PUNCTUATION)
[style] ~232-~232: Would you like to use the Oxford spelling “deserialization”? The spelling ‘deserialisation’ is also correct.
Context: ...arser validates this exclusivity during deserialisation. If multiple fields are provided, Netsu...
(OXFORD_SPELLING_Z_NOT_S)
[style] ~941-~941: Would you like to use the Oxford spelling “minimize”? The spelling ‘minimise’ is also correct.
Context: ...oot. This early acknowledgement aims to minimise surprises on CI systems. ### 6.2 The C...
(OXFORD_SPELLING_Z_NOT_S)
[style] ~1090-~1090: Would you like to use the Oxford spelling “serialized”? The spelling ‘serialised’ is also correct.
Context: ...When enabled, the entire error chain is serialised to JSON, allowing editors and CI tools ...
(OXFORD_SPELLING_Z_NOT_S)
[style] ~1191-~1191: Would you like to use the Oxford spelling “Visualizing”? The spelling ‘Visualising’ is also correct.
Context: ...l` using an embedded Dagre.js viewer. Visualising the graph is invaluable for understandi...
(OXFORD_SPELLING_Z_NOT_S)
| Importantly, the IR contains **no Ninja-isms**. Placeholders such as `$in` | ||
| and `$out` are resolved to plain lists of file paths, and command strings are | ||
| expanded before hashing. This deliberate absence of Ninja-specific syntax makes | ||
| the IR a stable contract that future back-ends—distributed builders, remote | ||
| executors, or otherwise—can consume without modification. |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Wrap “no Ninja-isms” paragraph
Lines 748 – 752 surpass 80 columns. Hard-wrap them.
-Importantly, the IR contains **no Ninja-isms**. Placeholders such as `$in`
-and `$out` are resolved to plain lists of file paths, and command strings are
-expanded before hashing. This deliberate absence of Ninja-specific syntax makes
-the IR a stable contract that future back-ends—distributed builders, remote
-executors, or otherwise—can consume without modification.
+Importantly, the IR contains **no Ninja-isms**. Placeholders such as `$in`
+and `$out` are resolved to plain lists of file paths, and command strings
+are expanded before hashing. This deliberate absence of Ninja-specific
+syntax makes the IR a stable contract that future back-ends—distributed
+builders, remote executors, or otherwise—can consume without modification.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Importantly, the IR contains **no Ninja-isms**. Placeholders such as `$in` | |
| and `$out` are resolved to plain lists of file paths, and command strings are | |
| expanded before hashing. This deliberate absence of Ninja-specific syntax makes | |
| the IR a stable contract that future back-ends—distributed builders, remote | |
| executors, or otherwise—can consume without modification. | |
| Importantly, the IR contains **no Ninja-isms**. Placeholders such as `$in` | |
| and `$out` are resolved to plain lists of file paths, and command strings | |
| are expanded before hashing. This deliberate absence of Ninja-specific | |
| syntax makes the IR a stable contract that future back-ends—distributed | |
| builders, remote executors, or otherwise—can consume without modification. |
🤖 Prompt for AI Agents
In docs/netsuke-design.md around lines 748 to 752, the paragraph containing "no
Ninja-isms" exceeds 80 columns. Reformat this paragraph by inserting line breaks
to hard-wrap the text at or before 80 characters per line, ensuring readability
and adherence to style guidelines.
| - `command`: A single command string to be executed. It may include the | ||
| placeholders `{{ ins }}` and `{{ outs }}` to represent input and output files. | ||
| Netsuke expands these placeholders to space-separated, shell-escaped lists | ||
| of file paths before hashing the action. When generating the Ninja rule, the | ||
| lists are replaced with Ninja's `$in` and `$out` macros. After interpolation | ||
| the command must be parsable by [shlex](https://docs.rs/shlex/latest/shlex/). | ||
| Any interpolation other than `ins` or `outs` is automatically shell-escaped. | ||
|
|
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Enforce 80-column wrap for command bullet
The bullet describing command still breaches the 80-column policy. Re-flow it.
- - `command`: A single command string to be executed. It may include the
- placeholders `{{ ins }}` and `{{ outs }}` to represent input and output files.
- Netsuke expands these placeholders to space-separated, shell-escaped lists
- of file paths before hashing the action. When generating the Ninja rule, the
- lists are replaced with Ninja's `$in` and `$out` macros. After interpolation
- the command must be parsable by [shlex](https://docs.rs/shlex/latest/shlex/).
- Any interpolation other than `ins` or `outs` is automatically shell-escaped.
+ - `command`: A single command string to be executed. It may include the
+ placeholders `{{ ins }}` and `{{ outs }}` to represent input and output
+ files. Netsuke expands these placeholders to space-separated,
+ shell-escaped lists of file paths before hashing the action. When
+ generating the Ninja rule, the lists are replaced with Ninja's `$in` and
+ `$out` macros. After interpolation, the command must be parsable by
+ [shlex](https://docs.rs/shlex/latest/shlex/). All other interpolations are
+ shell-escaped automatically.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - `command`: A single command string to be executed. It may include the | |
| placeholders `{{ ins }}` and `{{ outs }}` to represent input and output files. | |
| Netsuke expands these placeholders to space-separated, shell-escaped lists | |
| of file paths before hashing the action. When generating the Ninja rule, the | |
| lists are replaced with Ninja's `$in` and `$out` macros. After interpolation | |
| the command must be parsable by [shlex](https://docs.rs/shlex/latest/shlex/). | |
| Any interpolation other than `ins` or `outs` is automatically shell-escaped. | |
| - `command`: A single command string to be executed. It may include the | |
| placeholders `{{ ins }}` and `{{ outs }}` to represent input and output | |
| files. Netsuke expands these placeholders to space-separated, | |
| shell-escaped lists of file paths before hashing the action. When | |
| generating the Ninja rule, the lists are replaced with Ninja's `$in` and | |
| `$out` macros. After interpolation, the command must be parsable by | |
| [shlex](https://docs.rs/shlex/latest/shlex/). All other interpolations are | |
| shell-escaped automatically. |
🧰 Tools
🪛 LanguageTool
[uncategorized] ~176-~176: Loose punctuation mark.
Context: ...ng identifier for the rule. - command: A single command string to be executed....
(UNLIKELY_OPENING_PUNCTUATION)
[style] ~182-~182: Consider using a more formal/concise alternative here.
Context: ...lex/latest/shlex/). Any interpolation other than ins or outs is automatically shell-...
(OTHER_THAN)
🤖 Prompt for AI Agents
In docs/netsuke-design.md around lines 176 to 183, the bullet describing the
`command` field exceeds the 80-column line length limit. Reformat the text by
breaking long lines into shorter ones, ensuring each line does not exceed 80
characters while maintaining the original meaning and clarity.
|
@coderabbitai Please create a class diagram for the item types in a |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chainI'll help you create a class diagram for the item types in a Based on my exploration of the codebase, here's a comprehensive class diagram for the item types in a classDiagram
class NetsukeManifest {
+String netsuke_version
+HashMap<String, Value> vars
+HashMap<String, Target> targets
+HashMap<String, Rule> rules
}
class Target {
+Vec<String> ins
+Vec<String> outs
+Option<Vec<String>> deps
+Recipe recipe
+Option<HashMap<String, Value>> vars
+Option<String> depfile
+Option<String> pool
+Option<String> description
}
class Recipe <<enumeration>> {
+Rule(String)
+Command(String)
+Script(Script)
}
class Rule {
+String command
+Option<String> description
+Option<String> depfile
+Option<String> pool
+Option<String> generator
+Option<String> restat
+Option<HashMap<String, Value>> vars
}
class Script {
+String content
+Option<String> interpreter
+Option<HashMap<String, Value>> vars
}
class Value <<serde_yaml::Value>> {
+Null
+Bool(bool)
+Number(Number)
+String(String)
+Sequence(Sequence)
+Mapping(Mapping)
+Tagged(Tagged)
}
NetsukeManifest ||--o{ Target : contains
NetsukeManifest ||--o{ Rule : contains
NetsukeManifest ||--o{ Value : vars
Target ||--|| Recipe : has
Recipe ||--o| Script : contains
Target ||--o{ Value : vars
Rule ||--o{ Value : vars
Script ||--o{ Value : vars
note for Recipe "Mutually exclusive variants:\n- Rule: references a rule by name\n- Command: inline command string\n- Script: embedded script with interpreter"
note for NetsukeManifest "Top-level manifest structure\nValidates netsuke_version compatibility"
note for Target "Build target with inputs/outputs\nRecipe field determines how to build"
note for Script "Supports shebang detection\nDefault interpreter: /bin/sh -e\nWindows default: powershell -Command"
@leynos, this class diagram shows the main item types that can be defined in a
The key relationships are:
🏁 Script executed: Length of output: 366 🏁 Script executed: Length of output: 365 🏁 Script executed: Length of output: 325 🏁 Script executed: Length of output: 2448 🏁 Script executed: Length of output: 298 Let me get the complete structure definitions from the documentation: Based on the complete structure definitions I found in the documentation, here's a comprehensive class diagram for the item types in a classDiagram
class NetsukeManifest {
+String netsuke_version
+HashMap~String, Value~ vars
+Vec~Rule~ rules
+Vec~Target~ actions
+Vec~Target~ targets
+Vec~String~ defaults
}
class Target {
+StringOrList name
+Recipe recipe
+StringOrList sources
+StringOrList deps
+HashMap~String, Value~ vars
+Option~String~ depfile
+Option~String~ pool
+Option~String~ description
}
class Recipe {
<<enumeration>>
+Command~command: String~
+Script~script: String~
+Rule~rule: StringOrList~
}
class Rule {
+String name
+String command
+Option~String~ description
+Option~String~ depfile
+Option~String~ pool
+Option~String~ generator
+Option~String~ restat
+HashMap~String, Value~ vars
}
class StringOrList {
<<union type>>
+String
+Vec~String~
}
class Value {
<<serde_yaml::Value>>
+Null
+Bool~bool~
+Number~Number~
+String~String~
+Sequence~Sequence~
+Mapping~Mapping~
+Tagged~Tagged~
}
%% Relationships
NetsukeManifest ||--o{ Target : "contains actions/targets"
NetsukeManifest ||--o{ Rule : "contains rules"
NetsukeManifest ||--o{ Value : "global vars"
NetsukeManifest ||--o{ String : "defaults"
Target ||--|| Recipe : "has recipe"
Target ||--|| StringOrList : "name, sources, deps"
Target ||--o{ Value : "local vars"
Recipe ||--o| String : "command/script content"
Recipe ||--o| StringOrList : "rule reference"
Rule ||--|| String : "name, command"
Rule ||--o{ Value : "local vars"
Rule ||--o| String : "optional fields"
%% Constraints and notes
note for Recipe "Mutually exclusive variants enforce RecipeConflict:<br/>- Command: inline command string<br/>- Script: embedded script content<br/>- Rule: reference to named rule"
note for NetsukeManifest "Root manifest structure<br/>- netsuke_version for compatibility<br/>- actions: deprecated alias for targets<br/>- defaults: default target names"
note for Target "Build targets with I/O specification<br/>- name: output file(s)<br/>- sources: input file(s)<br/>- deps: order-only dependencies"
note for StringOrList "Flexible string specification<br/>Single string or list of strings<br/>Used for file paths and references"
note for Rule "Reusable command templates<br/>Referenced by name from Recipe::Rule<br/>Support Jinja2 templating with vars"
@leynos, this class diagram shows all the item types that can be defined in a Core Types:
Supporting Types:
Key Design Features:
This structure supports the deterministic build graph generation and IR decoupling documented in your PR! 🏁 Script executed: Length of output: 5878 🏁 Script executed: Length of output: 1541 Perfect! Now I have the complete structure definitions. Here's the accurate class diagram for the item types in a classDiagram
class NetsukeManifest {
+String netsuke_version
+HashMap~String, Value~ vars
+Vec~Rule~ rules
+Vec~Target~ actions
+Vec~Target~ targets
+Vec~String~ defaults
}
class Target {
+StringOrList name
+Recipe recipe
+StringOrList sources
+StringOrList deps
+StringOrList order_only_deps
+HashMap~String, String~ vars
+bool phony
+bool always
}
class Rule {
+String name
+Recipe recipe
+Option~String~ description
+Option~String~ deps
}
class Recipe {
<<enumeration>>
+Command~command: String~
+Script~script: String~
+Rule~rule: StringOrList~
}
class StringOrList {
<<enumeration>>
+Empty
+String~String~
+List~Vec String~
}
class Value {
<<serde_yaml::Value>>
+Null
+Bool~bool~
+Number~Number~
+String~String~
+Sequence~Sequence~
+Mapping~Mapping~
+Tagged~Tagged~
}
%% Relationships
NetsukeManifest ||--o{ Rule : "contains rules"
NetsukeManifest ||--o{ Target : "actions (deprecated)"
NetsukeManifest ||--o{ Target : "targets"
NetsukeManifest ||--o{ String : "defaults"
NetsukeManifest ||--o{ Value : "global vars"
Target ||--|| Recipe : "has recipe"
Target ||--|| StringOrList : "name, sources, deps, order_only_deps"
Target ||--o{ String : "local vars"
Rule ||--|| Recipe : "has recipe"
Rule ||--|| String : "name"
Rule ||--o| String : "description, deps"
Recipe ||--o| String : "command/script content"
Recipe ||--o| StringOrList : "rule reference"
%% Styling and annotations
note for Recipe "Tagged enum with 'kind' field:<br/>- Command: inline command string<br/>- Script: embedded script content<br/>- Rule: reference to named rule<br/><br/>Mutually exclusive - enforces RecipeConflict"
note for NetsukeManifest "Root manifest structure:<br/>- netsuke_version: compatibility check<br/>- vars: global YAML values<br/>- actions: deprecated, use targets<br/>- defaults: default target names"
note for Target "Build target specification:<br/>- name: output file(s)<br/>- sources: input file(s)<br/>- deps: implicit dependencies<br/>- order_only_deps: order-only deps<br/>- phony: always run if requested<br/>- always: run every invocation"
note for StringOrList "Flexible string specification:<br/>- Empty: no values<br/>- String: single value<br/>- List: multiple values<br/><br/>Untagged enum for user convenience"
note for Rule "Reusable command template:<br/>- name: unique identifier<br/>- recipe: how to execute<br/>- description: optional docs<br/>- deps: dependency specification"
@leynos, this comprehensive class diagram shows all the item types that can be defined in a Key Features Highlighted:
Notable Design Patterns:
This structure perfectly supports the deterministic build graph generation, IR decoupling, and manifest validation features documented in your PR! |
|
@coderabbitai the diagram is presenting the following error message: Please fix this |
|
Tip For best results, initiate chat on the files or code changes. @leynos, I see the issue! The Mermaid syntax I used has compatibility problems with GitHub's renderer. Let me fix this with a cleaner, more compatible version: classDiagram
class NetsukeManifest {
+String netsuke_version
+HashMap vars
+Vec rules
+Vec actions
+Vec targets
+Vec defaults
}
class Target {
+StringOrList name
+Recipe recipe
+StringOrList sources
+StringOrList deps
+StringOrList order_only_deps
+HashMap vars
+bool phony
+bool always
}
class Rule {
+String name
+Recipe recipe
+String description
+String deps
}
class Recipe {
<<enumeration>>
Command
Script
Rule
}
class StringOrList {
<<enumeration>>
Empty
String
List
}
class Value {
<<serde_yaml>>
Null
Bool
Number
String
Sequence
Mapping
Tagged
}
NetsukeManifest "1" o-- "*" Target
NetsukeManifest "1" o-- "*" Rule
NetsukeManifest "1" o-- "*" Value
Target "1" -- "1" Recipe
Target "1" -- "1" StringOrList
Rule "1" -- "1" Recipe
This simplified version removes the problematic syntax elements:
The diagram now shows the core Netsukefile item types and their relationships in a GitHub-compatible format! |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (5)
docs/netsuke-design.md (5)
650-652: Wrap glob determinism sentenceLine length still exceeds policy width.
- The results are returned sorted lexicographically and symlinks are followed to - keep builds deterministic. This function bridges a key feature gap, as Ninja - itself does not support globbing.[^3] + The results are returned sorted lexicographically, and symlinks are followed + to keep builds deterministic. + This function bridges a key feature gap, as Ninja itself does not support + globbing.[^3]
772-774: Wrap impurity noteRe-flow to satisfy MD013.
-Using `shell()` marks the template as *impure* and disables caching of the -rendered YAML between Stage 2 and Stage 3. This avoids accidental -reuse of results that depend on external commands. +Using `shell()` marks the template as *impure* and disables caching of the +rendered YAML between Stage 2 and Stage 3. +This avoids accidental reuse of results that depend on external commands.
748-750: Wrap filter-naming paragraphStill > 80 columns.
-All built-in filters use `snake_case`. The `camel_case` helper is provided in -place of `camelCase` so naming remains consistent with `snake_case` and `kebab- -case`. +All built-in filters use `snake_case`. +The `camel_case` helper is provided instead of `camelCase` so that naming +remains consistent with `snake_case` and `kebab-case`.
80-83: Wrap determinism paragraph to ≤ 80 columnsHard-wrap the four lines below so none exceed the 80-column limit.
- Netsuke's pipeline is **deterministic**. Given the same `Netsukefile` and - environment variables, the generated `build.ninja` will be byte-for-byte - identical. This property is essential for reproducible builds and makes the - output suitable for caching or source control. + Netsuke's pipeline is **deterministic**. + Given the same `Netsukefile` and environment variables, the generated + `build.ninja` will be byte-for-byte identical. + This property is essential for reproducible builds and makes the output + suitable for caching or source control.
1260-1264: Wrap graph sub-command explanationRe-flow to respect 80-column limit and keep the paragraph readable.
- run the Netsuke pipeline up to Stage 4 (IR Generation) and then invoke - Ninja with the graph tool, `ninja -t graph`. This outputs the complete build - dependency graph in the DOT language. The result can be piped through `dot - -Tsvg` or displayed via `netsuke graph --html` using an embedded Dagre.js - viewer. Visualising the graph is invaluable for understanding and debugging + Run the Netsuke pipeline up to Stage 4 (IR Generation) and then invoke + Ninja with the graph tool, `ninja -t graph`. + This outputs the complete build dependency graph in the DOT language. + Pipe the result through `dot -Tsvg`, or display it via + `netsuke graph --html` using the embedded Dagre.js viewer. + Visualising the graph is invaluable for understanding and debugging + complex projects.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
docs/netsuke-design.md(16 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.md
Instructions used from:
Sources:
⚙️ CodeRabbit Configuration File
🧠 Learnings (1)
docs/netsuke-design.md (2)
Learnt from: CR
PR: leynos/netsuke#0
File: docs/roadmap.md:0-0
Timestamp: 2025-07-12T13:36:28.501Z
Learning: Applies to docs/src/ninja_gen.rs : Mandate its use for all variable substitutions within the `command` strings during Ninja file synthesis to prevent command injection vulnerabilities.
Learnt from: CR
PR: leynos/netsuke#0
File: docs/netsuke-design.md:0-0
Timestamp: 2025-07-12T13:36:08.546Z
Learning: All third-party crate selections (clap, serde_yaml, minijinja, shell-quote, anyhow, thiserror, semver) must be justified and used as specified for their respective components.
🪛 LanguageTool
docs/netsuke-design.md
[style] ~185-~185: Would you like to use the Oxford spelling “summarizes”? The spelling ‘summarises’ is also correct.
Context: ...statement.[^3] The class diagram below summarises the structure of a Netsukefile and th...
(OXFORD_SPELLING_Z_NOT_S)
[uncategorized] ~248-~248: Loose punctuation mark.
Context: ...hat defines a reusable action. - name: A unique string identifier for the rule...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~250-~250: Loose punctuation mark.
Context: ...ng identifier for the rule. - command: A single command string to be executed....
(UNLIKELY_OPENING_PUNCTUATION)
[style] ~256-~256: Consider using a more formal/concise alternative here.
Context: ...lex/latest/shlex/). Any interpolation other than ins or outs is automatically shell-...
(OTHER_THAN)
[uncategorized] ~258-~258: Loose punctuation mark.
Context: ...automatically shell-escaped. - script: A multi-line script declared with the Y...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~265-~265: Possible missing comma found.
Context: ...ping rules. On Windows, scripts default to powershell -Command unless the mani...
(AI_HYDRA_LEO_MISSING_COMMA)
[style] ~1263-~1263: Would you like to use the Oxford spelling “Visualizing”? The spelling ‘Visualising’ is also correct.
Context: ...l` using an embedded Dagre.js viewer. Visualising the graph is invaluable for understandi...
(OXFORD_SPELLING_Z_NOT_S)
🔇 Additional comments (2)
docs/netsuke-design.md (2)
937-939: Wrap rule-writing paragraphLines exceed 80 columns.
- `ir::Action`, write a corresponding Ninja `rule` statement. The input and - output lists stored in the action replace the `ins` and `outs` placeholders. - These lists are then rewritten as Ninja's `$in` and `$out`. + `ir::Action`, write a corresponding Ninja `rule` statement. + The input and output lists stored in the action replace the `ins` and + `outs` placeholders. These lists are then rewritten as Ninja's `$in` and + `$out`.Likely an incorrect or invalid review comment.
271-273: Re-wrap internal deserialisation noteKeep every line within 80 columns.
- Internally, these options deserialize into a shared `Recipe` enum tagged with - a `kind` field. Serde aliases ensure manifests that omit the tag continue to - load correctly. + Internally, these options deserialize into a shared `Recipe` enum tagged + with a `kind` field. Serde aliases ensure manifests that omit the tag still + load correctly.Likely an incorrect or invalid review comment.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
1 participant
Summary
shell()filterinsandoutsplaceholders are expanded and escapedTesting
make fmtmake lintmake testmake markdownlintmake nixiehttps://chatgpt.com/codex/tasks/task_e_6873c2f78a9c8322be2c9beebaf8c2bb
Summary by Sourcery
Update the design document to address various design feedback and clarify core behaviors
Documentation: