Skip to content

Align v0.1.0 documentation and test every example - #427

Merged
leynos merged 23 commits into
mainfrom
docs/users-guide-example-coverage
Jul 26, 2026
Merged

Align v0.1.0 documentation and test every example#427
leynos merged 23 commits into
mainfrom
docs/users-guide-example-coverage

Conversation

@leynos

@leynos leynos commented Jul 23, 2026

Copy link
Copy Markdown
Owner

Summary

This branch gives early adopters an accurate account of Netsuke v0.1.0 and
prevents its user-facing examples from drifting away from executable
behaviour.

It replaces outdated product claims with an honest completion assessment and
road ahead, reorganizes the user's guide around real tasks, corrects linked
example manifests, and adds Rust integration, rstest-bdd, end-to-end, and
property coverage for the README and user's guide examples.

Review follow-ups align the configured default with hello.txt, correct the
photo-edit and writing outputs, and reject unsupported reusable-rule deps
instead of accepting and silently dropping them. Target and action
dependencies remain the supported implicit-dependency contract until the
planned rule-level deps_from feature is implemented.

Review walkthrough

Validation

  • Rebasing: clean replay onto origin/main at 2afbcb6; range-diff found
    one whitespace-only replay artefact, removed in 764e869
  • Documentation-loader properties: valid, duplicate-identifier, and
    unterminated-fence contracts passed; a deliberate duplicate-rejection
    mutation failed and shrank to the minimal input
  • make check-fmt: passed
  • make test: passed, 1,150 tests with no failures or skips
  • make typecheck: passed
  • make lint: passed, including Rustdoc, Clippy, and Whitaker
  • make markdownlint: passed, 69 files with zero errors
  • make nixie: passed
  • git diff --check: passed

Notes

Each manifest example runs in an isolated temporary workspace. The documented
first-run flows execute with real Ninja and assert the exact cat hello.txt
output; the rstest-bdd scenarios retain the novice-flow fake-Ninja process
boundary. Deterministic child-only stubs exercise the photo-edit and writing
manifests without requiring Darktable, Pandoc, or LaTeX.

The CLI rewrite requested in later review feedback was not applied: the live
parser still exposes manifest, build --emit, --diag-json, and the current
presentation flags, and does not expose generate, root --no-input, or
canonical --json. Changing the guide alone would make it inaccurate.

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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

Please try again later or upgrade to continue using Sourcery

@coderabbitai

coderabbitai Bot commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

Summary

  • Aligned README, user guide, developer guidance, examples, ADRs and design documents with Netsuke v0.1.0 behaviour.
  • Reworked the CLI around generate, removing the retired manifest command and build --emit; added policy-based output controls, canonical --json, non-interactive validation, and NETSUKE_CONFIG precedence.
  • Added versioned JSON result envelopes for successful build, clean, generate and graph commands, with diagnostics remaining on stderr and help/version passthrough preserved.
  • Rejected unsupported rule-level deps during manifest deserialisation.
  • Added executable documentation coverage, loader/property tests, BDD scenarios, isolated workspaces and Unix Ninja end-to-end tests for README, user-guide and example workflows.
  • Refactored shared test helpers, capability-scoped filesystem access and JSON assertions; fixed stderr-thread cleanup on process-wait failures.
  • Updated the CLI design documentation, ADR-004, Netsuke CLI design, Netsuke design and roadmap to record the implemented contracts.

Validation passed for 1,150 tests, formatting, type checking, linting, Markdown and Nixie validation, and whitespace checks.

Walkthrough

Netsuke’s v0.1.0 workflow now uses generate, typed output policies, unified JSON results and diagnostics, narrowed configuration selection, updated manifest rules, refreshed examples, and executable documentation contract tests.

Changes

v0.1.0 CLI and build workflow

Layer / File(s) Summary
Canonical CLI and configuration contracts
src/cli/*, src/main.rs, src/locale_resolution.rs, src/ast.rs
Replace legacy flags and selectors with typed policies, --json, --no-input, generate, NETSUKE_CONFIG, and explicit rejection of rule-level deps.
Command dispatch and JSON output
src/runner/*, src/json_envelope.rs, src/result_json.rs
Route commands through dispatchers with versioned JSON success documents and JSON-aware stream handling.
Documentation and examples
README.md, docs/*, examples/*, locales/*
Update release documentation, localized help, configuration guidance, and example manifests for the revised CLI and build flows.
Validation coverage
tests/*, .github/workflows/netsukefile-test.yml, build.rs
Align unit, BDD, integration, property, end-to-end, workflow, and documentation tests with the new contracts.

Possibly related issues

  • leynos/actix-v2a#53 — Documentation-example tests cover manifest-time branching and command-selection behaviour related to the issue.

Possibly related PRs

Suggested labels: Roadmap

Suggested reviewers: codescene-access, codescene-delta-analysis

Poem

YAML winds through Jinja bright,
Ninja graphs emerge in flight.
JSON speaks when streams grow still,
Policies shape each build and drill.
Docs and tests now guard the way.


Caution

Pre-merge checks failed

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

  • Ignore

❌ Failed checks (3 errors, 1 inconclusive)

Check name Status Explanation Resolution
Testing (Overall) ❌ Error src/runner/dispatch.rs adds a new cli.json && output.is_some() generate branch, but tests cover --json generate and generate --output separately, not the combined path. Add an integration/BDD test for netsuke --json generate --output <file> that asserts the file is written and stdout carries the JSON result envelope.
Module-Level Documentation ❌ Error tests/cli_tests/parsing.rs only says 'CLI parsing coverage.', which does not clearly explain the module’s purpose or its link to the parser layer. Expand the module doc in tests/cli_tests/parsing.rs to state that it exercises the Clap-facing CLI parser and merged CLI wiring, not just 'coverage'.
Rust Compiler Lint Integrity ❌ Error build.rs adds module-wide #[expect(dead_code, unused_imports)] on shared source modules, hiding real unused-code diagnostics instead of narrowing the build-script surface. Replace the broad expectations with a smaller build-script module boundary or a dedicated helper that imports only the symbols it actually uses.
Architectural Complexity And Maintainability ❓ Inconclusive placeholder Need to inspect architecture-relevant files before verdict.
✅ Passed checks (16 passed)
Check name Status Explanation
Title check ✅ Passed The title matches the PR’s main scope: aligning v0.1.0 docs and adding example coverage tests.
Description check ✅ Passed The description is clearly related and summarises the documentation, example, and test changes.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
User-Facing Documentation ✅ Passed docs/users-guide.md documents the new CLI, config, JSON, and safety behaviour; README signposts the core flow; en-US and es-ES locale keys stay aligned.
Developer Documentation ✅ Passed Developer guide, design docs, ADR, roadmap, execplan, and both locales are in sync with the changed CLI/config/runtime boundaries; roadmap items are checked off.
Testing (Unit And Behavioural) ✅ Passed PASS: The PR adds real CLI/E2E tests (assert_cmd, docs, logging) and unit tests cover parser/merge/AST edge cases, failures, and invariants.
Testing (Property / Proof) ✅ Passed PASS: The PR adds proptest/Kani coverage for the new invariants—documented-example parsing, config precedence, Ninja resolution, and cycle/JSON contracts—matching the check.
Testing (Compile-Time / Ui) ✅ Passed PASS: compile-time coverage already uses trybuild in tests/kani_cfg_ui_tests.rs, and the UI/text output changes are guarded by insta snapshots plus focused JSON/string assertions.
Unit Architecture ✅ Passed PASS: command-side effects stay at explicit edges; path/config resolvers remain read-only, and dispatch/result JSON helpers make publication boundaries visible.
Domain Architecture ✅ Passed Rule now models only domain fields, deps is rejected at the parse boundary, and env/fs/CLI concerns stay in manifest/runner adapters.
Observability ✅ Passed PASS: runtime paths add structured tracing at subprocess boundaries, context-rich file I/O errors, and JSON error/result envelopes for startup/runtime failures.
Security And Privacy ✅ Passed Keep it green: the diff adds no secrets or auth changes; JSON/generate paths only emit user-selected artefacts, and new tests use temp workspaces and fake Ninja stubs.
Performance And Resource Use ✅ Passed PASS: The patch only factors duplicate JSON test setup; it adds no unbounded loops, heavy allocations, or hot-path I/O regressions.
Concurrency And State ✅ Passed PASS: stderr forwarding now joins on all exit paths, and a regression test covers the wait() failure path; no unsynchronised shared state was introduced.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch docs/users-guide-example-coverage

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

codescene-access[bot]

This comment was marked as outdated.

codescene-access[bot]

This comment was marked as outdated.

codescene-access[bot]

This comment was marked as outdated.

codescene-access[bot]

This comment was marked as outdated.

@leynos
leynos marked this pull request as ready for review July 23, 2026 16:32

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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

Please try again later or upgrade to continue using Sourcery

chatgpt-codex-connector[bot]

This comment was marked as resolved.

coderabbitai[bot]

This comment was marked as resolved.

codescene-access[bot]

This comment was marked as outdated.

@buzzybee-df12

Copy link
Copy Markdown
Collaborator

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jul 24, 2026

Copy link
Copy Markdown
Contributor
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

coderabbitai[bot]

This comment was marked as resolved.

codescene-access[bot]

This comment was marked as outdated.

codescene-access[bot]

This comment was marked as outdated.

codescene-access[bot]

This comment was marked as outdated.

codescene-access[bot]

This comment was marked as outdated.

codescene-access[bot]

This comment was marked as outdated.

@lodyai
lodyai Bot force-pushed the docs/users-guide-example-coverage branch from f121d24 to 764e869 Compare July 24, 2026 07:15
codescene-access[bot]

This comment was marked as outdated.

codescene-access[bot]

This comment was marked as outdated.

@leynos

leynos commented Jul 24, 2026

Copy link
Copy Markdown
Owner Author

@coderabbitai resume

@coderabbitai

coderabbitai Bot commented Jul 24, 2026

Copy link
Copy Markdown
Contributor
✅ Action performed

Reviews resumed.

coderabbitai[bot]

This comment was marked as resolved.

codescene-access[bot]

This comment was marked as outdated.

codescene-access[bot]

This comment was marked as outdated.

@leynos

leynos commented Jul 24, 2026

Copy link
Copy Markdown
Owner Author

@coderabbitai Please suggest a fix for this issue and supply a prompt for an AI coding agent to enable it to apply the fix. Include the file and symbol names indicated in the issue at the head of your response. Ensure that this is validated against the current version of the codegraph.

If further refinement to address this finding would be deleterious, please supply a clear explanatory one to two paragraph markdown message I can paste into the CodeScene web ui's diagnostic suppression function so this diagnostic can be silenced.

tests/assert_cmd_tests.rs

Comment on lines +49 to +52

fn generate_streams_to_stdout_by_default() -> Result<()> {
    let temp = setup_simple_workspace("generate stdout test")?;
    let output = create_netsuke_command(temp.path())
        .arg("generate")

❌ New issue: Code Duplication
The module contains 2 functions with similar structure: generate_streams_to_stdout_by_default,generate_streams_to_stdout_with_directory

@leynos

leynos commented Jul 24, 2026

Copy link
Copy Markdown
Owner Author

@coderabbitai Please suggest a fix for this issue and supply a prompt for an AI coding agent to enable it to apply the fix. Include the file and symbol names indicated in the issue at the head of your response. Ensure that this is validated against the current version of the codegraph.

If further refinement to address this finding would be deleterious, please supply a clear explanatory one to two paragraph markdown message I can paste into the CodeScene web ui's diagnostic suppression function so this diagnostic can be silenced.

tests/bdd/steps/accessibility_preferences.rs

Comment on file

fn simulated_env(world: &TestWorld) -> impl Fn(&str) -> Option<String> + '_ {
    move |key| match key {
        "NO_COLOR" => world.simulated_no_color.get(),
        "NETSUKE_NO_EMOJI" => world.simulated_no_emoji.get(),

❌ New issue: Code Duplication
The module contains 2 functions with similar structure: prefix_has_non_ascii,prefix_is_ascii

codescene-access[bot]

This comment was marked as outdated.

Extract a shared `assert_captured_stream_is_empty` helper from the
`stdout should be empty` and `stderr should be empty` step definitions. The
helper takes the stream name and the captured `Option<&String>`, fails with
`<stream> should be captured` when nothing was recorded, and preserves the
`expected <stream> to be empty, got:\n<output>` diagnostic. Both `#[then]`
bindings are retained so the feature files keep specifying distinct stdout and
stderr contracts.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
codescene-access[bot]

This comment was marked as outdated.

Extract a Unix-only `assert_json_ninja_subcommand_success` helper that runs
`netsuke --json <command>` with `NINJA_ENV` pointed at a fake Ninja, asserts the
success envelope via the existing `parse_success_result`, and checks
`/result/content` is null. `json_success_build_keeps_stdout_result_and_stderr_empty`
and `json_success_clean_invokes_clean_tool_with_empty_stderr` remain as focused
wrappers, each retaining its distinct fake-Ninja fixture (valid build-file input
for build, `ninja -t clean` dispatch for clean) so the two contracts stay
separately proven. No production code, schema assertions, or unrelated tests
change.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
codescene-access[bot]

This comment was marked as outdated.

codescene-access[bot]

This comment was marked as outdated.

codescene-access[bot]

This comment was marked as outdated.

@leynos

leynos commented Jul 26, 2026

Copy link
Copy Markdown
Owner Author

@coderabbitai Please suggest a fix for this issue and supply a prompt for an AI coding agent to enable it to apply the fix. Include the file and symbol names indicated in the issue at the head of your response. Ensure that this is validated against the current version of the codegraph.

If further refinement to address this finding would be deleterious, please supply a clear explanatory one to two paragraph markdown message I can paste into the CodeScene web ui's diagnostic suppression function so this diagnostic can be silenced.

tests/logging_stderr/json.rs

Comment on file

//! JSON diagnostic, result-envelope, and standard stderr integration tests.

❌ New issue: String Heavy Function Arguments
In this module, 41.7% of all arguments to its 11 functions are strings. The threshold for string arguments is 39.0%

@coderabbitai

coderabbitai Bot commented Jul 26, 2026

Copy link
Copy Markdown
Contributor

Tip

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

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '== Current PR head =='
git rev-parse --short HEAD

printf '%s\n' '== JSON logging module outline =='
ast-grep outline tests/logging_stderr/json.rs --items all --view expanded

printf '%s\n' '== Function signatures, string parameters, and call contexts =='
rg -n -A45 -B10 --type rust \
  '^\s*(?:#\[[^]]+\]\s*)*(?:pub\(super\)\s+)?fn\s+\w+\s*\([^)]*(?:&str|String|OsStr|OsString)[^)]*\)|\b(parse_success_result|assert_json_ninja_subcommand_success)\s*\(' \
  tests/logging_stderr/json.rs || true

printf '%s\n' '== Full target source with line numbers =='
cat -n tests/logging_stderr/json.rs

printf '%s\n' '== Existing CodeScene suppressions/configuration =='
fd -HI -t f -E .git . | rg -n '(codescene|code.?health|suppress|\.codescene)' || true
rg -n -i -C3 'codescene|String Heavy Function Arguments|string-heavy' \
  .github docs .codescene.yml coderabbit.yaml 2>/dev/null || true

Length of output: 29312


@leynos tests/logging_stderr/json.rsparse_success_result, assert_json_ninja_subcommand_success, assert_json_passthrough, json_passthrough_uses_normal_clap_output

Suppress this CodeScene diagnostic. The current codegraph at PR head 2a2c88d confirms that the string parameters are intentional test-boundary values: CLI command/flag tokens ("build", "clean", "--help"), expected JSON command names, expected human-readable output markers, and context used only in assertion failures. They directly express the externally observable CLI and JSON contracts under test.

Do not introduce test-only newtypes, enums, or wrapper structs merely to lower this metric. Such a refactor would relocate fixed literals without strengthening type safety or reducing behavioural duplication, while making the concise parameterized Clap pass-through cases harder to read. The repository guidance on typed wrappers targets ambiguous BDD step parameters; these integration-test arguments are unambiguous and deliberately map to command-line and JSON string interfaces.

@leynos

leynos commented Jul 26, 2026

Copy link
Copy Markdown
Owner Author

@coderabbitai Have the following failed checks now been resolved?

If further work is required, please provide an AI agent prompt for the remaining work to be done to address these failures.

Do not treat warnings as optional or aspirational. Where a change is out of scope for this PR, propose a GitHub issue unless one exists already. (Treat documentation and validation coverage as in scope).

❌ Failed checks (1 error, 2 warnings)

Check name Status Explanation Resolution
Rust Compiler Lint Integrity ❌ Error build.rs keeps const _: fn symbol-audit anchors for live APIs, which are artificial references the check forbids. Remove the symbol-audit anchors or move them into targeted compile tests that do not hide unused-code signals; rely on real call sites instead.
Developer Documentation ⚠️ Warning The guide covers new APIs, but the design docs still show legacy --file, manifest, --diag-json and progress: Option<bool> examples, so the contract is not coherent. Remove or relabel the stale examples in docs/netsuke-design.md and docs/netsuke-cli-design-document.md, then align the CLI, output-policy, and JSON sections with the current contract.
Concurrency And State ⚠️ Warning spawn_and_stream_output only joins the stderr thread after child.wait()?, so a wait failure detaches background work with no cleanup/test coverage. Join the thread on every exit path (e.g. guard or scoped thread), and add a failure-path test for wait()/shutdown to prove cleanup.

@coderabbitai

coderabbitai Bot commented Jul 26, 2026

Copy link
Copy Markdown
Contributor

Tip

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

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '== Current PR head =='
git rev-parse --short HEAD

printf '%s\n' '== Roadmap task 3.15.4 =='
rg -n -A45 -B8 '3\.15\.4|color|emoji|progress|accessibility|OrthoConfig' docs/roadmap.md || true

printf '%s\n' '== Configuration-discovery step outline =='
ast-grep outline tests/bdd/steps/configuration_discovery.rs --items all --type function --view expanded

printf '%s\n' '== write_config_file implementation and filesystem APIs =='
rg -n -A55 -B15 --type rust \
  '^\s*use\s+(std::fs|std::path|cap_std)|^\s*fn\s+write_config_file\b|\b(fs::write|Dir::open_ambient_dir|\.write\(|std::fs)' \
  tests/bdd/steps/configuration_discovery.rs || true

printf '%s\n' '== Scenario headings in both feature files =='
awk '/^Scenario:/ { print FILENAME ":" FNR ":" $0 }' \
  tests/features/json_diagnostics.feature tests/features/advanced_usage.feature

printf '%s\n' '== JSON-generate scenario bodies and result assertions =='
rg -n -A16 -B5 \
  '(--json[[:space:]]+generate|generate[[:space:]]+--json|one generate result json document|JSON diagnostics with generate)' \
  tests/features/json_diagnostics.feature tests/features/advanced_usage.feature || true

Length of output: 31023


@leynos Mark all three findings as resolved at current PR head 2a2c88d.

  1. docs/roadmap.md — task 3.15.4: resolved.
    Lines 284-291 record the delivered color, progress, emoji, and accessibility policy changes as complete, including configuration, environment, localization, and test integration. Leave the parent task unchecked: Line 292 correctly identifies the genuinely outstanding OrthoConfig dependency.

  2. tests/bdd/steps/configuration_discovery.rswrite_config_file: resolved.
    The helper uses cap_std::{ambient_authority, fs_utf8::Dir} (Line 7), opens the temporary workspace through Dir::open_ambient_dir (Line 35), and writes through Dir::write (Line 37). Direct std::fs access is absent. The remaining std::env::set_current_dir only manages process-global working-directory state and does not violate the filesystem-access requirement.

  3. tests/features/json_diagnostics.feature / tests/features/advanced_usage.feature: resolved.
    tests/features/json_diagnostics.feature retains the canonical successful --json generate scenario at Lines 11-16. The current codegraph contains no matching JSON-generate scenario or result assertion in tests/features/advanced_usage.feature, so the duplicate has been removed.

@coderabbitai

coderabbitai Bot commented Jul 26, 2026

Copy link
Copy Markdown
Contributor

Tip

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

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '== Current PR head =='
git rev-parse --short HEAD

printf '%s\n' '== Target function outlines =='
ast-grep outline tests/assert_cmd_tests.rs --items all --type function --match 'generate_writes_file' --view expanded
ast-grep outline tests/bdd/steps/json_diagnostics.rs --items all --type function --match 'parse_single_result_document' --view expanded

printf '%s\n' '== generate_writes_file and predicate imports =='
rg -n -A50 -B15 --type rust \
  '^\s*fn\s+generate_writes_file\b|^\s*use\s+predicates|predicate::str::is_empty|\.stdout\s*\(' \
  tests/assert_cmd_tests.rs || true

printf '%s\n' '== parse_single_result_document =='
rg -n -A90 -B15 --type rust \
  '^\s*fn\s+parse_single_result_document\b|result JSON should contain only|object-valued result|schema_version' \
  tests/bdd/steps/json_diagnostics.rs || true

printf '%s\n' '== JSON design-contract wording =='
rg -n -A16 -B12 \
  '(exactly one|one versioned|one JSON|--json --help|--json --version|Clap|envelope-producing|result/diagnostic|NETSUKE_JSON)' \
  docs/netsuke-cli-design-document.md || true

printf '%s\n' '== Executable coverage of JSON help/version behaviour =='
rg -n -A16 -B8 --type rust \
  '(--json.*(--help|--version)|(--help|--version).*--json|normal Clap|Clap output)' \
  tests/logging_stderr tests 2>/dev/null || true

Length of output: 35866


@leynos Mark all three findings as resolved at current PR head 2a2c88d.

  1. tests/assert_cmd_tests.rsgenerate_writes_file: resolved.
    The non-JSON generate --output assertion chain now includes .stdout(predicate::str::is_empty()) on Line 66, while preserving the generated-file existence and Ninja-content checks.

  2. tests/bdd/steps/json_diagnostics.rsparse_single_result_document: resolved.
    The helper now requires a result branch, rejects a diagnostics branch, verifies that result is an object, and then validates its command. This rejects mixed or malformed success envelopes.

  3. docs/netsuke-cli-design-document.md — root --json contract: resolved.
    The document limits the one-document guarantee to commands that render Netsuke result or diagnostic envelopes. It explicitly documents that netsuke --json --help and netsuke --json --version retain ordinary human-readable Clap output. The same qualification appears in the machine-readable output, NETSUKE_JSON, configuration, and help-text sections.

codescene-access[bot]

This comment was marked as outdated.

@coderabbitai

coderabbitai Bot commented Jul 26, 2026

Copy link
Copy Markdown
Contributor

Tip

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

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '== Current PR head =='
git rev-parse --short HEAD

printf '%s\n' '== build.rs symbol-audit structure =='
ast-grep outline build.rs --items all --view expanded
rg -n -A100 -B15 --type rust \
  'verify_public_api_symbols|const\s+_\s*:\s*fn|size_of|symbol.audit|public.api' \
  build.rs

printf '%s\n' '== Current production references for audited CLI APIs =='
rg -n -C3 --type rust \
  '\b(resolve_merged_json|json_hint_from_args|AccessibilityPolicy|EmojiPolicy|ProgressPolicy|theme_preference|accessibility_override)\b' \
  src tests build.rs || true

printf '%s\n' '== Legacy CLI/doc-contract references =='
rg -n -i -C3 \
  '(--file\b|\bmanifest\b|--diag-json\b|progress\s*:\s*Option\s*<\s*bool\s*>)' \
  docs/netsuke-design.md docs/netsuke-cli-design-document.md || true

printf '%s\n' '== Current JSON/help/version contract wording =='
rg -n -i -C3 \
  '(--json\b|envelope|Clap|--help|--version|NETSUKE_JSON|generate\b|progress)' \
  docs/netsuke-design.md docs/netsuke-cli-design-document.md || true

printf '%s\n' '== Process streaming structure and wait/join paths =='
ast-grep outline src/runner/process/mod.rs --items all --type function --match 'spawn_and_stream_output|run_ninja_internal' --view expanded
rg -n -A180 -B30 --type rust \
  '^\s*fn\s+spawn_and_stream_output\b|child\.wait\(|\.join\(\)|JoinHandle|stderr_thread|spawn\(' \
  src/runner/process/mod.rs

printf '%s\n' '== Process streaming tests and test seams =='
rg -n -A30 -B15 --type rust \
  '(spawn_and_stream_output|run_command_and_stream|child\.wait|wait.*fail|JoinHandle|stderr.*join|stream.*output)' \
  src tests || true

Length of output: 2747


🏁 Script executed:

#!/bin/bash
set -u

printf '%s\n' '== build.rs: symbol audit and anchors =='
rg -n -A140 -B12 \
  'verify_public_api_symbols|const[[:space:]]+_[[:space:]]*:[[:space:]]*fn|size_of|audit' \
  build.rs || true

printf '%s\n' '== build.rs complete lower section (for anchor context) =='
nl -ba build.rs | sed -n '1,280p' || true

printf '%s\n' '== Legacy terms in both design documents =='
rg -n -i -C2 \
  -- '--file|--diag-json|progress[[:space:]]*:[[:space:]]*Option[[:space:]]*<[[:space:]]*bool[[:space:]]*>|\bmanifest\b' \
  docs/netsuke-design.md docs/netsuke-cli-design-document.md || true

printf '%s\n' '== Process module: streaming implementation =='
rg -n -A210 -B25 --type rust \
  'fn[[:space:]]+spawn_and_stream_output|child\.wait\(|stderr_thread|\.join\(' \
  src/runner/process/mod.rs || true

printf '%s\n' '== Tests covering stream/wait cleanup =='
rg -n -A25 -B10 --type rust \
  'spawn_and_stream_output|child\.wait|wait.*(error|fail)|stderr_thread|join.*thread|stream.*output' \
  src tests || true

Length of output: 129499


@leynos Treat two checks as resolved and Developer Documentation as unresolved at current PR head 2a2c88d.

  1. Rust Compiler Lint Integrity — resolved.
    build.rs no longer contains verify_public_api_symbols, const _: fn anchors, or size_of-based artificial API references. Its remaining build-time work is man-page generation and localization-key auditing.

  2. Concurrency And State — resolved.
    src/runner/process/mod.rs::spawn_and_stream_output now stores child.wait() in wait_result and passes it to finalize_streaming. finalize_streaming joins the stderr forwarding thread before returning wait_result, including when wait() fails. src/runner/process/tests.rs::finalize_streaming_joins_stderr_thread_when_wait_fails explicitly verifies that cleanup completes before the simulated wait failure propagates.

  3. Developer Documentation — unresolved.
    The current design documents still present retired CLI interfaces as current behaviour:

    • docs/netsuke-design.md contains the obsolete Cli.file, Commands::Manifest, BuildArgs.emit, and netsuke build [--emit FILE] model.
    • docs/netsuke-cli-design-document.md still directs users to --file / -f and includes -f, --file <FILE> in the sample help output.

Address this in the PR; documentation and validation coverage are in scope, so no follow-up issue is required.

AI coding-agent prompt:

Modify the current PR-head documentation to remove or clearly relabel obsolete
CLI-contract material. Work only in:

- docs/netsuke-design.md
- docs/netsuke-cli-design-document.md

Inspect the actual CLI implementation and current user documentation first,
especially:
- src/cli/parser.rs
- src/cli/config.rs
- src/cli/merge.rs
- docs/users-guide.md
- README.md
- tests/cli_tests/parsing.rs
- tests/logging_stderr/json.rs

The current contract uses:
- root manifest selection through the current Netsukefile behaviour, not `-f` /
  `--file`;
- `generate [--output <PATH>]`, with no `manifest` subcommand;
- `build` without `--emit`;
- root `--json`, not `--diag-json`;
- typed `color`, `emoji`, `progress`, and `accessibility` policies, not
  `progress: Option<bool>` or the retired output-policy interface;
- `--json --help` and `--json --version` retaining ordinary Clap output rather
  than Netsuke JSON envelopes.

Required changes:

1. In docs/netsuke-design.md:
   - Replace the obsolete example CLI definition containing `Cli.file`,
     `Commands::Manifest`, and `BuildArgs.emit`.
   - Replace command prose describing `manifest` and `build --emit`.
   - Document `generate --output <FILE>` as the file-output path and bare
     `generate` as stdout generation, matching the implemented behaviour.
   - Replace all remaining `--diag-json` examples with `--json`.
   - Replace obsolete output-policy examples, including
     `progress: Option<bool>`, with the current typed policy model.
   - Retain historical discussion only where it is explicitly marked as
     superseded historical design material; do not present it as the current
     public contract.

2. In docs/netsuke-cli-design-document.md:
   - Remove user guidance to pass `--file` / `-f`, including the missing-manifest
     recovery advice and the sample `--help` option line.
   - Ensure all subcommand descriptions use `generate`, never `manifest`.
   - Remove `build --emit` and diagnostic-JSON terminology/examples.
   - Align JSON, output-policy, configuration, and help-text sections with the
     current contract above.
   - Preserve the already documented exception that `--json --help` and
     `--json --version` produce ordinary human-readable Clap output.

3. Search both files before finishing for every occurrence of:
   - `--file`
   - `-f`
   - `manifest` when used as a CLI subcommand
   - `--emit`
   - `--diag-json`
   - `progress: Option<bool>`
   - retired output-policy field and flag names

   Update each current-contract occurrence. Keep legitimate references to a
   Netsuke manifest/Netsukefile and manifest-processing concepts unchanged.

4. Keep the edits concise and internally consistent. Do not change runtime
   code merely to match obsolete prose.

5. Run the repository’s available Markdown/documentation validation and the
   focused documentation-example tests. Report the exact commands and results.

@codescene-access codescene-access Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

No quality gates enabled for this code.

@leynos

leynos commented Jul 26, 2026

Copy link
Copy Markdown
Owner Author

@coderabbitai are these features actually implemented?

  • Developer Documentation — unresolved.
    The current design documents still present retired CLI interfaces as current behaviour:

    • docs/netsuke-design.md contains the obsolete Cli.file, Commands::Manifest, BuildArgs.emit, and netsuke build [--emit FILE] model.
    • docs/netsuke-cli-design-document.md still directs users to --file / -f and includes -f, --file <FILE> in the sample help output.

@coderabbitai

coderabbitai Bot commented Jul 26, 2026

Copy link
Copy Markdown
Contributor

Tip

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

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '== Current PR head =='
git rev-parse --short HEAD

printf '%s\n' '== CLI parser outline =='
ast-grep outline src/cli/parser.rs --items all --view expanded

printf '%s\n' '== Active CLI definitions and legacy identifiers =='
rg -n -A18 -B8 --type rust \
  'pub struct Cli|pub enum Commands|pub struct BuildArgs|Cli\.file|Commands::Manifest|BuildArgs\s*\{[^}]*emit|emit:|file:|Generate|Manifest|--emit|--file' \
  src/cli src/runner tests/cli_tests tests/assert_cmd_tests.rs || true

printf '%s\n' '== Clap-facing argument annotations and command dispatch =='
rg -n -A14 -B6 --type rust \
  '#\[arg\(|#\[command\(|Commands::(Generate|Manifest)|BuildArgs' \
  src/cli/parser.rs src/cli/merge.rs src/runner || true

printf '%s\n' '== Current CLI parsing acceptance/rejection coverage =='
rg -n -A12 -B6 --type rust \
  '(generate|manifest|--emit|--file|-f\b|UnknownArgument)' \
  tests/cli_tests/parsing.rs tests/features/cli.feature || true

printf '%s\n' '== Named design-document legacy occurrences =='
rg -n -i -C3 -- \
  '--file\b|-f\b|--emit\b|--diag-json\b|progress[[:space:]]*:[[:space:]]*Option[[:space:]]*<[[:space:]]*bool[[:space:]]*>|Commands::Manifest|Cli\.file|BuildArgs\.emit|build[[:space:]]+\[--emit' \
  docs/netsuke-design.md docs/netsuke-cli-design-document.md || true

Length of output: 104532


@leynos Yes—but only partially. My prior finding was inaccurate because it incorrectly classified --file as retired.

At current PR head 2a2c88d:

Interface Implemented? Evidence
Cli.file Yes src/cli/parser.rs::Cli has pub file: PathBuf.
-f / --file <FILE> Yes The field has Clap short and long annotations; parsing tests and BDD scenarios cover it.
Commands::Manifest / netsuke manifest No Commands contains Generate, not Manifest; manifest - is tested as an invalid subcommand.
BuildArgs.emit / netsuke build --emit FILE No BuildArgs contains only targets; build --emit out.ninja is tested as an unknown argument.
netsuke generate [--output FILE] Yes Commands::Generate { output: Option<PathBuf> } is parsed and dispatched.

Therefore, retain documentation for the live root-level manifest selector:

netsuke --file alternate.yml generate --output build.ninja

Do not remove Cli.file, --file, or -f from the design documents. Remove or relabel only the genuinely obsolete material:

  • Commands::Manifest / the manifest subcommand;
  • build --emit FILE;
  • BuildArgs.emit and configuration prose claiming that emit can be supplied through [cmds.build].

docs/netsuke-design.md is definitely stale where it presents netsuke build [--emit FILE] as current behaviour. docs/netsuke-cli-design-document.md is not stale merely because it documents --file / -f; that documentation matches the current parser.

🐇🔎


✏️ Learnings added
Learnt from: leynos
URL: https://github.com/leynos/netsuke/pull/427

Timestamp: 2026-07-26T22:26:56.837Z
Learning: At PR head `2a2c88d`, Netsuke's root CLI still supports the manifest-path selector `Cli.file` through Clap `-f` / `--file <FILE>`. The `manifest` subcommand and `build --emit` are retired: `Commands` exposes `Generate { output: Option<PathBuf> }`, `BuildArgs` has only `targets`, and CLI parsing tests reject `manifest -` and `build --emit`.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

@leynos
leynos merged commit a1b694b into main Jul 26, 2026
20 checks passed
@leynos
leynos deleted the docs/users-guide-example-coverage branch July 26, 2026 22:27
@buzzybee-df12

Copy link
Copy Markdown
Collaborator

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jul 26, 2026

Copy link
Copy Markdown
Contributor
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 3

Caution

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

⚠️ Outside diff range comments (1)
src/runner/process/mod.rs (1)

222-222: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Update the stale diagnostics-only terminology.

request.cli.json now selects unified JSON mode, but the nearby forwarding comment still describes “JSON diagnostics mode”. Rename that wording to “JSON mode” so the suppression contract matches the value passed here.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/runner/process/mod.rs` at line 222, Update the nearby forwarding comment
associated with request.cli.json to say “JSON mode” instead of “JSON diagnostics
mode,” preserving the existing suppression contract and code behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@build.rs`:
- Around line 11-14: Replace std::fs and std::path usage in build.rs with
capability-scoped I/O: add cap-std and camino under [build-dependencies], and
use cap_std::fs_utf8::Dir with camino::Utf8PathBuf to stage the man page. In
tests/assert_cmd_tests.rs, replace std::fs::read_to_string with reads through a
capability-scoped handle, preserving the existing generated-file assertions.

In `@docs/netsuke-design.md`:
- Around line 2426-2428: Update the surrounding CLI model in
docs/netsuke-design.md to consistently remove the retired generate command
contract: revise or remove the related Commands::Manifest schema and behavior
examples, and eliminate the build --emit surface where it represents the same
retired workflow. Ensure the document no longer publishes both command
interfaces while preserving unrelated command documentation.
- Around line 2277-2282: Update the JSON output contract in the documented
automation behavior to guarantee exactly one JSON document only when JSON mode
has been successfully resolved. Explicitly exclude early-startup
configuration-file failures that occur before JSON mode can be determined, while
preserving the existing success and failure document requirements for resolved
JSON mode.

---

Outside diff comments:
In `@src/runner/process/mod.rs`:
- Line 222: Update the nearby forwarding comment associated with
request.cli.json to say “JSON mode” instead of “JSON diagnostics mode,”
preserving the existing suppression contract and code behavior.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 4a9881d9-6188-4750-9954-e17caab3e966

📥 Commits

Reviewing files that changed from the base of the PR and between 5789ea9 and 2a2c88d.

📒 Files selected for processing (21)
  • build.rs
  • docs/developers-guide.md
  • docs/execplans/netsuke-cli-overhaul.md
  • docs/netsuke-cli-design-document.md
  • docs/netsuke-design.md
  • docs/roadmap.md
  • docs/users-guide.md
  • src/json_envelope.rs
  • src/runner/process/mod.rs
  • src/runner/process/tests.rs
  • tests/assert_cmd_tests.rs
  • tests/bdd/steps/configuration_discovery.rs
  • tests/bdd/steps/configuration_preferences.rs
  • tests/bdd/steps/json_diagnostics.rs
  • tests/bdd/steps/progress_output.rs
  • tests/features/advanced_usage.feature
  • tests/features/json_diagnostics.feature
  • tests/logging_stderr/json.rs
  • tests/logging_stderr/support.rs
  • tests/logging_stderr/verbose.rs
  • tests/runner_tests.rs
🔗 Linked repositories identified

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

  • leynos/rstest-bdd (auto-detected)
  • leynos/ortho-config (auto-detected)
  • leynos/shared-actions (auto-detected)
💤 Files with no reviewable changes (1)
  • tests/features/advanced_usage.feature

Comment thread build.rs
Comment on lines 11 to 14
use std::{
env,
ffi::OsString,
fs,
env, fs,
path::{Path, PathBuf},
sync::Arc,
};

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy lift

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

rg -n -C 2 'std::\{[^}]*\b(fs|path)\b|std::fs|std::path|fs::read_to_string' \
  build.rs tests/assert_cmd_tests.rs
rg -n -C 2 'cap_std|fs_utf8|camino' Cargo.toml build.rs tests

Repository: leynos/netsuke

Length of output: 19575


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '\n== build.rs ==\n'
cat -n build.rs | sed -n '1,220p'

printf '\n== tests/assert_cmd_tests.rs ==\n'
cat -n tests/assert_cmd_tests.rs | sed -n '1,220p'

Repository: leynos/netsuke

Length of output: 12258


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '\n== Cargo.toml dependencies ==\n'
cat -n Cargo.toml | sed -n '1,240p'

Repository: leynos/netsuke

Length of output: 8472


Replace std::fs/std::path with capability-scoped I/O.

  • build.rs: add cap-std and camino to [build-dependencies] too, then stage the man page through cap_std::fs_utf8::Dir and camino::Utf8PathBuf.
  • tests/assert_cmd_tests.rs: read the generated file through a capability-scoped handle instead of std::fs::read_to_string.
📍 Affects 2 files
  • build.rs#L11-L14 (this comment)
  • tests/assert_cmd_tests.rs#L68-L70
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@build.rs` around lines 11 - 14, Replace std::fs and std::path usage in
build.rs with capability-scoped I/O: add cap-std and camino under
[build-dependencies], and use cap_std::fs_utf8::Dir with camino::Utf8PathBuf to
stage the man page. In tests/assert_cmd_tests.rs, replace
std::fs::read_to_string with reads through a capability-scoped handle,
preserving the existing generated-file assertions.

Source: Coding guidelines

Comment thread docs/netsuke-design.md
Comment on lines +2277 to +2282
For automation use cases, Netsuke supports a root `--json` flag layered through
OrthoConfig as `--json`, `NETSUKE_JSON`, and `json = true`. When enabled, every
invocation emits exactly one versioned JSON document: on success, a result
document on `stdout`; on failure, a diagnostic document on `stderr`. Both
documents share `schema_version = 1` and a `generator` object of `name` and
`version`:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Limit the JSON guarantee to resolved JSON mode.

This sentence says “every invocation” emits a JSON document, while the same section states that configuration-file failures cannot request JSON during early startup. Limit the guarantee to invocations where JSON mode has been resolved, or state the early-startup exception in the same contract.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@docs/netsuke-design.md` around lines 2277 - 2282, Update the JSON output
contract in the documented automation behavior to guarantee exactly one JSON
document only when JSON mode has been successfully resolved. Explicitly exclude
early-startup configuration-file failures that occur before JSON mode can be
determined, while preserving the existing success and failure document
requirements for resolved JSON mode.

Comment thread docs/netsuke-design.md
Comment on lines +2426 to +2428
- `netsuke generate`: This command performs the pipeline up to Ninja synthesis
and writes the resulting Ninja file to stdout without invoking Ninja.
Supplying `--output FILE` writes the generated Ninja file to `FILE` instead.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Remove the retired command contract from the surrounding CLI model.

This new generate description still sits beside the unchanged Commands::Manifest and build --emit schema and behaviour examples above. Update those entries together so the design document does not publish both command surfaces.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@docs/netsuke-design.md` around lines 2426 - 2428, Update the surrounding CLI
model in docs/netsuke-design.md to consistently remove the retired generate
command contract: revise or remove the related Commands::Manifest schema and
behavior examples, and eliminate the build --emit surface where it represents
the same retired workflow. Ensure the document no longer publishes both command
interfaces while preserving unrelated command documentation.

lodyai Bot pushed a commit that referenced this pull request Jul 27, 2026
)

The rebase onto main lands #427 and ADR-004, which remove the legacy
`NETSUKE_CONFIG_PATH` alias and keep `NETSUKE_CONFIG` as the only environment
selector. Reconcile the config-path instrumentation with that decision:

- Drop the `NETSUKE_CONFIG_PATH` branch from `resolve_config_selector`,
  `explicit_config_path`, and their tracing so selection is `--config` then
  `NETSUKE_CONFIG` only.
- Remove the legacy dimension from the discovery tracing tests (fixture,
  scenario struct, and the legacy-only rstest case) and correct the stale
  docstrings that still referenced the legacy variable.
- Track main's `diag_json` to `json` rename in the developers guide while
  keeping the `explicit_config_path -> ConfigPathResolution` signature fix and
  the pure-query/bounded-hash wording.
- Rebuild `Cargo.lock` from main's, adding only the `test_support`
  tracing/tracing-subscriber edges.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants