Refactor CLI build handling into helper functions#82
Conversation
Reviewer's GuideRefactored run() to delegate build orchestration to dedicated helpers: handle_build, create_temp_ninja_file, write_ninja_file, and resolve_manifest_path, streamlining CLI dispatch and consolidating common logic. Sequence diagram for refactored build command handling in CLIsequenceDiagram
participant CLI
participant runner
participant handle_build
participant generate_ninja
participant BuildTargets
participant write_ninja_file
participant create_temp_ninja_file
participant run_ninja
CLI->>runner: run(cli)
runner->>handle_build: handle_build(cli, args)
handle_build->>generate_ninja: generate_ninja(cli)
handle_build->>BuildTargets: BuildTargets::new(args.targets)
alt args.emit is Some
handle_build->>write_ninja_file: write_ninja_file(path, ninja)
handle_build->>run_ninja: run_ninja("ninja", cli, path, targets)
else args.emit is None
handle_build->>create_temp_ninja_file: create_temp_ninja_file(ninja)
handle_build->>run_ninja: run_ninja("ninja", cli, tmp.path(), targets)
end
Class diagram for new and refactored helper functions in runner.rsclassDiagram
class Cli {
+file: PathBuf
+directory: Option<PathBuf>
+jobs: Option<u32>
+verbose: bool
+command: Option<Commands>
}
class BuildArgs {
+emit: Option<PathBuf>
+targets: Vec<String>
}
class NinjaContent {
+new(content: String)
+as_str(): &str
}
class NamedTempFile {
+path(): &Path
}
class BuildTargets {
+new(targets: Vec<String>)
}
class runner {
+run(cli: &Cli): Result<()>
+handle_build(cli: &Cli, args: &BuildArgs): Result<()>
+create_temp_ninja_file(content: &NinjaContent): Result<NamedTempFile>
+write_ninja_file(path: &Path, content: &NinjaContent): Result<()>
+resolve_manifest_path(cli: &Cli): PathBuf
+generate_ninja(cli: &Cli): Result<NinjaContent>
}
Cli --> runner: uses
BuildArgs --> runner: uses
NinjaContent --> runner: uses
NamedTempFile --> runner: uses
BuildTargets --> runner: uses
Class diagram for refactored write_ninja_file and manifest path resolutionclassDiagram
class runner {
+write_ninja_file(path: &Path, content: &NinjaContent): Result<()>
+resolve_manifest_path(cli: &Cli): PathBuf
}
class Cli {
+file: PathBuf
+directory: Option<PathBuf>
}
runner <.. Cli: uses
File-Level Changes
Assessment against linked issues
Possibly linked issues
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. Summary by CodeRabbit
WalkthroughRefactor the Changes
Sequence Diagram(s)sequenceDiagram
participant CLI as CLI
participant Runner as run()
participant Helper as handle_build()
participant Manifest as resolve_manifest_path()
participant Writer as write_ninja_file()
participant Temp as create_temp_ninja_file()
CLI->>Runner: Invoke run()
Runner->>Helper: handle_build(cli, args)
Helper->>Manifest: resolve_manifest_path(cli)
Manifest-->>Helper: manifest_path
Helper->>Writer: write_ninja_file(path, content)
alt Temp file needed
Helper->>Temp: create_temp_ninja_file(content)
Temp-->>Helper: temp_file
end
Helper-->>Runner: Result
Runner-->>CLI: Result
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~15 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changesNo out-of-scope changes detected. 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. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![codescene-delta-analysis[bot]](https://avatars.githubusercontent.com/in/53921?s=60&v=4){kind=small}
![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 found some issues that need to be addressed.
Blocking issues:
- Demonstrate any bug fixes in the Build command refactor with a test. (link)
General comments:
- Consider extracting the hardcoded "ninja" executable path into a constant or CLI‐configurable parameter to avoid duplication and make it easier to change later.
- handle_build currently clones the target vector; you might refactor BuildTargets::new to accept an iterator or a slice to avoid unnecessary allocation.
- runner.rs is accumulating helper functions—consider moving handle_build, create_temp_ninja_file, write_ninja_file, and resolve_manifest_path into their own module to keep the file focused.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider extracting the hardcoded "ninja" executable path into a constant or CLI‐configurable parameter to avoid duplication and make it easier to change later.
- handle_build currently clones the target vector; you might refactor BuildTargets::new to accept an iterator or a slice to avoid unnecessary allocation.
- runner.rs is accumulating helper functions—consider moving handle_build, create_temp_ninja_file, write_ninja_file, and resolve_manifest_path into their own module to keep the file focused.
## Individual Comments
### Comment 1
<location> `src/runner.rs:77` </location>
<code_context>
- }
- Ok(())
- }
+ Commands::Build(args) => handle_build(cli, &args),
Commands::Manifest { file } => {
let ninja = generate_ninja(cli)?;
</code_context>
<issue_to_address>
Demonstrate any bug fixes in the Build command refactor with a test.
If this refactor addresses any bugs, add a test that demonstrates the fix and prevents regression.
</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: 3
🔭 Outside diff range comments (1)
src/runner.rs (1)
153-160: Ensure parent directories exist before writing the Ninja fileWriting to an
--emitpath under a non-existent directory currently fails. Create parents first to improve UX.fn write_ninja_file(path: &Path, content: &NinjaContent) -> Result<()> { - fs::write(path, content.as_str()) + if let Some(parent) = path.parent() { + fs::create_dir_all(parent) + .with_context(|| format!("failed to create parent directory {}", parent.display()))?; + } + fs::write(path, content.as_str()) .with_context(|| format!("failed to write Ninja file to {}", path.display()))?; info!("Generated Ninja file at {}", path.display()); Ok(()) }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
src/runner.rs(6 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.rs
📄 CodeRabbit Inference Engine (AGENTS.md)
**/*.rs: Clippy warnings MUST be disallowed.
Fix any warnings emitted during tests in the code itself rather than silencing them.
Where a function is too long, extract meaningfully named helper functions adhering to separation of concerns and CQRS.
Where a function has too many parameters, group related parameters in meaningfully named structs.
Where a function is returning a large error consider usingArcto reduce the amount of data returned.
Write unit and behavioural tests for new functionality. Run both before and after making any change.
Every module must begin with a module level (//!) comment explaining the module's purpose and utility.
Document public APIs using Rustdoc comments (///) so documentation can be generated with cargo doc.
Prefer immutable data and avoid unnecessarymutbindings.
Handle errors with theResulttype instead of panicking where feasible.
Avoidunsafecode unless absolutely necessary and document any usage clearly.
Place function attributes after doc comments.
Do not usereturnin single-line functions.
Use predicate functions for conditional criteria with more than two branches.
Lints must not be silenced except as a last resort.
Lint rule suppressions must be tightly scoped and include a clear reason.
Preferexpectoverallow.
Prefer.expect()over.unwrap().
Useconcat!()to combine long string literals rather than escaping newlines with a backslash.
Prefer semantic error enums: Derivestd::error::Error(via thethiserrorcrate) for any condition the caller might inspect, retry, or map to an HTTP status.
Use an opaque error only at the app boundary: Useeyre::Reportfor human-readable logs; these should not be exposed in public APIs.
Never export the opaque type from a library: Convert to domain enums at API boundaries, and toeyreonly in the mainmain()entrypoint or top-level async task.
Files:
src/runner.rs
⚙️ CodeRabbit Configuration File
**/*.rs: * Seek to keep the cyclomatic complexity of functions no more than 12.
Adhere to single responsibility and CQRS
Place function attributes after doc comments.
Do not use
returnin single-line functions.Move conditionals with >2 branches into a predicate function.
Avoid
unsafeunless absolutely necessary.Every module must begin with a
//!doc comment that explains the module's purpose and utility.Comments and docs must follow en-GB-oxendict (-ize / -our) spelling and grammar
Lints must not be silenced except as a last resort.
#[allow]is forbidden.- Only narrowly scoped
#[expect(lint, reason = "...")]is allowed.- No lint groups, no blanket or file-wide suppression.
- Include
FIXME:with link if a fix is expected.Use
rstestfixtures for shared setup and to avoid repetition between tests.Replace duplicated tests with
#[rstest(...)]parameterised cases.Prefer
mockallfor mocks/stubs.Prefer
.expect()over.unwrap()Ensure that any API or behavioural changes are reflected in the documentation in
docs/Ensure that any completed roadmap steps are recorded in the appropriate roadmap in
docs/Files must not exceed 400 lines in length
- Large modules must be decomposed
- Long match statements or dispatch tables should be decomposed by domain and collocated with targets
- Large blocks of inline data (e.g., test fixtures, constants or templates) must be moved to external files and inlined at compile-time or loaded at run-time.
Files:
src/runner.rs
🧬 Code Graph Analysis (1)
src/runner.rs (1)
tests/steps/cli_steps.rs (1)
manifest_path(104-107)
🔍 MCP Research (1 server)
Deepwiki:
-
The
BuildArgsstruct introduced in a related PR (#66) is used in the refactoredhandle_buildfunction to represent build command arguments, indicating coordination between PRs for modular CLI command handling. (Related PRs) -
The temporary file creation and writing logic for the Ninja build file is moved into a dedicated helper function
create_temp_ninja_filethat returns aNamedTempFile, improving modularity and testability. (PR #82 Summary) -
The function
write_and_logwas renamed towrite_ninja_fileand updated to unify file writing with logging, used in both theCommands::Manifestbranch and the new helper functions. (PR #82 Summary)
⏰ 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 (5)
src/runner.rs (5)
16-16: Importtempfilefor scoped tmpfile handling — LGTMThe import enables
NamedTempFileandBuilderuse for safe, scoped temp files.
77-77: Delegate build handling tohandle_buildto keeprunshallow — LGTMThis aligns with the refactor objective and reduces
runcomplexity.
122-143: Encapsulate tmp Ninja file creation — LGTMFunction cleanly scopes the tmpfile and reuses the unified writer. Context messages are clear.
183-183: Useresolve_manifest_pathfor path resolution — LGTMThis removes inline path-joining logic and centralises manifest resolution.
80-81: No stale references remain
All instances of the oldwrite_and_logandcreate_tempfilehelpers have been removed. The only match for.tempfile()at src/runner.rs:138 is the expected call ontempfile::Builderwithin the newcreate_temp_ninja_filehelper.
- Refine your grep to only match legacy identifiers:
rg -n "write_and_log|create_tempfile" -S
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 7
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (6)
AGENTS.md(3 hunks)docs/srgn.md(3 hunks)src/runner.rs(8 hunks)tests/runner_tests.rs(3 hunks)tests/steps/process_steps.rs(1 hunks)tests/support/mod.rs(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.md
📄 CodeRabbit Inference Engine (AGENTS.md)
**/*.md: Documentation must use en-GB-oxendict spelling and grammar, except for the naming of the "LICENSE" file.
Validate Markdown files usingmake markdownlint.
Runmake fmtafter any documentation changes to format all Markdown files and fix table markup.
Validate Mermaid diagrams in Markdown files by runningmake nixie.
Markdown paragraphs and bullet points must be wrapped at 80 columns.
Code blocks in Markdown must be wrapped at 120 columns.
Tables and headings in Markdown must not be wrapped.
Use dashes (-) for list bullets in Markdown.
Use GitHub-flavoured Markdown footnotes ([^1]) for references and footnotes.
Files:
AGENTS.mddocs/srgn.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:
AGENTS.mddocs/srgn.md
**/*.rs
📄 CodeRabbit Inference Engine (AGENTS.md)
**/*.rs: Clippy warnings MUST be disallowed.
Fix any warnings emitted during tests in the code itself rather than silencing them.
Where a function is too long, extract meaningfully named helper functions adhering to separation of concerns and CQRS.
Where a function has too many parameters, group related parameters in meaningfully named structs.
Where a function is returning a large error consider usingArcto reduce the amount of data returned.
Write unit and behavioural tests for new functionality. Run both before and after making any change.
Every module must begin with a module level (//!) comment explaining the module's purpose and utility.
Document public APIs using Rustdoc comments (///) so documentation can be generated with cargo doc.
Prefer immutable data and avoid unnecessarymutbindings.
Handle errors with theResulttype instead of panicking where feasible.
Avoidunsafecode unless absolutely necessary and document any usage clearly.
Place function attributes after doc comments.
Do not usereturnin single-line functions.
Use predicate functions for conditional criteria with more than two branches.
Lints must not be silenced except as a last resort.
Lint rule suppressions must be tightly scoped and include a clear reason.
Preferexpectoverallow.
Prefer.expect()over.unwrap().
Useconcat!()to combine long string literals rather than escaping newlines with a backslash.
Prefer semantic error enums: Derivestd::error::Error(via thethiserrorcrate) for any condition the caller might inspect, retry, or map to an HTTP status.
Use an opaque error only at the app boundary: Useeyre::Reportfor human-readable logs; these should not be exposed in public APIs.
Never export the opaque type from a library: Convert to domain enums at API boundaries, and toeyreonly in the mainmain()entrypoint or top-level async task.
Files:
tests/steps/process_steps.rstests/runner_tests.rstests/support/mod.rssrc/runner.rs
⚙️ CodeRabbit Configuration File
**/*.rs: * Seek to keep the cyclomatic complexity of functions no more than 12.
Adhere to single responsibility and CQRS
Place function attributes after doc comments.
Do not use
returnin single-line functions.Move conditionals with >2 branches into a predicate function.
Avoid
unsafeunless absolutely necessary.Every module must begin with a
//!doc comment that explains the module's purpose and utility.Comments and docs must follow en-GB-oxendict (-ize / -our) spelling and grammar
Lints must not be silenced except as a last resort.
#[allow]is forbidden.- Only narrowly scoped
#[expect(lint, reason = "...")]is allowed.- No lint groups, no blanket or file-wide suppression.
- Include
FIXME:with link if a fix is expected.Use
rstestfixtures for shared setup and to avoid repetition between tests.Replace duplicated tests with
#[rstest(...)]parameterised cases.Prefer
mockallfor mocks/stubs.Prefer
.expect()over.unwrap()Ensure that any API or behavioural changes are reflected in the documentation in
docs/Ensure that any completed roadmap steps are recorded in the appropriate roadmap in
docs/Files must not exceed 400 lines in length
- Large modules must be decomposed
- Long match statements or dispatch tables should be decomposed by domain and collocated with targets
- Large blocks of inline data (e.g., test fixtures, constants or templates) must be moved to external files and inlined at compile-time or loaded at run-time.
Files:
tests/steps/process_steps.rstests/runner_tests.rstests/support/mod.rssrc/runner.rs
docs/**/*.md
📄 CodeRabbit Inference Engine (AGENTS.md)
docs/**/*.md: Use the markdown files within thedocs/directory as a knowledge base and source of truth for project requirements, dependency choices, and architectural decisions.
Proactively update the relevant file(s) in thedocs/directory to reflect the latest state when new decisions are made, requirements change, libraries are added/removed, or architectural patterns evolve.
Files:
docs/srgn.md
🧬 Code Graph Analysis (2)
tests/steps/process_steps.rs (1)
src/runner.rs (3)
new(26-28)new(43-45)new(56-58)
src/runner.rs (1)
tests/steps/cli_steps.rs (1)
manifest_path(104-107)
🪛 LanguageTool
AGENTS.md
[typographical] ~208-~208: Consider using an em dash in dialogues and enumerations.
Context: ...g mbake validate Makefile. - strace – Traces system calls and signals made by...
(DASH_RULE)
[typographical] ~210-~210: Consider using an em dash in dialogues and enumerations.
Context: ...runtime behaviour and syscalls. - gdb – The GNU Debugger, for inspecting and co...
(DASH_RULE)
[typographical] ~212-~212: Consider using an em dash in dialogues and enumerations.
Context: ...ost-mortem via core dumps). - ripgrep – Fast, recursive text search tool (`grep...
(DASH_RULE)
[typographical] ~214-~214: Consider using an em dash in dialogues and enumerations.
Context: ...respects .gitignore files. - ltrace – Traces calls to dynamic library functio...
(DASH_RULE)
[typographical] ~215-~215: Consider using an em dash in dialogues and enumerations.
Context: ...nctions made by a process. - valgrind – Suite for detecting memory leaks, profi...
(DASH_RULE)
[typographical] ~217-~217: Consider using an em dash in dialogues and enumerations.
Context: ... low-level memory errors. - bpftrace – High-level tracing tool for eBPF, using...
(DASH_RULE)
[typographical] ~227-~227: Consider using an em dash in dialogues and enumerations.
Context: ...k traffic at the packet level. - nmap – Network scanner for host discovery, por...
(DASH_RULE)
[typographical] ~229-~229: Consider using an em dash in dialogues and enumerations.
Context: ... and service identification. - lldb – LLVM debugger, alternative to gdb. - ...
(DASH_RULE)
[typographical] ~230-~230: Consider using an em dash in dialogues and enumerations.
Context: ...debugger, alternative to gdb. - eza – Modern ls replacement with more featu...
(DASH_RULE)
[typographical] ~231-~231: Consider using an em dash in dialogues and enumerations.
Context: ...e features and better defaults. - fzf – Interactive fuzzy finder for selecting ...
(DASH_RULE)
[typographical] ~233-~233: Consider using an em dash in dialogues and enumerations.
Context: ...with statistical output. - shellcheck – Linter for shell scripts, identifying e...
(DASH_RULE)
[typographical] ~234-~234: Consider using an em dash in dialogues and enumerations.
Context: ...ifying errors and bad practices. - fd – Fast, user-friendly find alternative ...
(DASH_RULE)
[typographical] ~235-~235: Consider using an em dash in dialogues and enumerations.
Context: ...e with sensible defaults. - checkmake – Linter for Makefiles, ensuring they f...
(DASH_RULE)
[typographical] ~237-~237: Consider using an em dash in dialogues and enumerations.
Context: ...t practices and conventions. - srgn – [Structural grep](https://github.com/al...
(DASH_RULE)
docs/srgn.md
[uncategorized] ~19-~19: Loose punctuation mark.
Context: ... regex-based pattern matching of grep, the stream-editing capabilities of tr...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~24-~24: Possible missing comma found.
Context: ...or full-featured IDE refactoring engines but a specialized scalpel for tasks that ar...
(AI_HYDRA_LEO_MISSING_COMMA)
[typographical] ~56-~56: The word ‘where’ starts a question. Add a question mark (“?”) at the end of the sentence.
Context: ...ld be done to the text within that scope. This separation of concerns is central ...
(WRB_QUESTION_MARK)
[grammar] ~60-~60: There is an agreement error between ‘know’ and ‘regex’. Insert ‘a(n)’ or change the noun to plural.
Context: ...states its design goal clearly: "if you know regex and the basics of the language you are ...
(PRP_VB_NN)
[grammar] ~61-~61: Please add a punctuation mark at the end of paragraph.
Context: ..., you are good to go".2 This philosophy distinguishes srgn from other advanced code-queryi...
(PUNCTUATION_PARAGRAPH_END)
[style] ~65-~65: Consider using the typographical ellipsis character here instead.
Context: ...with metavariables ($X) and ellipses (...) to find code that matches an abstract...
(ELLIPSIS)
[typographical] ~80-~80: To join two clauses or introduce examples, consider using an em dash.
Context: ...ommand line. ## Part 2: Getting Started - Installation and First Cuts ### 2.1 Ins...
(DASH_RULE)
[style] ~84-~84: To form a complete sentence, be sure to include a subject.
Context: ...Preparing the Operating Theater srgn can be installed across various platforms, ...
(MISSING_IT_THERE)
[uncategorized] ~92-~92: Loose punctuation mark.
Context: ...tHub Releases page.1 - cargo-binstall: For users with the Rust toolchain, this...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~93-~93: Possible missing comma found.
Context: ...ignificantly faster than compiling from source as it downloads prebuilt binaries whe...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~108-~108: Loose punctuation mark.
Context: ...install srgn ``` - cargo install: The traditional method of compiling fro...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~153-~153: Loose punctuation mark.
Context: ...es (e.g., --python, --rust). - '': This is the mandatory, positional regul...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~161-~161: Loose punctuation mark.
Context: ...n extensions and shebangs.1 - -- '': This is the optional replacement string...
(UNLIKELY_OPENING_PUNCTUATION)
[misspelling] ~193-~193: This word is normally spelled as one.
Context: ...of code) in under 3 seconds on a modern multi-core machine.1 This combination of speed and...
(EN_COMPOUNDS_MULTI_CORE)
[typographical] ~196-~196: To join two clauses or introduce examples, consider using an em dash.
Context: ...d auditing. ## Part 3: The Core Concept - Surgical Scoping ### 3.1 What srgn Mean...
(DASH_RULE)
[typographical] ~297-~297: To join two clauses or introduce examples, consider using an em dash.
Context: ...guage scopes.2 ## Part 4: Taking Action - Manipulation and Refactoring ### 4.1 Si...
(DASH_RULE)
[uncategorized] ~318-~318: Loose punctuation mark.
Context: ... as follows: - --python 'doc-strings': The operation is scoped exclusively to ...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~321-~321: Loose punctuation mark.
Context: ...docstrings. - '(?<!The )GNU ([a-z]+)': The regex scope uses a negative lookbeh...
(UNLIKELY_OPENING_PUNCTUATION)
[style] ~321-~321: Consider using the typographical ellipsis character here instead.
Context: ... regex scope uses a negative lookbehind (?<!...) to match the word "GNU" only when it ...
(ELLIPSIS)
[uncategorized] ~325-~325: Loose punctuation mark.
Context: ...roup 1. - -- '$1: GNU 🐂 is not Unix': The replacement string uses $1 to sub...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~341-~341: Loose punctuation mark.
Context: ...lt-in action flags include: - --upper, --lower, --titlecase: For changing ...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~343-~343: Loose punctuation mark.
Context: ...the case of matched text.2 - --delete: Removes the matched text. As a safety m...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~347-~347: Loose punctuation mark.
Context: ...n entire file's content.1 - --squeeze: Collapses sequences of whitespace. Like...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~350-~350: Loose punctuation mark.
Context: ...quires an explicit scope.1 - --german: A specialized action that correctly han...
(UNLIKELY_OPENING_PUNCTUATION)
[style] ~379-~379: Consider using the typographical ellipsis character here instead.
Context: ...core_utils. All import old_utilsandfrom old_utils import...` statements across the entire codebas...
(ELLIPSIS)
[style] ~396-~396: Consider using the typographical ellipsis character here instead.
Context: ...e module names within import and from... import statements, completely avoiding...
(ELLIPSIS)
[style] ~403-~403: This word has been used in one of the immediately preceding sentences. Using a synonym could make your text more interesting to read, unless the repetition is intentional.
Context: ...print` Calls to Structured Logging - Problem: A legacy section of the codebase use...
(EN_REPEATEDWORDS_PROBLEM)
[style] ~403-~403: Consider using the typographical ellipsis character here instead.
Context: ...: A legacy section of the codebase uses print(f"...") statements for debugging. These ne...
(ELLIPSIS)
[style] ~404-~404: Consider using the typographical ellipsis character here instead.
Context: ...hese need to be converted to structured logging.info(...) calls to integrate with a centraliz...
(ELLIPSIS)
[style] ~418-~418: Consider using the typographical ellipsis character here instead.
Context: ...))$is designed to match the entireprint(...)` expression, capturing all of its argu...
(ELLIPSIS)
[uncategorized] ~447-~447: Loose punctuation mark.
Context: ... advanced regex. 1. --py 'function': The search is first narrowed to the com...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~450-~450: Loose punctuation mark.
Context: ... 2. 'def\s+\w+\(.*\):\n\s+[^"''#\s]': This multi-line regex is then applied. ...
(UNLIKELY_OPENING_PUNCTUATION)
[style] ~483-~483: Consider using the typographical ellipsis character here instead.
Context: ...o focus the search exclusively within #[...] blocks. The regex `allow((clippy::so...
(ELLIPSIS)
[grammar] ~504-~504: Please add a punctuation mark at the end of paragraph.
Context: ...elease update 9, correctly identifies both unsafe fn definitions and `unsafe ...
(PUNCTUATION_PARAGRAPH_END)
[style] ~506-~506: Consider using the typographical ellipsis character here instead.
Context: ...s both unsafe fn definitions and unsafe {... } blocks—a task that would be diffic...
(ELLIPSIS)
[style] ~514-~514: This word has been used in one of the immediately preceding sentences. Using a synonym could make your text more interesting to read, unless the repetition is intentional.
Context: ...ate Renaming in use Declarations - Problem: A foundational crate within a large ...
(EN_REPEATEDWORDS_PROBLEM)
[style] ~531-~531: Consider using the typographical ellipsis character here instead.
Context: ...cope targets only the paths inside use...; statements. It will correctly change ...
(ELLIPSIS)
[typographical] ~539-~539: To join two clauses or introduce examples, consider using an em dash.
Context: ...alone cannot. ## Part 7: The Next Level - srgn as a Rust Library ### 7.1 Programm...
(DASH_RULE)
[uncategorized] ~562-~562: Loose punctuation mark.
Context: ... few key types 7: - ScopedViewBuilder: This is the entry point for all operati...
(UNLIKELY_OPENING_PUNCTUATION)
[typographical] ~578-~578: To join two clauses or introduce examples, consider using an em dash.
Context: ...ive path forward. ## Part 8: Conclusion - The Right Tool for the Right Cut srgn...
(DASH_RULE)
[grammar] ~618-~618: Please add a punctuation mark at the end of paragraph.
Context: ...release notes.2 As direct inspection of the PreparedQuery source enum was not po...
(PUNCTUATION_PARAGRAPH_END)
[style] ~632-~632: Consider using the typographical ellipsis character here instead.
Context: ...| Selects the content of docstrings ("""...""" or '''...'''). | srgn ...
(ELLIPSIS)
[style] ~632-~632: Consider using the typographical ellipsis character here instead.
Context: ... content of docstrings ("""...""" or '''...'''). | srgn --py 'doc-str...
(ELLIPSIS)
[style] ~633-~633: Consider using the typographical ellipsis character here instead.
Context: ... Selects the content of line comments (#...). | srgn -...
(ELLIPSIS)
[style] ~636-~636: Consider using the typographical ellipsis character here instead.
Context: ...only the module names in import and from... import statements. | srgn --py 'modu...
(ELLIPSIS)
[style] ~644-~644: Consider using the typographical ellipsis character here instead.
Context: ...s the content of line (//) and block (/.../) comments. | srgn --rs 'comments' 'HA...
(ELLIPSIS)
[style] ~646-~646: Consider using the typographical ellipsis character here instead.
Context: ... | Selects the content of attributes (#[...] and #![...]). | srgn --rs 'attr...
(ELLIPSIS)
[style] ~646-~646: Consider using the typographical ellipsis character here instead.
Context: ...he content of attributes (#[...] and #![...]). | srgn --rs 'attribute' 'depr...
(ELLIPSIS)
[style] ~653-~653: Consider using the typographical ellipsis character here instead.
Context: ...ate | Selects extern crate...; declarations. ...
(ELLIPSIS)
[typographical] ~658-~658: To join two clauses or introduce examples, consider using an em dash.
Context: ...s for manipulation in addition to search - GitHub, accessed on July 11, 2025, <...
(DASH_RULE)
[typographical] ~666-~666: Consider using an em dash in dialogues and enumerations.
Context: ...entation of super-resolution paper. - GitHub, accessed on July 11, 2025, ...
(DASH_RULE)
[typographical] ~669-~669: To join two clauses or introduce examples, consider using an em dash.
Context: ...natang/SRGAN-PyTorch> 4. hep-lbdl/SRGN - GitHub, accessed on July 11, 2025, <...
(DASH_RULE)
[typographical] ~672-~672: To join two clauses or introduce examples, consider using an em dash.
Context: .../github.com/hep-lbdl/SRGN> 5. Security - hep-lbdl/SRGN - GitHub, accessed on July...
(DASH_RULE)
[typographical] ~672-~672: To join two clauses or introduce examples, consider using an em dash.
Context: ...lbdl/SRGN> 5. Security - hep-lbdl/SRGN - GitHub, accessed on July 11, 2025, <...
(DASH_RULE)
[typographical] ~676-~676: To join two clauses or introduce examples, consider using an em dash.
Context: ... · synthesizearrayHSy/generatemonitorGhZ - GitHub, accessed on July 11, 2025, ...
(DASH_RULE)
[typographical] ~680-~680: To join two clauses or introduce examples, consider using an em dash.
Context: ...y/generatemonitorGhZ/issues/5> 7. srgn - Rust - Docs.rs, access...
(DASH_RULE)
[typographical] ~680-~680: To join two clauses or introduce examples, consider using an em dash.
Context: ...atemonitorGhZ/issues/5> 7. srgn - Rust - Docs.rs, accessed on J...
(DASH_RULE)
[typographical] ~683-~683: To join two clauses or introduce examples, consider using an em dash.
Context: ...ttps://docs.rs/srgn> 8. Pattern syntax - Semgrep, accessed on July 11, 2025, ...
(DASH_RULE)
[typographical] ~686-~686: To join two clauses or introduce examples, consider using an em dash.
Context: ...n-syntax> 9. Releases · alexpovel/srgn - GitHub, accessed on July 11, 2025, <...
(DASH_RULE)
[typographical] ~692-~692: To join two clauses or introduce examples, consider using an em dash.
Context: ...com/python-scope-legb-rule/> 11. Scopes - The Rust Reference, accessed on July 11,...
(DASH_RULE)
[typographical] ~696-~696: To join two clauses or introduce examples, consider using an em dash.
Context: ...Language, 2nd Ed. Klabnik & Nichols) - Stack Overflow, accessed on July 11, 202...
(DASH_RULE)
[grammar] ~697-~697: Please add a punctuation mark at the end of paragraph.
Context: ...erflow, accessed on July 11, 2025, https://stackoverflow.com/questions/77423163/i-cant-understand-the-rust-scope-definition-rust-programming-language-2nd-e 13. betterletter/[README.md](http://R...
(PUNCTUATION_PARAGRAPH_END)
[typographical] ~703-~703: To join two clauses or introduce examples, consider using an em dash.
Context: ...terletter/blob/main/README.md> 14. srgn - Rust Package Registry - [Crates.io](http...
(DASH_RULE)
[typographical] ~703-~703: To join two clauses or introduce examples, consider using an em dash.
Context: ...ME.md> 14. srgn - Rust Package Registry - Crates.io, accessed ...
(DASH_RULE)
🔍 MCP Research (1 server)
Deepwiki:
-
Document [Command Line Interface]: The
runner::runfunction dispatches CLI commands parsed byclap. TheBuildcommand triggers the full build pipeline: manifest ingestion, Jinja rendering, YAML parsing, IR generation, Ninja synthesis, and finally invokes the Ninja subprocess. Therunfunction is responsible for orchestrating these steps and forwarding CLI options such as--directoryand--jobsto Ninja. TheBuildcommand accepts a list of targets to build, defaulting to the manifest's default targets if empty. The CLI module uses a clean separation of concerns withCliandCommandsenums, and therunfunction is the main execution entry point for commands. (Source: Command Line Interface doc) -
Document [Architecture]: The
src/runner.rsmodule is part of the application layer responsible for executing commands based on the CLI input. It interacts with core library modules likemanifestfor loading and parsing the manifest,irfor generating the build graph, andninja_genfor synthesizing the Ninja build file. The runner invokes the Ninja executable as a subprocess using Rust'sstd::process::Command. The architecture enforces a five-stage pipeline where the runner coordinates stages 1 (manifest ingestion), 3 (YAML parsing), 4 (IR generation), and 5 (Ninja synthesis and execution). (Source: Architecture doc) -
Document [Implementation Details]: The
runner::runfunction callsmanifest::load_from_pathto read and parse the manifest, then callsir::from_manifestto generate the build graph IR. It then usesninja_gen::generateto produce the Ninja build file. The runner manages error propagation usinganyhow::Resultand enriches errors with context. The Ninja subprocess is executed with the appropriate command-line arguments derived from CLI options. The runner module encapsulates the orchestration logic for the build command, including manifest path resolution and temporary file handling for the Ninja manifest. (Source: Implementation Details doc) -
Document [Code Quality Standards]: The codebase enforces strict code style and modularity. Functions like
runner::runshould be small and maintainable, with clear separation of concerns. Helper functions are encouraged to keep the main run function shallow and readable. Error handling usesanyhowandthiserrorwith contextual messages. Naming conventions and documentation standards require descriptive names and module-level comments. (Source: Code Quality Standards doc)
⏰ 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 (10)
AGENTS.md (1)
153-163: Example block update looks goodAdding the
rustlanguage identifier improves syntax highlighting and meets documentation
requirements.src/runner.rs (9)
13-16: Imports updated appropriately for new helpersKeep
PathBufandNamedTempFileimports; they are required for the new path handling and temp file helpers.
52-61: Borrowing build targets removes allocationWrap targets as a borrowed slice. This eliminates cloning and aligns with how
Command::argsconsumes references.
80-84: Flatten Build arm and reuse helpers (stale code removed)Delegate Build to
handle_buildand callwrite_ninja_filein Manifest arm. This resolves earlier duplication and removes inline tempfile/manifest join logic in the CLI path.
129-149: Temp-file helper is correct and robustCreate a temp file with stable prefix/suffix, write once via the shared writer, and return the guard to manage lifetime. Good error context.
160-167: File writer is correct and logs locationKeep this as the single write path used by both Manifest and Build flows. The contextual error message is useful.
190-190: Centralise manifest path resolutionRoute manifest loading through
resolve_manifest_path. This removes ad-hoc joins and aligns with the documented pipeline.
199-214: Manifest resolution helper is correct; doctest scope handledKeep
#[must_use]and the ignored doctest. This matches the earlier guidance and avoids stale inline path join logic elsewhere.
294-294: Update run_ninja signature to borrowed targetsPass a borrowed slice via
BuildTargetsto avoid allocations.Command::argsaccepts this shape; usage is correct.
97-127: Confirm coverage and resolve testing suggestion
- Verified all call sites use
write_ninja_fileandcreate_temp_ninja_filewith no stale helpers.- Confirmed a single
run_ninjainvocation covers both branches.- Located tests in
•tests/runner_tests.rsfor non-emit and emit branches
•tests/assert_cmd_tests.rsfor--emitCLI behaviour
• CLI feature tests verifying--emitand temp-file scenariosRemove the “add tests” suggestion; existing tests already cover both branches.
There was a problem hiding this comment.
Actionable comments posted: 2
🔭 Outside diff range comments (1)
docs/srgn.md (1)
98-108: Remove stray “Bash” label and use lowercase language tagDrop the standalone “Bash” lines and switch the language tag to lowercase
bashto meet documentation conventions.- Bash - - ```Bash +```bashAlso applies to: 113-118
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
docs/srgn.md(3 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
docs/**/*.md
📄 CodeRabbit Inference Engine (AGENTS.md)
docs/**/*.md: Use the markdown files within thedocs/directory as a knowledge base and source of truth for project requirements, dependency choices, and architectural decisions.
Proactively update the relevant file(s) in thedocs/directory to reflect the latest state when new decisions are made, requirements change, libraries are added/removed, or architectural patterns evolve.
Files:
docs/srgn.md
**/*.md
📄 CodeRabbit Inference Engine (AGENTS.md)
**/*.md: Documentation must use en-GB-oxendict spelling and grammar, except for the naming of the "LICENSE" file.
Validate Markdown files usingmake markdownlint.
Runmake fmtafter any documentation changes to format all Markdown files and fix table markup.
Validate Mermaid diagrams in Markdown files by runningmake nixie.
Markdown paragraphs and bullet points must be wrapped at 80 columns.
Code blocks in Markdown must be wrapped at 120 columns.
Tables and headings in Markdown must not be wrapped.
Use dashes (-) for list bullets in Markdown.
Use GitHub-flavoured Markdown footnotes ([^1]) for references and footnotes.
Files:
docs/srgn.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/srgn.md
🪛 LanguageTool
docs/srgn.md
[uncategorized] ~20-~20: Loose punctuation mark.
Context: ... regex-based pattern matching of grep, the stream-editing capabilities of tr...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~25-~25: Possible missing comma found.
Context: ...or full-featured IDE refactoring engines but a specialized scalpel for tasks that ar...
(AI_HYDRA_LEO_MISSING_COMMA)
[typographical] ~57-~57: The word ‘where’ starts a question. Add a question mark (“?”) at the end of the sentence.
Context: ...ld be done to the text within that scope. This separation of concerns is central ...
(WRB_QUESTION_MARK)
[grammar] ~61-~61: There is an agreement error between ‘know’ and ‘regex’. Insert ‘a(n)’ or change the noun to plural.
Context: ...states its design goal clearly: "if you know regex and the basics of the language you are ...
(PRP_VB_NN)
[grammar] ~62-~62: Please add a punctuation mark at the end of paragraph.
Context: ...ou are good to go".[^2] This philosophy distinguishes srgn from other advanced code-queryi...
(PUNCTUATION_PARAGRAPH_END)
[style] ~66-~66: Consider using the typographical ellipsis character here instead.
Context: ...with metavariables ($X) and ellipses (...) to find code that matches an abstract...
(ELLIPSIS)
[typographical] ~81-~81: To join two clauses or introduce examples, consider using an em dash.
Context: ...ommand line. ## Part 2: Getting Started - Installation and First Cuts ### 2.1 Ins...
(DASH_RULE)
[style] ~85-~85: To form a complete sentence, be sure to include a subject.
Context: ...Preparing the Operating Theater srgn can be installed across various platforms, ...
(MISSING_IT_THERE)
[uncategorized] ~93-~93: Loose punctuation mark.
Context: ...b Releases page.[^1] - cargo-binstall: For users with the Rust toolchain, this...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~94-~94: Possible missing comma found.
Context: ...ignificantly faster than compiling from source as it downloads prebuilt binaries whe...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~109-~109: Loose punctuation mark.
Context: ...install srgn ``` - cargo install: The traditional method of compiling fro...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~155-~155: Loose punctuation mark.
Context: ...es (e.g., --python, --rust). - '': This is the mandatory, positional regul...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~163-~163: Loose punctuation mark.
Context: ...xtensions and shebangs.[^1] - -- '': This is the optional replacement string...
(UNLIKELY_OPENING_PUNCTUATION)
[misspelling] ~196-~196: This word is normally spelled as one.
Context: ...of code) in under 3 seconds on a modern multi-core machine.[^1] This combination of speed ...
(EN_COMPOUNDS_MULTI_CORE)
[typographical] ~199-~199: To join two clauses or introduce examples, consider using an em dash.
Context: ...d auditing. ## Part 3: The Core Concept - Surgical Scoping ### 3.1 What srgn Mean...
(DASH_RULE)
[typographical] ~300-~300: To join two clauses or introduce examples, consider using an em dash.
Context: ...ge scopes.[^2] ## Part 4: Taking Action - Manipulation and Refactoring ### 4.1 Si...
(DASH_RULE)
[uncategorized] ~321-~321: Loose punctuation mark.
Context: ... as follows: - --python 'doc-strings': The operation is scoped exclusively to ...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~324-~324: Loose punctuation mark.
Context: ...docstrings. - '(?<!The )GNU ([a-z]+)': The regex scope uses a negative lookbeh...
(UNLIKELY_OPENING_PUNCTUATION)
[style] ~324-~324: Consider using the typographical ellipsis character here instead.
Context: ... regex scope uses a negative lookbehind (?<!...) to match the word "GNU" only when it ...
(ELLIPSIS)
[uncategorized] ~328-~328: Loose punctuation mark.
Context: ...roup 1. - -- '$1: GNU 🐂 is not Unix': The replacement string uses $1 to sub...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~335-~335: Possible missing comma found.
Context: ... These actions are applied in a defined order after the main replacement has occurr...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~344-~344: Loose punctuation mark.
Context: ...lt-in action flags include: - --upper, --lower, --titlecase: For changing ...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~347-~347: Loose punctuation mark.
Context: ...ase of matched text.[^2] - --delete: Removes the matched text. As a safety m...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~351-~351: Loose punctuation mark.
Context: ...ntire file's content.[^1] - --squeeze: Collapses sequences of whitespace. Like...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~354-~354: Loose punctuation mark.
Context: ...res an explicit scope.[^1] - --german: A specialized action that correctly han...
(UNLIKELY_OPENING_PUNCTUATION)
[style] ~383-~383: Consider using the typographical ellipsis character here instead.
Context: ...core_utils. All import old_utilsandfrom old_utils import...` statements across the entire codebas...
(ELLIPSIS)
[style] ~400-~400: Consider using the typographical ellipsis character here instead.
Context: ...e module names within import and from... import statements, completely avoiding...
(ELLIPSIS)
[style] ~407-~407: This word has been used in one of the immediately preceding sentences. Using a synonym could make your text more interesting to read, unless the repetition is intentional.
Context: ...print` Calls to Structured Logging - Problem: A legacy section of the codebase use...
(EN_REPEATEDWORDS_PROBLEM)
[style] ~407-~407: Consider using the typographical ellipsis character here instead.
Context: ...: A legacy section of the codebase uses print(f"...") statements for debugging. These ne...
(ELLIPSIS)
[style] ~408-~408: Consider using the typographical ellipsis character here instead.
Context: ...hese need to be converted to structured logging.info(...) calls to integrate with a centraliz...
(ELLIPSIS)
[style] ~422-~422: Consider using the typographical ellipsis character here instead.
Context: ...))$is designed to match the entireprint(...)` expression, capturing all of its argu...
(ELLIPSIS)
[uncategorized] ~451-~451: Loose punctuation mark.
Context: ... advanced regex. 1. --py 'function': The search is first narrowed to the com...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~454-~454: Loose punctuation mark.
Context: ... 2. 'def\s+\w+\(.*\):\n\s+[^"''#\s]': This multi-line regex is then applied. ...
(UNLIKELY_OPENING_PUNCTUATION)
[style] ~487-~487: Consider using the typographical ellipsis character here instead.
Context: ...o focus the search exclusively within #[...] blocks. The regex `allow((clippy::so...
(ELLIPSIS)
[grammar] ~508-~508: Please add a punctuation mark at the end of paragraph.
Context: ...elease update 9, correctly identifies both unsafe fn definitions and `unsafe ...
(PUNCTUATION_PARAGRAPH_END)
[style] ~510-~510: Consider using the typographical ellipsis character here instead.
Context: ...s both unsafe fn definitions and unsafe {... } blocks—a task that would be diffic...
(ELLIPSIS)
[style] ~518-~518: This word has been used in one of the immediately preceding sentences. Using a synonym could make your text more interesting to read, unless the repetition is intentional.
Context: ...ate Renaming in use Declarations - Problem: A foundational crate within a large ...
(EN_REPEATEDWORDS_PROBLEM)
[style] ~535-~535: Consider using the typographical ellipsis character here instead.
Context: ...cope targets only the paths inside use...; statements. It will correctly change ...
(ELLIPSIS)
[typographical] ~543-~543: To join two clauses or introduce examples, consider using an em dash.
Context: ...alone cannot. ## Part 7: The Next Level - srgn as a Rust Library ### 7.1 Programm...
(DASH_RULE)
[uncategorized] ~566-~566: Loose punctuation mark.
Context: ... few key types 7: - ScopedViewBuilder: This is the entry point for all operati...
(UNLIKELY_OPENING_PUNCTUATION)
[typographical] ~582-~582: To join two clauses or introduce examples, consider using an em dash.
Context: ...ive path forward. ## Part 8: Conclusion - The Right Tool for the Right Cut srgn...
(DASH_RULE)
[grammar] ~622-~622: Please add a punctuation mark at the end of paragraph.
Context: ...ease notes.[^2] As direct inspection of the PreparedQuery source enum was not po...
(PUNCTUATION_PARAGRAPH_END)
[style] ~636-~636: Consider using the typographical ellipsis character here instead.
Context: ...| Selects the content of docstrings ("""...""" or '''...'''). | srgn ...
(ELLIPSIS)
[style] ~636-~636: Consider using the typographical ellipsis character here instead.
Context: ... content of docstrings ("""...""" or '''...'''). | srgn --py 'doc-str...
(ELLIPSIS)
[style] ~637-~637: Consider using the typographical ellipsis character here instead.
Context: ... Selects the content of line comments (#...). | srgn -...
(ELLIPSIS)
[style] ~640-~640: Consider using the typographical ellipsis character here instead.
Context: ...only the module names in import and from... import statements. | srgn --py 'modu...
(ELLIPSIS)
[style] ~648-~648: Consider using the typographical ellipsis character here instead.
Context: ...s the content of line (//) and block (/.../) comments. | srgn --rs 'comments' 'HA...
(ELLIPSIS)
[style] ~650-~650: Consider using the typographical ellipsis character here instead.
Context: ... | Selects the content of attributes (#[...] and #![...]). | srgn --rs 'attr...
(ELLIPSIS)
[style] ~650-~650: Consider using the typographical ellipsis character here instead.
Context: ...he content of attributes (#[...] and #![...]). | srgn --rs 'attribute' 'depr...
(ELLIPSIS)
[style] ~657-~657: Consider using the typographical ellipsis character here instead.
Context: ...ate | Selects extern crate...; declarations. ...
(ELLIPSIS)
[typographical] ~662-~662: To join two clauses or introduce examples, consider using an em dash.
Context: ...s for manipulation in addition to search - GitHub, accessed on July 11, 2025, <...
(DASH_RULE)
[typographical] ~670-~670: Consider using an em dash in dialogues and enumerations.
Context: ...entation of super-resolution paper. - GitHub, accessed on July 11, 2025, ...
(DASH_RULE)
[typographical] ~673-~673: To join two clauses or introduce examples, consider using an em dash.
Context: ...natang/SRGAN-PyTorch> 4. hep-lbdl/SRGN - GitHub, accessed on July 11, 2025, <...
(DASH_RULE)
[typographical] ~676-~676: To join two clauses or introduce examples, consider using an em dash.
Context: .../github.com/hep-lbdl/SRGN> 5. Security - hep-lbdl/SRGN - GitHub, accessed on July...
(DASH_RULE)
[typographical] ~676-~676: To join two clauses or introduce examples, consider using an em dash.
Context: ...lbdl/SRGN> 5. Security - hep-lbdl/SRGN - GitHub, accessed on July 11, 2025, <...
(DASH_RULE)
[typographical] ~680-~680: To join two clauses or introduce examples, consider using an em dash.
Context: ... · synthesizearrayHSy/generatemonitorGhZ - GitHub, accessed on July 11, 2025, ...
(DASH_RULE)
[typographical] ~684-~684: To join two clauses or introduce examples, consider using an em dash.
Context: ...y/generatemonitorGhZ/issues/5> 7. srgn - Rust - Docs.rs, access...
(DASH_RULE)
[typographical] ~684-~684: To join two clauses or introduce examples, consider using an em dash.
Context: ...atemonitorGhZ/issues/5> 7. srgn - Rust - Docs.rs, accessed on J...
(DASH_RULE)
[typographical] ~687-~687: To join two clauses or introduce examples, consider using an em dash.
Context: ...ttps://docs.rs/srgn> 8. Pattern syntax - Semgrep, accessed on July 11, 2025, ...
(DASH_RULE)
[typographical] ~690-~690: To join two clauses or introduce examples, consider using an em dash.
Context: ...n-syntax> 9. Releases · alexpovel/srgn - GitHub, accessed on July 11, 2025, <...
(DASH_RULE)
[typographical] ~696-~696: To join two clauses or introduce examples, consider using an em dash.
Context: ...com/python-scope-legb-rule/> 11. Scopes - The Rust Reference, accessed on July 11,...
(DASH_RULE)
[typographical] ~700-~700: To join two clauses or introduce examples, consider using an em dash.
Context: ...Language, 2nd Ed. Klabnik & Nichols) - Stack Overflow, accessed on July 11, 202...
(DASH_RULE)
[grammar] ~701-~701: Please add a punctuation mark at the end of paragraph.
Context: ...erflow, accessed on July 11, 2025, https://stackoverflow.com/questions/77423163/i-cant-understand-the-rust-scope-definition-rust-programming-language-2nd-e 13. betterletter/[README.md](http://R...
(PUNCTUATION_PARAGRAPH_END)
[typographical] ~707-~707: To join two clauses or introduce examples, consider using an em dash.
Context: ...terletter/blob/main/README.md> 14. srgn - Rust Package Registry - [Crates.io](http...
(DASH_RULE)
[typographical] ~707-~707: To join two clauses or introduce examples, consider using an em dash.
Context: ...ME.md> 14. srgn - Rust Package Registry - Crates.io, accessed ...
(DASH_RULE)
🔍 MCP Research (1 server)
Deepwiki:
-
The
BuildTargetsstruct was changed from owning aVec<String>to a lightweight wrapper over a string slice (&[String]), with corresponding lifetime annotations and method signature updates. (src/runner.rs) -
The
runfunction's handling of theCommands::Buildvariant was refactored by extracting the build logic into a new helper functionhandle_build. This function generates the Ninja manifest, creates aBuildTargetsinstance from a slice reference to the targets, and conditionally writes the Ninja file either to a specified path or to a newly created temporary file. The temporary file creation and writing logic was moved into a dedicated functioncreate_temp_ninja_filethat returns aNamedTempFile. The temporary file is kept alive during the Ninja invocation by holding theNamedTempFilein an option tuple element. (src/runner.rs) -
The function
write_and_logwas renamed towrite_ninja_fileand its usage was updated accordingly, including in theCommands::Manifestbranch and the new helper functions. Thegenerate_ninjafunction was modified to use a new helperresolve_manifest_pathto determine the manifest path based on the CLI's directory option, replacing the previous inline path resolution logic. (src/runner.rs) -
A constant
NINJA_PROGRAMwas introduced to represent the default Ninja executable name, replacing hardcoded"ninja"string literals. (src/runner.rs)
⏰ 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
- publish NINJA_PROGRAM so tests can import the default executable\n- point cucumber steps at the constant instead of repeating the string\n- tidy srgn guide paragraph and narrow test helper lint
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 9
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (4)
docs/srgn.md(3 hunks)src/runner.rs(8 hunks)tests/steps/process_steps.rs(2 hunks)tests/support/mod.rs(2 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.rs
⚙️ CodeRabbit Configuration File
**/*.rs: * Seek to keep the cyclomatic complexity of functions no more than 12.
Adhere to single responsibility and CQRS
Place function attributes after doc comments.
Do not use
returnin single-line functions.Move conditionals with >2 branches into a predicate function.
Avoid
unsafeunless absolutely necessary.Every module must begin with a
//!doc comment that explains the module's purpose and utility.Comments and docs must follow en-GB-oxendict (-ize / -our) spelling and grammar
Lints must not be silenced except as a last resort.
#[allow]is forbidden.- Only narrowly scoped
#[expect(lint, reason = "...")]is allowed.- No lint groups, no blanket or file-wide suppression.
- Include
FIXME:with link if a fix is expected.Use
rstestfixtures for shared setup and to avoid repetition between tests.Replace duplicated tests with
#[rstest(...)]parameterised cases.Prefer
mockallfor mocks/stubs.Prefer
.expect()over.unwrap()Ensure that any API or behavioural changes are reflected in the documentation in
docs/Ensure that any completed roadmap steps are recorded in the appropriate roadmap in
docs/Files must not exceed 400 lines in length
- Large modules must be decomposed
- Long match statements or dispatch tables should be decomposed by domain and collocated with targets
- Large blocks of inline data (e.g., test fixtures, constants or templates) must be moved to external files and inlined at compile-time or loaded at run-time.
Files:
tests/steps/process_steps.rssrc/runner.rstests/support/mod.rs
**/*.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/srgn.md
🧬 Code Graph Analysis (1)
src/runner.rs (1)
tests/steps/cli_steps.rs (1)
manifest_path(104-107)
🪛 LanguageTool
docs/srgn.md
[uncategorized] ~23-~23: Possible missing comma found.
Context: ...or full-featured IDE refactoring engines but a specialized scalpel for tasks that ar...
(AI_HYDRA_LEO_MISSING_COMMA)
[typographical] ~55-~55: The word ‘where’ starts a question. Add a question mark (“?”) at the end of the sentence.
Context: ...ld be done to the text within that scope. This separation of concerns is central ...
(WRB_QUESTION_MARK)
[grammar] ~59-~59: There is an agreement error between ‘know’ and ‘regex’. Insert ‘a(n)’ or change the noun to plural.
Context: ...states its design goal clearly: "if you know regex and the basics of the language you are ...
(PRP_VB_NN)
[grammar] ~60-~60: Please add a punctuation mark at the end of paragraph.
Context: ...ou are good to go".[^2] This philosophy distinguishes srgn from other advanced code-queryi...
(PUNCTUATION_PARAGRAPH_END)
[style] ~64-~64: Consider using the typographical ellipsis character here instead.
Context: ...with metavariables ($X) and ellipses (...) to find code that matches an abstract...
(ELLIPSIS)
[typographical] ~79-~79: To join two clauses or introduce examples, consider using an em dash.
Context: ...ommand line. ## Part 2: Getting Started - Installation and First Cuts ### 2.1 Ins...
(DASH_RULE)
[style] ~83-~83: To form a complete sentence, be sure to include a subject.
Context: ...Preparing the Operating Theater srgn can be installed across various platforms, ...
(MISSING_IT_THERE)
[uncategorized] ~91-~91: Loose punctuation mark.
Context: ...b Releases page.[^1] - cargo-binstall: For users with the Rust toolchain, this...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~92-~92: Possible missing comma found.
Context: ...ignificantly faster than compiling from source as it downloads prebuilt binaries whe...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~107-~107: Loose punctuation mark.
Context: ...install srgn ``` - cargo install: The traditional method of compiling fro...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~153-~153: Loose punctuation mark.
Context: ...es (e.g., --python, --rust). - '': This is the mandatory, positional regul...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~161-~161: Loose punctuation mark.
Context: ...xtensions and shebangs.[^1] - -- '': This is the optional replacement string...
(UNLIKELY_OPENING_PUNCTUATION)
[misspelling] ~194-~194: This word is normally spelled as one.
Context: ...of code) in under 3 seconds on a modern multi-core machine.[^1] This combination of speed ...
(EN_COMPOUNDS_MULTI_CORE)
[typographical] ~197-~197: To join two clauses or introduce examples, consider using an em dash.
Context: ...d auditing. ## Part 3: The Core Concept - Surgical Scoping ### 3.1 What srgn Mean...
(DASH_RULE)
[typographical] ~298-~298: To join two clauses or introduce examples, consider using an em dash.
Context: ...ge scopes.[^2] ## Part 4: Taking Action - Manipulation and Refactoring ### 4.1 Si...
(DASH_RULE)
[uncategorized] ~319-~319: Loose punctuation mark.
Context: ... as follows: - --python 'doc-strings': The operation is scoped exclusively to ...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~322-~322: Loose punctuation mark.
Context: ...docstrings. - '(?<!The )GNU ([a-z]+)': The regex scope uses a negative lookbeh...
(UNLIKELY_OPENING_PUNCTUATION)
[style] ~322-~322: Consider using the typographical ellipsis character here instead.
Context: ... regex scope uses a negative lookbehind (?<!...) to match the word "GNU" only when it ...
(ELLIPSIS)
[uncategorized] ~326-~326: Loose punctuation mark.
Context: ...roup 1. - -- '$1: GNU 🐂 is not Unix': The replacement string uses $1 to sub...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~333-~333: Possible missing comma found.
Context: ... These actions are applied in a defined order after the main replacement has occurr...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~342-~342: Loose punctuation mark.
Context: ...lt-in action flags include: - --upper, --lower, --titlecase: For changing ...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~345-~345: Loose punctuation mark.
Context: ...ase of matched text.[^2] - --delete: Removes the matched text. As a safety m...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~349-~349: Loose punctuation mark.
Context: ...ntire file's content.[^1] - --squeeze: Collapses sequences of whitespace. Like...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~352-~352: Loose punctuation mark.
Context: ...res an explicit scope.[^1] - --german: A specialized action that correctly han...
(UNLIKELY_OPENING_PUNCTUATION)
[style] ~381-~381: Consider using the typographical ellipsis character here instead.
Context: ...core_utils. All import old_utilsandfrom old_utils import...` statements across the entire codebas...
(ELLIPSIS)
[style] ~398-~398: Consider using the typographical ellipsis character here instead.
Context: ...e module names within import and from... import statements, completely avoiding...
(ELLIPSIS)
[style] ~405-~405: This word has been used in one of the immediately preceding sentences. Using a synonym could make your text more interesting to read, unless the repetition is intentional.
Context: ...print` Calls to Structured Logging - Problem: A legacy section of the codebase use...
(EN_REPEATEDWORDS_PROBLEM)
[style] ~405-~405: Consider using the typographical ellipsis character here instead.
Context: ...: A legacy section of the codebase uses print(f"...") statements for debugging. These ne...
(ELLIPSIS)
[style] ~406-~406: Consider using the typographical ellipsis character here instead.
Context: ...hese need to be converted to structured logging.info(...) calls to integrate with a centraliz...
(ELLIPSIS)
[style] ~420-~420: Consider using the typographical ellipsis character here instead.
Context: ...))$is designed to match the entireprint(...)` expression, capturing all of its argu...
(ELLIPSIS)
[uncategorized] ~449-~449: Loose punctuation mark.
Context: ... advanced regex. 1. --py 'function': The search is first narrowed to the com...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~452-~452: Loose punctuation mark.
Context: ... 2. 'def\s+\w+\(.*\):\n\s+[^"''#\s]': This multi-line regex is then applied. ...
(UNLIKELY_OPENING_PUNCTUATION)
[style] ~485-~485: Consider using the typographical ellipsis character here instead.
Context: ...o focus the search exclusively within #[...] blocks. The regex `allow((clippy::so...
(ELLIPSIS)
[grammar] ~506-~506: Please add a punctuation mark at the end of paragraph.
Context: ...elease update 9, correctly identifies both unsafe fn definitions and `unsafe ...
(PUNCTUATION_PARAGRAPH_END)
[style] ~508-~508: Consider using the typographical ellipsis character here instead.
Context: ...s both unsafe fn definitions and unsafe {... } blocks—a task that would be diffic...
(ELLIPSIS)
[style] ~516-~516: This word has been used in one of the immediately preceding sentences. Using a synonym could make your text more interesting to read, unless the repetition is intentional.
Context: ...ate Renaming in use Declarations - Problem: A foundational crate within a large ...
(EN_REPEATEDWORDS_PROBLEM)
[style] ~533-~533: Consider using the typographical ellipsis character here instead.
Context: ...cope targets only the paths inside use...; statements. It will correctly change ...
(ELLIPSIS)
[typographical] ~541-~541: To join two clauses or introduce examples, consider using an em dash.
Context: ...alone cannot. ## Part 7: The Next Level - srgn as a Rust Library ### 7.1 Programm...
(DASH_RULE)
[uncategorized] ~564-~564: Loose punctuation mark.
Context: ... few key types 7: - ScopedViewBuilder: This is the entry point for all operati...
(UNLIKELY_OPENING_PUNCTUATION)
[typographical] ~580-~580: To join two clauses or introduce examples, consider using an em dash.
Context: ...ive path forward. ## Part 8: Conclusion - The Right Tool for the Right Cut srgn...
(DASH_RULE)
[grammar] ~620-~620: Please add a punctuation mark at the end of paragraph.
Context: ...ease notes.[^2] As direct inspection of the PreparedQuery source enum was not po...
(PUNCTUATION_PARAGRAPH_END)
[style] ~634-~634: Consider using the typographical ellipsis character here instead.
Context: ...| Selects the content of docstrings ("""...""" or '''...'''). | srgn ...
(ELLIPSIS)
[style] ~634-~634: Consider using the typographical ellipsis character here instead.
Context: ... content of docstrings ("""...""" or '''...'''). | srgn --py 'doc-str...
(ELLIPSIS)
[style] ~635-~635: Consider using the typographical ellipsis character here instead.
Context: ... Selects the content of line comments (#...). | srgn -...
(ELLIPSIS)
[style] ~638-~638: Consider using the typographical ellipsis character here instead.
Context: ...only the module names in import and from... import statements. | srgn --py 'modu...
(ELLIPSIS)
[style] ~646-~646: Consider using the typographical ellipsis character here instead.
Context: ...s the content of line (//) and block (/.../) comments. | srgn --rs 'comments' 'HA...
(ELLIPSIS)
[style] ~648-~648: Consider using the typographical ellipsis character here instead.
Context: ... | Selects the content of attributes (#[...] and #![...]). | srgn --rs 'attr...
(ELLIPSIS)
[style] ~648-~648: Consider using the typographical ellipsis character here instead.
Context: ...he content of attributes (#[...] and #![...]). | srgn --rs 'attribute' 'depr...
(ELLIPSIS)
[style] ~655-~655: Consider using the typographical ellipsis character here instead.
Context: ...ate | Selects extern crate...; declarations. ...
(ELLIPSIS)
[typographical] ~660-~660: To join two clauses or introduce examples, consider using an em dash.
Context: ...s for manipulation in addition to search - GitHub, accessed on July 11, 2025, <...
(DASH_RULE)
[typographical] ~668-~668: Consider using an em dash in dialogues and enumerations.
Context: ...entation of super-resolution paper. - GitHub, accessed on July 11, 2025, ...
(DASH_RULE)
[typographical] ~671-~671: To join two clauses or introduce examples, consider using an em dash.
Context: ...natang/SRGAN-PyTorch> 4. hep-lbdl/SRGN - GitHub, accessed on July 11, 2025, <...
(DASH_RULE)
[typographical] ~674-~674: To join two clauses or introduce examples, consider using an em dash.
Context: .../github.com/hep-lbdl/SRGN> 5. Security - hep-lbdl/SRGN - GitHub, accessed on July...
(DASH_RULE)
[typographical] ~674-~674: To join two clauses or introduce examples, consider using an em dash.
Context: ...lbdl/SRGN> 5. Security - hep-lbdl/SRGN - GitHub, accessed on July 11, 2025, <...
(DASH_RULE)
[typographical] ~678-~678: To join two clauses or introduce examples, consider using an em dash.
Context: ... · synthesizearrayHSy/generatemonitorGhZ - GitHub, accessed on July 11, 2025, ...
(DASH_RULE)
[typographical] ~682-~682: To join two clauses or introduce examples, consider using an em dash.
Context: ...y/generatemonitorGhZ/issues/5> 7. srgn - Rust - Docs.rs, access...
(DASH_RULE)
[typographical] ~682-~682: To join two clauses or introduce examples, consider using an em dash.
Context: ...atemonitorGhZ/issues/5> 7. srgn - Rust - Docs.rs, accessed on J...
(DASH_RULE)
[typographical] ~685-~685: To join two clauses or introduce examples, consider using an em dash.
Context: ...ttps://docs.rs/srgn> 8. Pattern syntax - Semgrep, accessed on July 11, 2025, ...
(DASH_RULE)
[typographical] ~688-~688: To join two clauses or introduce examples, consider using an em dash.
Context: ...n-syntax> 9. Releases · alexpovel/srgn - GitHub, accessed on July 11, 2025, <...
(DASH_RULE)
[typographical] ~694-~694: To join two clauses or introduce examples, consider using an em dash.
Context: ...com/python-scope-legb-rule/> 11. Scopes - The Rust Reference, accessed on July 11,...
(DASH_RULE)
[typographical] ~698-~698: To join two clauses or introduce examples, consider using an em dash.
Context: ...Language, 2nd Ed. Klabnik & Nichols) - Stack Overflow, accessed on July 11, 202...
(DASH_RULE)
[grammar] ~699-~699: Please add a punctuation mark at the end of paragraph.
Context: ...erflow, accessed on July 11, 2025, https://stackoverflow.com/questions/77423163/i-cant-understand-the-rust-scope-definition-rust-programming-language-2nd-e 13. betterletter/[README.md](http://R...
(PUNCTUATION_PARAGRAPH_END)
[typographical] ~705-~705: To join two clauses or introduce examples, consider using an em dash.
Context: ...terletter/blob/main/README.md> 14. srgn - Rust Package Registry - [Crates.io](http...
(DASH_RULE)
[typographical] ~705-~705: To join two clauses or introduce examples, consider using an em dash.
Context: ...ME.md> 14. srgn - Rust Package Registry - Crates.io, accessed ...
(DASH_RULE)
🔍 MCP Research (1 server)
Deepwiki:
-
The
BuildTargetsstruct inrunner.rswas changed from owning aVec<String>to a lightweight wrapper over a string slice (&[String]), with corresponding lifetime annotations and method signature updates. (src/runner.rs) -
A constant
NINJA_PROGRAMwas introduced insrc/runner.rsto represent the default Ninja executable name, replacing hardcoded"ninja"string literals. (src/runner.rs) -
The
runfunction's handling of theCommands::Buildvariant was refactored by extracting the build logic into a new helper functionhandle_build. This function generates the Ninja manifest, creates aBuildTargetsinstance from a slice reference to the targets, and conditionally writes the Ninja file either to a specified path or to a newly created temporary file. Temporary file creation and writing logic was moved intocreate_temp_ninja_filereturning aNamedTempFile. The temporary file is kept alive during Ninja invocation by holding theNamedTempFilein an option tuple element. (src/runner.rs) -
The function
write_and_logwas renamed towrite_ninja_fileand its usage updated, including in theCommands::Manifestbranch and new helper functions. Thegenerate_ninjafunction was modified to use a new helperresolve_manifest_pathto determine the manifest path based on the CLI's directory option, replacing previous inline path resolution logic. (src/runner.rs)
There was a problem hiding this comment.
Actionable comments posted: 3
🔭 Outside diff range comments (4)
tests/steps/process_steps.rs (2)
20-24: Build the PATH in a cross-platform way and drop string concatenation.Use
split_paths/join_pathsto prepend the temp dir, and avoid shell-style separators.- let new_path = format!("{}:{}", dir.path().display(), original.to_string_lossy()); - // SAFETY: nightly marks `set_var` as unsafe; override path for test isolation. - unsafe { - std::env::set_var("PATH", &new_path); - } + let mut paths: Vec<_> = std::env::split_paths(&original).collect(); + paths.insert(0, dir.path().to_path_buf()); + let new_path = std::env::join_paths(paths).expect("join paths"); + std::env::set_var("PATH", &new_path);
22-24: Remove unnecessaryunsafeblock aroundstd::env::set_var.
set_varis safe; theunsafeblock is redundant and violates the repo guideline to avoidunsafeunless required.tests/runner_tests.rs (1)
54-64: Gate the shell-based Ninja test on Unix and drop the unnecessaryunsafe
- In
tests/runner_tests.rs, add#[cfg(unix)]above the#[rstest]onrun_executes_ninja_without_persisting_fileso the POSIX-shell fake Ninja won’t break on Windows.- Remove the
unsafeblock aroundstd::env::set_var("PATH", &new_path);—set_varis a safe API.- Clean up any other
unsafe { std::env::set_var(…) }occurrences in this file.- #[rstest] - #[serial] - fn run_executes_ninja_without_persisting_file() { + #[cfg(unix)] + #[rstest] + #[serial] + fn run_executes_ninja_without_persisting_file() { @@ - unsafe { - std::env::set_var("PATH", &new_path); - } + std::env::set_var("PATH", &new_path);src/runner.rs (1)
300-372: Stream Ninja output and redact sensitive args; keep error kind explicit.The subprocess orchestration is solid. Optionally log thread join errors to aid debugging instead of discarding them.
- let _ = out_handle.join(); - let _ = err_handle.join(); + if let Err(e) = out_handle.join() { + debug!("stdout forwarder thread error: {:?}", e); + } + if let Err(e) = err_handle.join() { + debug!("stderr forwarder thread error: {:?}", e); + }
♻️ Duplicate comments (2)
tests/steps/process_steps.rs (1)
111-111: Switch toBuildTargets::default()to clarify empty targets.This resolves the earlier ambiguity with
&[]and matches the refactor to a slice-backed type. LGTM.tests/runner_tests.rs (1)
43-43: Replace anonymous empty slice withBuildTargets::default().This removes hidden type inference and matches the new slice-backed type. LGTM.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (3)
src/runner.rs(8 hunks)tests/runner_tests.rs(5 hunks)tests/steps/process_steps.rs(2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.rs
📄 CodeRabbit Inference Engine (AGENTS.md)
**/*.rs: Clippy warnings MUST be disallowed.
Fix any warnings emitted during tests in the code itself rather than silencing them.
Where a function is too long, extract meaningfully named helper functions adhering to separation of concerns and CQRS.
Where a function has too many parameters, group related parameters in meaningfully named structs.
Where a function is returning a large error consider usingArcto reduce the amount of data returned.
Write unit and behavioural tests for new functionality. Run both before and after making any change.
Every module must begin with a module level (//!) comment explaining the module's purpose and utility.
Document public APIs using Rustdoc comments (///) so documentation can be generated with cargo doc.
Prefer immutable data and avoid unnecessarymutbindings.
Handle errors with theResulttype instead of panicking where feasible.
Avoidunsafecode unless absolutely necessary and document any usage clearly.
Place function attributes after doc comments.
Do not usereturnin single-line functions.
Use predicate functions for conditional criteria with more than two branches.
Lints must not be silenced except as a last resort.
Lint rule suppressions must be tightly scoped and include a clear reason.
Preferexpectoverallow.
Prefer.expect()over.unwrap().
Useconcat!()to combine long string literals rather than escaping newlines with a backslash.
Prefer semantic error enums: Derivestd::error::Error(via thethiserrorcrate) for any condition the caller might inspect, retry, or map to an HTTP status.
Use an opaque error only at the app boundary: Useeyre::Reportfor human-readable logs; these should not be exposed in public APIs.
Never export the opaque type from a library: Convert to domain enums at API boundaries, and toeyreonly in the mainmain()entrypoint or top-level async task.
Files:
tests/steps/process_steps.rstests/runner_tests.rssrc/runner.rs
⚙️ CodeRabbit Configuration File
**/*.rs: * Seek to keep the cyclomatic complexity of functions no more than 12.
Adhere to single responsibility and CQRS
Place function attributes after doc comments.
Do not use
returnin single-line functions.Move conditionals with >2 branches into a predicate function.
Avoid
unsafeunless absolutely necessary.Every module must begin with a
//!doc comment that explains the module's purpose and utility.Comments and docs must follow en-GB-oxendict (-ize / -our) spelling and grammar
Lints must not be silenced except as a last resort.
#[allow]is forbidden.- Only narrowly scoped
#[expect(lint, reason = "...")]is allowed.- No lint groups, no blanket or file-wide suppression.
- Include
FIXME:with link if a fix is expected.Use
rstestfixtures for shared setup and to avoid repetition between tests.Replace duplicated tests with
#[rstest(...)]parameterised cases.Prefer
mockallfor mocks/stubs.Prefer
.expect()over.unwrap()Ensure that any API or behavioural changes are reflected in the documentation in
docs/Ensure that any completed roadmap steps are recorded in the appropriate roadmap in
docs/Files must not exceed 400 lines in length
- Large modules must be decomposed
- Long match statements or dispatch tables should be decomposed by domain and collocated with targets
- Large blocks of inline data (e.g., test fixtures, constants or templates) must be moved to external files and inlined at compile-time or loaded at run-time.
Files:
tests/steps/process_steps.rstests/runner_tests.rssrc/runner.rs
🔍 MCP Research (1 server)
Deepwiki:
-
Document [Testing Framework]: The tests in
tests/runner_tests.rsinclude a new test verifying that the runner respects the environment variable override for the Ninja executable path (NINJA_ENV), consistent with the newresolve_ninja_programhelper function. The refactor supports testability by isolating concerns into helper functions. (tests/runner_tests.rs) -
Document [Implementation Details]: The refactor in
src/runner.rsmodularizes the build command handling by extracting manifest path resolution, Ninja file writing, temporary file creation, and Ninja executable resolution into dedicated functions. This aligns with the implementation patterns described, improving separation of concerns and code clarity without changing external behavior or error handling. The use of constants for Ninja program name and environment variable matches the documented security and configuration practices. (src/runner.rs) -
Document [Data Structures]: The change of
BuildTargetsfrom owning aVec<String>to a slice wrapper&[String]aligns with the data structure design where build targets are represented as slices or vectors of strings. This change improves efficiency and lifetime management in the build command handling. The helper functions operate on these types accordingly. (src/runner.rs) -
Document [Command Line Interface]: The CLI uses
clapto parse commands, withCommands::Buildvariant containing a vector of targets. Therunfunction insrc/runner.rsdispatches to command-specific handlers. The refactor extracts the build command logic intohandle_build(cli, targets), which processes the manifest, generates the build graph, writes the Ninja file, and invokes Ninja. The new helper functions improve clarity and maintain the existing behavior, including handling the--directoryand--fileoptions for manifest path resolution. The constantsNINJA_PROGRAMandNINJA_ENVprovide configurability for the Ninja executable path, consistent with CLI options and environment variable overrides. (src/runner.rs)
⏰ 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 (10)
tests/steps/process_steps.rs (2)
5-5: Adopt runner constants to align with refactor.Import
NINJA_PROGRAMandBuildTargetsfromrunnerto keep tests in sync with the new helpers and types. LGTM.
109-109: Use theNINJA_PROGRAMconstant instead of a string literal.Default to the module constant for maintainability and consistency with
resolve_ninja_program. LGTM.tests/runner_tests.rs (2)
2-2: ImportNINJA_ENVfor environment override tests.Align tests with the new configurability. LGTM.
96-96: Use the build-file-checking fake Ninja to validate-fhandling.This strengthens the test by verifying the temporary/emitted build file path is real. LGTM.
src/runner.rs (6)
19-23: IntroduceNINJA_PROGRAMandNINJA_ENVfor program resolution.Expose clear defaults and an env override to meet configurability and security guidance. LGTM.
82-86: Usewrite_ninja_filein Manifest branch.Unify file writing with logging across code paths. LGTM.
144-152: Create and write temp Ninja files via a dedicated helper.Use of
tempfile::Builderwith.prefix/.suffixand contextual errors meets clarity and testability goals. LGTM.
193-200: Resolve manifest path via helper ingenerate_ninja.Remove ad-hoc joins, centralise path logic, and keep logs informative. LGTM.
211-216: Provide a single place for manifest path resolution.Keep
resolve_manifest_pathprivate and documented; the example is markedignore, avoiding doctest issues. LGTM.
220-222: Resolve Ninja program via env override with a sane default.Respect
$NETSUKE_NINJAwhile preserving default behaviour. LGTM.
1 participant
Summary
handle_buildhelperrunshallow for easier maintenanceTesting
cargo fmt --allmake lintmake testmake fmt(fails: docs/srgn.md:99 MD040/fenced-code-language Fenced code blocks should have a language specified)closes #47
https://chatgpt.com/codex/tasks/task_e_6895d788cd8883229d612eac1cd7e13c
Summary by Sourcery
Refactor the CLI’s build handling by extracting the build orchestration and file operations into dedicated helper functions to simplify the run dispatcher and improve maintainability.
Enhancements:
Documentation: