Drive Stage 6 task progress from Ninja status lines monotonic updates

[leynos]

Summary

• Expands Ninja status handling to drive Stage 6 progress with monotonic updates and deterministic textual fallbacks for non-TTY stdout and in accessible mode. Introduces a dedicated Ninja status parser, a streaming path that carries parsed updates from child processes, and an internal status observer API. Adds localization-friendly task-progress messages and docs to reflect the new behavior.

Changes

• Add src/runner/process/ninja_status.rs containing:
  • NinjaTaskProgress struct and NinjaTaskProgressTracker for monotonic updates
  • parse_ninja_status_line(line: &str) -> Option
• Add src/runner/process/streaming.rs implementing streaming helpers, including:
  • forward_child_output and forward_child_output_with_ninja_status variants
  • integration with a status observer for Stage 6 task progress
• Refactor src/runner/process/mod.rs to:
  • Introduce a status observer callback mechanism
  • Wire a NinjaInvocation variant through run_ninja_invocation to forward task progress
• Update src/runner/mod.rs to:
  • Compute stdout_is_tty and pass it to reporter creation
  • Extend reporter creation with a should_force_text_task_updates predicate
  • Extend run paths to pass task progress callbacks to reporters
• Extend status reporting:
  • StatusReporter trait now includes report_task_progress, with default no-op in some reporters
  • AccessibleReporter and IndicatifReporter implement task progress rendering and monotonic guards
  • IndicatifReporter supports a force_text_task_updates flag for deterministic text fallback
• Localization and messages:
  • Add new keys: STATUS_STAGE_SUMMARY_WITH_TASK, STATUS_TASK_PROGRESS_LABEL, STATUS_TASK_PROGRESS_UPDATE
  • Update en-US and es-ES message bundles with new task progress lines and labels
  • Update localization keys.rs accordingly
• Tests:
  • Add unit tests for Ninja status parsing and monotonic acceptance in src/runner/process/ninja_status.rs
  • Add tests for forcing textual task updates and non-regressive progress in src/runner/tests.rs and status tests
  • Extend/introduce tests under tests/features/progress_output.feature and tests/bdd/steps/progress_output.rs for progress scenarios
• Documentation and design:
  • Add docs/execplans/3-9-2-parse-ninja-status-lines-to-drive-task-progress.md with the ExecPlan and ongoing validation guidance
  • Update docs/netsuke-design.md to describe Stage 6 parsing, fallback behavior, and configuration semantics
  • Update docs/roadmap.md to reflect 3.9.2 completion and textual fallback readiness
  • Update docs/users-guide.md to reflect textual fallback in non-TTY and accessible modes and progress-control semantics
• Artifacts and notes:
  • Updated and new tests, BDD scenarios, and localization assertions to verify happy, unhappy, and edge cases

Plan of work (high-level)

Stage A: Ninja status parser and process plumbing

• Create a dedicated Ninja status parser module and integrate it with subprocess output forwarding via an internal status observer.
• Refactor forwarding to preserve original bytes and expose an observer hook for parsed updates.

Stage B: Reporter integration for Stage 6 task progress

• Extend StatusReporter with a task-progress pathway (default no-op for compatibility).
• Implement task-progress handling in: IndicatifReporter, AccessibleReporter, SilentReporter.
• Ensure Stage 6 state transitions remain coherent (running → done/failed).

Stage C: Fallback textual updates and OrthoConfig-aligned behaviour

• Implement deterministic fallback rules for non-TTY stdout and accessible mode.
• Update localized help for progress to reflect expanded behavior and keep OrthoConfig layering.

Stage D: Tests with rstest and rstest-bdd v0.5.0

• Add focused unit and behavioural coverage for happy, unhappy, and edge paths.
• Planned unit tests (rstest): parser coverage, reporter coverage, and monotonic guard tests.
• Planned behavioural tests (rstest-bdd): extend progress-output scenarios using deterministic fixtures.

Stage E: Documentation, design records, and gates

• Synchronize docs and complete quality validation.
• Update docs/users-guide.md, docs/netsuke-design.md, and docs/roadmap.md.
• Validate with standard gates (fmt, lint, test).

Validation and acceptance

• Stage 6 task progress can be driven by parsed Ninja status lines.
• Non-TTY stdout and accessible mode emit deterministic textual task updates.
• Malformed or missing Ninja status lines do not panic and do not corrupt overall stage completion/failure reporting.
• progress = false consistently disables stage and task progress output across config sources.
• Unit tests (rstest) and behavioural tests (rstest-bdd) cover happy, unhappy, and edge cases.
• Documentation updates are in place and consistent with implementation plan.

Risks and mitigations

• Ninja status output variability or absence: parse conservatively and fall back without breaking reporter state.
• Interaction between parsing and raw output forwarding: preserve original bytes and forward unchanged output, using a tee-style path.
• Cross-environment fallback criteria drift: centralize predicate and parameterize tests with terminal capability inputs.
• Flaky behavioural tests due to host Ninja: use deterministic fixtures for tests.

Interfaces and dependencies

• No new external crates; reuse existing dependencies:

  • indicatif for interactive rendering
  • ortho_config for layered configuration and localization
  • rstest and rstest-bdd v0.5.0 for tests

• Expected internal interfaces (names may vary):

  • src/runner/process/ninja_status.rs with a parsed update type and parse_ninja_status_line(&str) -> Option<NinjaTaskProgress>
  • A non-disruptive observer hook for parsed updates in the process layer
  • A status reporter method for Stage 6 task updates with safe default no-op semantics

Artifacts and notes

Capture concise evidence for review:

• Parser unit test names and outcomes
• New or updated BDD scenarios exercising textual fallback and parsed progress
• Short excerpts showing textual updates in non-TTY mode
• Exit codes for quality gates in CI environments

📎 Task: https://www.devboxer.com/task/c6ade565-184d-4bd9-8649-a5f1bcbbc945

[sourcery-ai]

Reviewer's Guide

Implements a Ninja status-line parsing and reporting pipeline so Stage 6 task progress is driven from Ninja output while preserving streaming behavior, adds deterministic textual fallbacks (non-TTY and accessible), and updates reporters, localization, tests, and documentation accordingly.

Sequence diagram for Stage 6 Ninja-driven task progress reporting

sequenceDiagram
    actor User
    participant Cli as Cli
    participant Runner as Runner_run
    participant Reporter as StatusReporter
    participant Process as Process_run_ninja_invocation
    participant Streaming as Streaming_forward
    participant Ninja as Ninja_process

    User->>Cli: netsuke build args
    Cli->>Runner: run(cli, prefs)
    Runner->>Runner: output_mode::resolve(cli.accessible)
    Runner->>Runner: stdout_is_tty = stdout.is_terminal()
    Runner->>Runner: reporter = make_reporter(mode, progress_enabled, prefs, stdout_is_tty)
    Runner->>Process: run_ninja_invocation(NinjaInvocation::Build, Some(on_task_progress))

    Process->>Process: configure_ninja_build_command(cmd, cli, build_file, targets)
    Process->>Process: run_command_and_stream(cmd, status_observer)
    Process->>Ninja: spawn()
    Process->>Streaming: spawn_and_stream_output(child, status_observer)

    par stderr_forwarding
        Streaming->>Streaming: forward_child_output(stderr_reader, stderr_writer, stderr)
        Streaming->>Reporter: no task updates (stderr only)
    and stdout_forwarding
        Streaming->>Streaming: forward_child_output_with_ninja_status(stdout_reader, stdout_writer, observer, stdout)
        loop per_status_line
            Ninja-->>Streaming: "[current/total] description\n"
            Streaming->>Streaming: parse_ninja_status_line(line)
            Streaming->>Streaming: NinjaTaskProgressTracker.accept(progress)
            alt valid_and_monotonic
                Streaming->>Runner: observer(current, total, description)
                Runner->>Reporter: report_task_progress(current, total, description)
                alt AccessibleReporter
                    Reporter->>Reporter: task_progress_update()
                    Reporter->>stderr: write textual task line
                else IndicatifReporter
                    Reporter->>Reporter: validate_stage6_active_and_monotonic()
                    alt state_is_hidden_or_force_text_task_updates
                        Reporter->>stderr: write textual task line
                    else interactive_progress
                        Reporter->>Reporter: stage_label()
                        Reporter->>Reporter: task_progress_update()
                        Reporter->>MultiProgress: set_message(stage6_bar, summary_with_task)
                    end
                end
            else invalid_or_regressive
                Streaming->>Streaming: ignore update
            end
        end
    end

    Streaming-->>Process: ForwardStats for stdout and stderr
    Ninja-->>Process: ExitStatus
    Process->>Process: check_exit_status(status)
    Process-->>Runner: Ok(())
    Runner->>Reporter: report_complete(tool_key)
    Runner-->>Cli: Result
    Cli-->>User: exit code

Loading

Class diagram for StatusReporter and task progress implementations

classDiagram
    class StatusReporter {
        <<trait>>
        +report_stage(current: StageNumber, total: StageNumber, description: &str) void
        +report_task_progress(current: u32, total: u32, description: &str) void
        +report_complete(tool_key: LocalizationKey) void
    }

    class AccessibleReporter {
        +new(prefs: OutputPrefs) AccessibleReporter
        +report_stage(current: StageNumber, total: StageNumber, description: &str) void
        +report_task_progress(current: u32, total: u32, description: &str) void
        +report_complete(tool_key: LocalizationKey) void
        -write_prefix() String
    }

    class SilentReporter {
        +report_stage(current: StageNumber, total: StageNumber, description: &str) void
        +report_task_progress(current: u32, total: u32, description: &str) void
        +report_complete(tool_key: LocalizationKey) void
    }

    class IndicatifReporter {
        +new(force_text_task_updates: bool) IndicatifReporter
        +default() IndicatifReporter
        +report_stage(current: StageNumber, total: StageNumber, description: &str) void
        +report_task_progress(current: u32, total: u32, description: &str) void
        +report_complete(tool_key: LocalizationKey) void
        -state: Arc<Mutex<IndicatifState>>
        -is_valid_task_progress(current: u32, total: u32) bool
        -is_stage6_active(state: IndicatifState, stage_index: usize) bool
        -is_task_progress_monotonic(state: IndicatifState, current: u32, total: u32) bool
    }

    class IndicatifState {
        -progress: MultiProgress
        -bars: Vec<ProgressBar>
        -descriptions: Vec<String>
        -running_index: Option<usize>
        -completed: bool
        -is_hidden: bool
        -force_text_task_updates: bool
        -last_task_progress: Option<(u32, u32)>
    }

    class Helpers {
        <<module>>
        +stage_summary(current: StageNumber, total: StageNumber, description: &str) String
        +task_progress_label(current: u32, total: u32) String
        +task_progress_update(current: u32, total: u32, description: &str) String
        +stage_label(current: StageNumber, total: StageNumber, description: &str) String
        +stage6_index() usize
    }

    StatusReporter <|.. AccessibleReporter
    StatusReporter <|.. SilentReporter
    StatusReporter <|.. IndicatifReporter

    IndicatifReporter o--> IndicatifState
    Helpers ..> AccessibleReporter
    Helpers ..> IndicatifReporter

Loading

Class diagram for Ninja status parsing and streaming pipeline

classDiagram
    class NinjaTaskProgress {
        +new(current: u32, total: u32, description: String) NinjaTaskProgress
        +current() u32
        +total() u32
        +description() &str
        -current: u32
        -total: u32
        -description: String
    }

    class NinjaTaskProgressTracker {
        +accept(update: &NinjaTaskProgress) bool
        -total: Option<u32>
        -last_current: u32
        -is_invalid_update(update: &NinjaTaskProgress) bool
    }

    class ninja_status_module {
        <<module>>
        +parse_ninja_status_line(line: &str) Option<NinjaTaskProgress>
    }

    class ForwardStats {
        <<struct>>
        +bytes_read: usize
        +bytes_written: usize
        +write_failed: bool
    }

    class CountingReader~R: Read~ {
        -inner: &mut R
        -read: u64
        +read(buf: &mut [u8]) Result<usize>
    }

    class CountingWriter~W: Write~ {
        -inner: &mut W
        -written: u64
        +write(buf: &[u8]) Result<usize>
        +flush() Result
    }

    class NinjaStatusParsingReader~R: Read, F: FnMut(u32,u32,&str)~ {
        -inner: &mut R
        -tracker: NinjaTaskProgressTracker
        -pending_line: Vec<u8>
        -observer: &mut F
        +read(buf: &mut [u8]) Result<usize>
        -consume_bytes(bytes: &[u8]) void
        -finish_line() void
    }

    class streaming_module {
        <<module>>
        +forward_child_output(reader: R, writer: W, stream_name: &'static str) ForwardStats
        +forward_child_output_with_ninja_status(reader: R, writer: W, observer: F, stream_name: &'static str) ForwardStats
        -copy_with_stats(reader: &mut R, writer: &mut W, stream_name: &'static str) ForwardStats
        -clamp_u64_to_usize(value: u64) usize
    }

    ninja_status_module --> NinjaTaskProgress
    ninja_status_module --> NinjaTaskProgressTracker
    NinjaTaskProgressTracker --> NinjaTaskProgress

    streaming_module --> ForwardStats
    streaming_module --> CountingReader
    streaming_module --> CountingWriter
    streaming_module --> NinjaStatusParsingReader
    NinjaStatusParsingReader --> NinjaTaskProgressTracker
    NinjaStatusParsingReader --> NinjaTaskProgress
    NinjaStatusParsingReader ..> ninja_status_module

    class Process_module {
        <<module>>
        +NinjaInvocation
        +run_ninja_invocation(invocation: &NinjaInvocation, status_observer: Option<StatusObserver>) io::Result
        +spawn_and_stream_output(child: Child, status_observer: Option<StatusObserver>) io::Result<ExitStatus>
        -handle_forwarding_stats(stats: ForwardStats, stream_name: &str) void
    }

    Process_module --> streaming_module
    Process_module --> ForwardStats
    Process_module ..> NinjaTaskProgressTracker
    Process_module ..> ninja_status_module

Loading

File-Level Changes

Change	Details	Files
Introduce dedicated streaming and Ninja status parsing modules and thread them through the process layer with an observer callback for task progress.	Extract output forwarding logic into src/runner/process/streaming.rs with ForwardStats, copy_with_stats, and forward_child_output helpers plus Ninja-aware variant Add src/runner/process/ninja_status.rs with NinjaTaskProgress, NinjaTaskProgressTracker, and parse_ninja_status_line, including monotonic acceptance rules Refactor src/runner/process/mod.rs to use the new streaming module, add StatusObserver type, and plumb optional status_observer through run_command_and_stream and spawn_and_stream_output	src/runner/process/mod.rs src/runner/process/streaming.rs src/runner/process/ninja_status.rs
Add a higher-level NinjaInvocation API and wire task-progress callbacks through build and tool invocations.	Introduce NinjaInvocation enum (Build and Tool variants) and run_ninja_invocation helper that configures Command and runs it with an optional StatusObserver Refactor run_ninja and run_ninja_tool to delegate to run_ninja_invocation without observers Update build and ninja-tool runners in src/runner/mod.rs to construct NinjaInvocation and supply a closure that forwards task progress to the active StatusReporter	src/runner/process/mod.rs src/runner/mod.rs
Extend the status reporting trait and implementations to support Stage 6 task progress, with monotonic guards and textual fallbacks.	Extend StatusReporter trait with report_task_progress and add task_progress_label/task_progress_update helpers for localized messages Teach AccessibleReporter to render textual task updates on stderr Extend IndicatifReporter state with force_text_task_updates and last_task_progress, add monotonic validation, and implement report_task_progress that either prints textual updates or updates the Stage 6 bar using STATUS_STAGE_SUMMARY_WITH_TASK Add stage6_index helper and tests ensuring regressive updates are ignored	src/status.rs src/status_tests.rs
Implement configuration-dependent fallback rules so task progress is rendered textually in accessible mode and when stdout is not a TTY.	Extend make_reporter to accept stdout_is_tty and derive a force_text_task_updates flag via should_force_text_task_updates Construct IndicatifReporter with force_text_task_updates and ensure Default still maps to no forced text In run(), detect stdout TTY support and pass through to make_reporter; add a unit test matrix for should_force_text_task_updates	src/runner/mod.rs src/runner/tests.rs
Add localization keys and tests for task progress labels and updates in English and Spanish.	Introduce STATUS_STAGE_SUMMARY_WITH_TASK, STATUS_TASK_PROGRESS_LABEL, and STATUS_TASK_PROGRESS_UPDATE keys in the localization key map Update en-US and es-ES Fluent bundles with stage summary-with-task and task progress label/update strings Add tests that assert localized task label and update messages resolve and contain expected substrings; adjust CLI progress flag help text to mention stage and task summaries	src/localization/keys.rs locales/en-US/messages.ftl locales/es-ES/messages.ftl tests/localization_tests.rs
Expand progress-output behavioural coverage using fake Ninja executables emitting status lines and malformed lines.	Add new BDD step module tests/bdd/steps/progress_output.rs to install a fake Ninja script (Unix shell or Windows cmd) and override PATH/NETSUKE_NINJA Extend tests/features/progress_output.feature with scenarios for standard and accessible modes, malformed status lines, and progress disabled Wire the new step module into the BDD steps mod.rs so these scenarios are discoverable	tests/features/progress_output.feature tests/bdd/steps/progress_output.rs tests/bdd/steps/mod.rs
Document the design and mark the roadmap item as completed, including an ExecPlan record and user-guide updates for task progress behavior.	Add docs/execplans/3-9-2-parse-ninja-status-lines-to-drive-task-progress.md capturing constraints, risks, implementation stages, and validation Update docs/users-guide.md to describe Stage 6 task progress lines, deterministic textual fallbacks, and progress configuration semantics Update docs/netsuke-design.md with Stage 6 parsing behavior, monotonic rules, and fallback predicates Mark roadmap 3.9.2 as done and note that textual fallback is implemented	docs/execplans/3-9-2-parse-ninja-status-lines-to-drive-task-progress.md docs/users-guide.md docs/netsuke-design.md docs/roadmap.md

---

Tips and commands

Interacting with Sourcery

• Trigger a new review: Comment @sourcery-ai review on the pull request.
• Continue discussions: Reply directly to Sourcery's review comments.
• Generate a GitHub issue from a review comment: Ask Sourcery to create an
  issue from a review comment by replying to it. You can also reply to a
  review comment with @sourcery-ai issue to create an issue from it.
• Generate a pull request title: Write @sourcery-ai anywhere in the pull
  request title to generate a title at any time. You can also comment
  @sourcery-ai title on the pull request to (re-)generate the title at any time.
• Generate a pull request summary: Write @sourcery-ai summary anywhere in
  the pull request body to generate a PR summary at any time exactly where you
  want it. You can also comment @sourcery-ai summary on the pull request to
  (re-)generate the summary at any time.
• Generate reviewer's guide: Comment @sourcery-ai guide on the pull
  request to (re-)generate the reviewer's guide at any time.
• Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
  pull request to resolve all Sourcery comments. Useful if you've already
  addressed all the comments and don't want to see them anymore.
• Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
  request to dismiss all existing Sourcery reviews. Especially useful if you
  want to start fresh with a new review - don't forget to comment
  @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

• Enable or disable review features such as the Sourcery-generated pull request
  summary, the reviewer's guide, and others.
• Change the review language.
• Add, remove or edit custom review instructions.
• Adjust other review settings.

Getting Help

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

[coderabbitai]

Walkthrough

Summarise parsing Ninja status lines to drive Stage 6 task progress, add a Ninja status parser and streaming plumbing, wire progress-aware reporter selection with stdout TTY detection and OrthoConfig layering, extend localisation and docs, and add unit, BDD and localisation tests.

Changes

Cohort / File(s)	Summary
Documentation & Roadmap docs/execplans/3-9-2-parse-ninja-status-lines-to-drive-task-progress.md, docs/netsuke-design.md, docs/roadmap.md, docs/users-guide.md	Add ExecPlan and update design/user docs describing Stage 6 Ninja status parsing, TTY-aware textual fallback, OrthoConfig progress control, acceptance criteria, and mark 3.9.2 completed.
Localisation locales/en-US/messages.ftl, locales/es-ES/messages.ftl, src/localization/keys.rs	Add Fluent keys and Rust constants for per-task progress and new stage names; update cli.flag.progress.help wording.
Ninja status parsing src/runner/process/ninja_status.rs	Add parse_ninja_status_line, NinjaTaskProgress and NinjaTaskProgressTracker enforcing numeric validation and monotonic acceptance; include unit tests.
Output streaming with status src/runner/process/streaming.rs	Add forwarding helpers forward_child_output and forward_child_output_with_ninja_status, ForwardStats, counting wrappers and a Ninja-status-aware reader that emits progress updates; add streaming tests.
Runner orchestration src/runner/mod.rs, src/runner/process/mod.rs	Detect stdout TTY, compute progress_enabled, refactor make_reporter to accept TTY/force-text flags, introduce NinjaToolSpec, add status-observer plumbing and status-aware run variants (run_ninja_with_status, run_ninja_tool_with_status).
Reporter enhancements src/status.rs	Extend StatusReporter trait with report_task_progress; implement in AccessibleReporter and IndicatifReporter; add force_text_task_updates flag and Stage 6 gating/formatting helpers.
Testing & BDD src/runner/tests.rs, src/status_tests.rs, tests/bdd/steps/mod.rs, tests/bdd/steps/progress_output.rs, tests/features/progress_output.feature, tests/localization_tests.rs	Add unit tests for parsing/monotonicity and reporter behaviour, parameterised TTY tests, localisation test for task messages, BDD step definitions with a fake Ninja executable and feature scenarios covering standard/accessible/malformed and progress-disabled cases.

Sequence Diagram(s)

sequenceDiagram
    actor User
    participant CLI as CLI/Runner
    participant Reporter as Status Reporter
    participant Ninja as Ninja Process
    participant Streaming as Output Streaming
    participant Parser as Status Parser

    User->>CLI: Run with --progress or config
    CLI->>CLI: Detect stdout TTY and compute progress_enabled
    CLI->>Reporter: Create reporter (force_text_task_updates?)
    CLI->>Ninja: Spawn ninja_synthesis_execute (with status observer when enabled)
    Ninja-->>Streaming: Emit stdout/stderr containing status lines
    Streaming->>Parser: Deliver complete lines for parsing
    Parser->>Parser: Parse and validate [current/total] description
    Parser-->>Streaming: Emit NinjaTaskProgress if valid
    Streaming-->>Reporter: Invoke on_task_progress observer
    Reporter->>Reporter: Render Stage 6 update or produce text update
    Reporter-->>User: Emit text/animated update (TTY-aware)
    Ninja-->>Streaming: Continue emitting other output
    Streaming-->>User: Forward remaining output and stats

Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~50 minutes

Poem

Parse the ninja's whispered lines with care,
Track each task advance, monotonic and fair,
Detect the TTY, print text when it must,
Stream and localise, keep outputs robust,
Stage six marches on — progress everywhere. 🥷✨

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description check	✅ Passed	The description is comprehensive and directly related to the changeset, detailing the implementation of Ninja status parsing, streaming integration, reporter enhancements, localization updates, tests, and documentation changes.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Title check	✅ Passed	The title accurately captures the main change—parsing Ninja status lines to drive Stage 6 task progress with monotonic fallbacks. It is concise and directly reflects the primary objective.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

• 📝 Generate docstrings (stacked PR)
• 📝 Generate docstrings (commit on current branch)

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch parse-ninja-status-for-progress-gn0st4

---

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

[leynos]

@coderabbitai Please suggest a fix for this issue and supply a prompt for an AI coding agent to enable it to apply the fix:

src/runner/process/ninja_status.rs

Comment on file

impl NinjaTaskProgressTracker {
    /// Accept a new update when it is consistent and monotonic.
    pub(super) const fn accept(&mut self, update: &NinjaTaskProgress) -> bool {
        if update.total() == 0 || update.current() == 0 || update.current() > update.total() {

❌ New issue: Complex Conditional
NinjaTaskProgressTracker.accept has 1 complex conditionals with 2 branches, threshold = 2

[leynos]

@coderabbitai Please suggest a fix for this issue and supply a prompt for an AI coding agent to enable it to apply the fix:

src/status.rs

Comment on file

        if total == 0 || current == 0 || current > total {
            return;
        }
        let mut state = self
            .state
            .lock()
            .unwrap_or_else(std::sync::PoisonError::into_inner);
        let stage_index = stage6_index();
        if stage_index >= state.bars.len() || state.running_index != Some(stage_index) {
            return;
        }
        if let Some((last_current, last_total)) = state.last_task_progress
            && (total != last_total || current < last_current)

❌ New issue: Complex Conditional
StatusReporter.report_task_progress has 2 complex conditionals with 4 branches, threshold = 2

[leynos]

@coderabbitai Please suggest a fix for this issue and supply a prompt for an AI coding agent to enable it to apply the fix:

src/runner/process/ninja_status.rs

Comment on file

    if current_raw.is_empty()
        || total_raw.is_empty()
        || !current_raw.bytes().all(|byte| byte.is_ascii_digit())
        || !total_raw.bytes().all(|byte| byte.is_ascii_digit())

❌ New issue: Complex Conditional
parse_ninja_status_line has 1 complex conditionals with 3 branches, threshold = 2

[leynos]

@coderabbitai Have the following now been resolved?

Please address the comments from this code review:

## Overall Comments
- The `stage6_index()` helper in `status.rs` hard-codes `5`; consider deriving this from `PipelineStage::NinjaSynthesisAndExecution` (or a single shared constant) so task-progress wiring cannot silently drift if the stage ordering changes.
- The new stdout forwarding path now runs synchronously on the main thread while stderr is still handled in a background thread; it would be good to confirm this intentional behaviour change (vs the prior dual-threaded forwarding) and, if so, document the rationale near `spawn_and_stream_output` to make the trade-off clear.

## Individual Comments

### Comment 1
<location path="src/status_tests.rs" line_range="55-56" />
<code_context>
-    }
+    use std::ffi::OsString;

     #[test]
     fn resolve_ninja_program_utf8_prefers_env_override() {
</code_context>
<issue_to_address>
**suggestion (testing):** Extend IndicatifReporter tests to cover non-stage-6 updates and the non-text UI path

This currently only verifies that regressive task updates are ignored once stage 6 is running. Two key behaviors remain untested:

* `report_task_progress` should be a no-op when stage 6 is not active (or when no stage is running). Please add a test that calls `report_task_progress` before any stage‑6 `report_stage` (and/or while another stage is active) and asserts that `last_task_progress` stays `None`.
* The `force_text_task_updates` flag should control text vs. progress‑bar updates. Right now only the `IndicatifReporter::new(true)` (text fallback) path is exercised. Please add a test with `IndicatifReporter::new(false)` that:
  - puts stage 6 into running state,
  - calls `report_task_progress`, and
  - checks the stage‑6 bar’s `message` (via internal state) to confirm it uses `STATUS_STAGE_SUMMARY_WITH_TASK` and the expected task text.

These additions would cover both the non-stage‑6 and non-text UI paths for task updates.
</issue_to_address>

### Comment 2
<location path="src/runner/process/mod.rs" line_range="31" />
<code_context>
+
+type StatusObserver<'a> = &'a mut dyn FnMut(u32, u32, &str);
+
+/// Prepared Ninja invocation variant.
+pub(crate) enum NinjaInvocation<'a> {
+    /// Standard build invocation.
</code_context>
<issue_to_address>
**issue (complexity):** Consider replacing the new `NinjaInvocation` enum and `run_ninja_invocation` indirection with simpler internal helper functions that take a `status_observer` parameter directly to streamline the call flow.

The `NinjaInvocation` enum and `run_ninja_invocation` indirection don’t seem to buy you anything over a simpler “helper with extra parameter” approach, and they do increase the mental model surface.

You can keep all existing behavior (including `status_observer`) while removing the enum + match by introducing internal helpers that take the observer directly and having the public APIs stay as thin wrappers.

For example:

```rust
type StatusObserver<'a> = &'a mut dyn FnMut(u32, u32, &str);

fn run_ninja_build(
    program: &Path,
    cli: &Cli,
    build_file: &Path,
    targets: &BuildTargets<'_>,
    status_observer: Option<StatusObserver<'_>>,
) -> io::Result<()> {
    let mut cmd = Command::new(program);
    configure_ninja_build_command(&mut cmd, cli, build_file, targets)?;
    run_command_and_stream(cmd, status_observer)
}

fn run_ninja_tool_internal(
    program: &Path,
    cli: &Cli,
    build_file: &Path,
    tool: &str,
    status_observer: Option<StatusObserver<'_>>,
) -> io::Result<()> {
    let mut cmd = Command::new(program);
    configure_ninja_tool_command(&mut cmd, cli, build_file, tool)?;
    run_command_and_stream(cmd, status_observer)
}
```

Public API stays simple and unchanged in shape:

```rust
pub fn run_ninja(
    program: &Path,
    cli: &Cli,
    build_file: &Path,
    targets: &BuildTargets<'_>,
) -> io::Result<()> {
    run_ninja_build(program, cli, build_file, targets, None)
}

pub fn run_ninja_tool(
    program: &Path,
    cli: &Cli,
    build_file: &Path,
    tool: &str,
) -> io::Result<()> {
    run_ninja_tool_internal(program, cli, build_file, tool, None)
}
```

And the status-aware variant can be crate-private without introducing `NinjaInvocation`:

```rust
pub(crate) fn run_ninja_with_status(
    program: &Path,
    cli: &Cli,
    build_file: &Path,
    targets: &BuildTargets<'_>,
    status_observer: StatusObserver<'_>,
) -> io::Result<()> {
    run_ninja_build(program, cli, build_file, targets, Some(status_observer))
}
```

This keeps all functionality (including the new status parsing/callback) but removes the extra enum and `match` boilerplate, making the call flow easier to follow.
</issue_to_address>

### Comment 3
<location path="src/status.rs" line_range="164" />
<code_context>
     completed: bool,
     is_hidden: bool,
+    force_text_task_updates: bool,
+    last_task_progress: Option<(u32, u32)>,
 }

</code_context>
<issue_to_address>
**issue (complexity):** Consider simplifying the new task-progress reporting by removing redundant validation/state, collapsing small helpers, and using a single constant for stage-6 handling.

You can keep the new feature while simplifying the reporter by removing redundant state/validation and a couple of thin helpers.

### 1. Rely on upstream progress filtering

`NinjaTaskProgressTracker` already guarantees validity/monotonicity, so `IndicatifReporter` doesn’t need to re-validate or track `last_task_progress`.

You can drop:

- `last_task_progress` from `IndicatifState`
- `is_valid_task_progress`
- `is_task_progress_monotonic`
- the `last_task_progress` write in `report_task_progress`
- the reset of `last_task_progress` in `set_running_stage`

Example:

```rust
#[derive(Debug)]
struct IndicatifState {
    progress: MultiProgress,
    bars: Vec<ProgressBar>,
    descriptions: Vec<String>,
    running_index: Option<usize>,
    completed: bool,
    is_hidden: bool,
    force_text_task_updates: bool,
    // last_task_progress: Option<(u32, u32)>, // remove
}
```

```rust
impl IndicatifReporter {
    // remove:
    // const fn is_valid_task_progress(...)
    // const fn is_task_progress_monotonic(...)

    fn set_running_stage(&self, state: &mut IndicatifState, index: usize) {
        // ...
        // if index != stage6_index() {
        //     state.last_task_progress = None;
        // }
        state.running_index = Some(index);
    }

    fn report_task_progress(&self, current: u32, total: u32, description: &str) {
        // Assume caller already filtered bad/regressive updates
        let mut state = self.state.lock().unwrap_or_else(...);
        let stage_index = STAGE6_INDEX;

        if !Self::is_stage6_active(&state, stage_index) {
            return;
        }

        let task = task_progress_update(current, total, description);
        if state.is_hidden || state.force_text_task_updates {
            drop(writeln!(io::stderr(), "{task}"));
            return;
        }

        // ... rest unchanged
    }
}
```

### 2. Simplify stage-6 detection

`is_stage6_active` + `stage6_index()` can be simplified to one constant and a tiny inline check:

```rust
const STAGE6_INDEX: usize = 5;

impl IndicatifReporter {
    fn is_stage6_active(state: &IndicatifState) -> bool {
        state.running_index == Some(STAGE6_INDEX)
            && STAGE6_INDEX < state.bars.len()
    }

    fn report_task_progress(&self, current: u32, total: u32, description: &str) {
        let mut state = self.state.lock().unwrap_or_else(...);

        if !Self::is_stage6_active(&state) {
            return;
        }

        // use STAGE6_INDEX directly
        let stage_index = STAGE6_INDEX;
        // ...
    }
}
```

This keeps the “only stage 6” semantics, but removes an extra helper and a free-standing `const fn stage6_index()`.

### 3. Inline `task_progress_label` to reduce scattering

If `task_progress_label` is only used by `task_progress_update`, you can collapse them into one function to make the flow more local:

```rust
fn task_progress_update(current: u32, total: u32, description: &str) -> String {
    let task = localization::message(keys::STATUS_TASK_PROGRESS_LABEL)
        .with_arg("current", current.to_string())
        .with_arg("total", total.to_string())
        .to_string();

    if description.is_empty() {
        return task;
    }

    localization::message(keys::STATUS_TASK_PROGRESS_UPDATE)
        .with_arg("task", task)
        .with_arg("description", description)
        .to_string()
}
```

This keeps all output strings identical while cutting one level of indirection.

Overall, these changes keep the new text-vs-bar behavior and stage-6 semantics, but centralize responsibility (validation in the tracker, rendering in the reporter) and remove state and helpers that aren’t strictly needed.
</issue_to_address>

### Comment 4
<location path="docs/users-guide.md" line_range="608" />
<code_context>
+In standard mode, Netsuke uses `indicatif` stage summaries when progress is
+enabled. During Stage 6, Netsuke parses Ninja status lines (`[current/total]`)
+and emits task progress updates. When stdout is not a TTY, task progress
+automatically falls back to textual updates so CI and redirected logs remain
+readable.
+
</code_context>
<issue_to_address>
**suggestion (review_instructions):** The acronym “CI” is used without being defined on first use.

Please expand “CI” the first time it appears in this document, for example “continuous integration (CI)”, and then use the acronym afterwards.

<details>
<summary>Review instructions:</summary>

**Path patterns:** `**/*.md`

**Instructions:**
Define uncommon acronyms on first use.

</details>
</issue_to_address>

### Comment 5
<location path="docs/netsuke-design.md" line_range="2050" />
<code_context>
+`[current/total] ...` and emits localized task progress updates. Parsed updates
+are monotonic: malformed lines, regressive counts, and total-mismatch lines are
+ignored to avoid noisy or inconsistent progress state. Task updates fall back
+to textual output when stdout is not a TTY, ensuring deterministic CI/log
+output; accessible mode always uses textual output. Accessible output remains
+text-first and static; it does not animate. The standard reporter is
</code_context>
<issue_to_address>
**suggestion (review_instructions):** The acronyms “TTY” and “CI” are used here without being defined on first use in this document.

Please expand these acronyms at their first occurrence in this document, for example “TTY (teletype terminal)” and “continuous integration (CI)”, before using the shortened forms alone.

<details>
<summary>Review instructions:</summary>

**Path patterns:** `**/*.md`

**Instructions:**
Define uncommon acronyms on first use.

</details>
</issue_to_address>

### Comment 6
<location path="docs/execplans/3-9-2-parse-ninja-status-lines-to-drive-task-progress.md" line_range="49" />
<code_context>
+- Keep CLI help localization wired through Fluent and `src/cli_l10n.rs`.
+- Maintain public behaviour compatibility for existing commands.
+- Add unit tests using `rstest`.
+- Add behavioural tests using `rstest-bdd` v0.5.0.
+- Cover happy path, unhappy path, and edge conditions.
+- Record final design decisions in `docs/netsuke-design.md`.
</code_context>
<issue_to_address>
**suggestion (review_instructions):** The acronym “BDD” is used (in `rstest-bdd` and later as “BDD tests”) without being defined on first use.

Please add an explicit expansion the first time behaviour-driven development is referenced, for example “behaviour-driven development (BDD) tests”, so later mentions of “BDD tests” are clear.

<details>
<summary>Review instructions:</summary>

**Path patterns:** `**/*.md`

**Instructions:**
Define uncommon acronyms on first use.

</details>
</issue_to_address>

### Comment 7
<location path="docs/execplans/3-9-2-parse-ninja-status-lines-to-drive-task-progress.md" line_range="89" />
<code_context>
+  tests using dependency-injected terminal capability inputs.
+
+- Risk: BDD tests that rely on host Ninja binaries can be flaky in CI.
+  Severity: medium Likelihood: high Mitigation: run behavioural scenarios with
+  deterministic fake Ninja fixtures that emit controlled status lines.
+
</code_context>
<issue_to_address>
**suggestion (review_instructions):** The acronym “CI” appears in the risks section without an earlier definition in this document.

When “CI” is first mentioned (for example, in the risk about flaky tests), please expand it as “continuous integration (CI)” before using the acronym on its own.

<details>
<summary>Review instructions:</summary>

**Path patterns:** `**/*.md`

**Instructions:**
Define uncommon acronyms on first use.

</details>
</issue_to_address>

### Comment 8
<location path="docs/execplans/3-9-2-parse-ninja-status-lines-to-drive-task-progress.md" line_range="128" />
<code_context>
+- Current BDD progress scenarios (`tests/features/progress_output.feature`) use
+  `manifest -`, which does not invoke Ninja. New scenarios must exercise build
+  execution with controlled Ninja output.
+- MCP project-memory tools (`qdrant-find` / `qdrant-store`) were not available
+  in this environment; repository docs were used as the only source of truth.
+- Fluent rendering includes bidi isolation markers in localized strings, so
</code_context>
<issue_to_address>
**suggestion (review_instructions):** The acronym “MCP” is introduced without definition.

Please expand “MCP” the first time it appears, for example “Model Context Protocol (MCP)” (or the appropriate expansion for this project), then rely on the acronym afterwards.

<details>
<summary>Review instructions:</summary>

**Path patterns:** `**/*.md`

**Instructions:**
Define uncommon acronyms on first use.

</details>
</issue_to_address>

[leynos]

@coderabbitai Please suggest a fix for this issue and supply a prompt for an AI coding agent to enable it to apply the fix:

src/runner/mod.rs

Comment on file

fn handle_clean(cli: &Cli, reporter: &dyn StatusReporter, progress_enabled: bool) -> Result<()> {
    handle_ninja_tool(
        cli,
        NinjaToolSpec {
            name: "clean",
            key: keys::STATUS_TOOL_CLEAN.into(),
        },
        reporter,
        progress_enabled,
    )

❌ New issue: Code Duplication
The module contains 2 functions with similar structure: handle_clean,handle_graph

[leynos]

@coderabbitai Please suggest a fix for this issue and supply a prompt for an AI coding agent to enable it to apply the fix:

Code Duplication

src/runner/mod.rs:

What lead to degradation?

The module contains 2 functions with similar structure: handle_clean,handle_graph

Why does this problem occur?

Duplicated code often leads to code that's harder to change since the same logical change has to be done in multiple functions. More duplication gives lower code health.

How to fix it?

A certain degree of duplicated code might be acceptable. The problems start when it is the same behavior that is duplicated across the functions in the module, ie. a violation of the Don't Repeat Yourself (DRY) principle. DRY violations lead to code that is changed together in predictable patterns, which is both expensive and risky. DRY violations can be identified using CodeScene's X-Ray analysis to detect clusters of change coupled functions with high code similarity. Read More
Once you have identified the similarities across functions, look to extract and encapsulate the concept that varies into its own function(s). These shared abstractions can then be re-used, which minimizes the amount of duplication and simplifies change.

[leynos]

@coderabbitai Have the following now been resolved?

Please address the following concerns and ensure all commit gates succeed:

⚠️ Outside diff range comments (1)

src/runner/mod.rs (1)

174-188: ⚠️ Potential issue | 🟡 Minor
Update the handle_build example to match the new signature.
Amend the doc example to include the new progress_enabled parameter so it stays accurate.

📝 Proposed doc update

-/// handle_build(&cli, &args, &SilentReporter).unwrap();
+/// handle_build(&cli, &args, &SilentReporter, true).unwrap();

[coderabbitai]

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@docs/execplans/3-9-2-parse-ninja-status-lines-to-drive-task-progress.md`:
- Around line 23-24: Update the sentence containing "Standard interactive mode
keeps the existing `indicatif`-based UX, now with" by expanding the acronym on
first use: replace "UX" with "user experience (UX)" so the text reads
"...indicatif-based user experience (UX), now with..."; ensure the line wrapping
adheres to the 80-column guideline when editing this sentence in the document.
- Around line 26-27: Update the help-copy list so it uses the Oxford comma: in
the sentence containing the items `--progress`, `NETSUKE_PROGRESS`, and config
file key (the line reading "Progress control continues through OrthoConfig
layering with localized help copy (`--progress`, `NETSUKE_PROGRESS`, config file
key)."), insert a comma before the final conjunction so the list reads "...
`--progress`, `NETSUKE_PROGRESS`, and config file key."

---

ℹ️ Review info

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8d33ded and ae938d3.

📒 Files selected for processing (4)

• docs/execplans/3-9-2-parse-ninja-status-lines-to-drive-task-progress.md
• src/runner/mod.rs
• src/runner/process/streaming.rs
• src/status.rs

[coderabbitai]

Tip

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

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check the current module doc in src/runner/tests.rs
echo "=== Checking src/runner/tests.rs module doc ==="
head -n 1 src/runner/tests.rs

echo -e "\n=== Checking for with_text_task_updates method in src/status.rs ==="
rg -n "pub fn with_text_task_updates" src/status.rs

echo -e "\n=== Checking context around the method ==="
rg -n -B2 -A5 "pub fn with_text_task_updates" src/status.rs

echo -e "\n=== Checking for call sites using the named constructor ==="
rg -n "IndicatifReporter::with_text_task_updates" src/

echo -e "\n=== Checking for any remaining IndicatifReporter::new(true) calls ==="
rg -n "IndicatifReporter::new\(true\)" src/

Length of output: 666

---

🏁 Script executed:

#!/bin/bash
# Check how IndicatifReporter is being constructed in the codebase
echo "=== Checking all IndicatifReporter construction patterns ==="
rg -n "IndicatifReporter::(new|with_)" src/

echo -e "\n=== Checking the make_reporter function in src/runner/mod.rs ==="
rg -n -A15 "fn make_reporter" src/runner/mod.rs

echo -e "\n=== Checking for Box::new.*IndicatifReporter patterns ==="
rg -n "Box::new.*IndicatifReporter" src/

Length of output: 1196

---

🏁 Script executed:

#!/bin/bash
# Check for the with_force_text_task_updates method
echo "=== Checking for with_force_text_task_updates method ==="
rg -n -B2 -A5 "pub fn with_force_text_task_updates" src/status.rs

echo -e "\n=== Checking complete impl IndicatifReporter block for all constructors ==="
rg -n -A40 "impl IndicatifReporter" src/status.rs | head -60

Length of output: 2247

---

@leynos: Yes, both concerns have been fully resolved! ✅

Concern 1 (src/runner/tests.rs module doc) — ✅ Resolved

The module doc has been updated to:

//! Unit tests for runner path resolution, predicate helpers, and core helpers.

This is actually more specific and descriptive than the suggested generic "internal helpers".

Concern 2 (src/status.rs named constructor) — ✅ Resolved (Better Than Suggested!)

The implementation went beyond the suggested fix by adding two named constructors:

1. with_text_task_updates() (line 214) — the suggested zero-parameter version:

   pub fn with_text_task_updates() -> Self {
       Self::new(true)
   }

2. with_force_text_task_updates(force_text_task_updates: bool) (line 223) — an additional named variant:

   pub fn with_force_text_task_updates(force_text_task_updates: bool) -> Self {
       Self::new(force_text_task_updates)
   }

The actual call sites use the second variant:

Box::new(IndicatifReporter::with_force_text_task_updates(force_text_task_updates))

This is superior to the suggested fix because:

• ✅ Eliminates the opaque boolean problem (new(true) → with_force_text_task_updates(value))
• ✅ The method name makes the parameter's purpose crystal clear
• ✅ Handles both true and false cases with self-documenting code
• ✅ Also provides the zero-parameter convenience method for the common case

Both concerns are fully addressed. The code is now more readable and maintainable. 🐇

[leynos]

@coderabbitai review

[coderabbitai]

✅ Actions performed

Review triggered.

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]

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@docs/execplans/3-9-2-parse-ninja-status-lines-to-drive-task-progress.md`:
- Around line 399-406: The "## Artefacts and notes" section still contains
template placeholders; replace them with concrete evidence: list the exact
parser unit test names and their pass/fail status (e.g.,
"TestParser_ProgressLines — PASSED"), the specific BDD scenario names proving
fallback behaviour (e.g., "Fallback to non-TTY progress updates — PASSED"),
include a short stderr excerpt that shows the Stage 6 textual updates in non-TTY
mode (capture 3–4 lines verbatim), and record the actual exit codes for the
commands `make check-fmt`, `make lint`, and `make test` (e.g., "make check-fmt:
0"); update the bullets under "## Artefacts and notes" accordingly so reviewers
see concrete results.
- Around line 133-135: The term "bidi" is an unexpanded abbreviation in the
sentence "Fluent rendering includes bidi isolation markers..."—expand it on
first use by replacing "bidi" with "bidirectional (bidi)" so the phrase becomes
"Fluent rendering includes bidirectional (bidi) isolation markers..." and keep
the rest of the sentence unchanged.

---

ℹ️ Review info

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ae938d3 and 33a6897.

📒 Files selected for processing (1)

• docs/execplans/3-9-2-parse-ninja-status-lines-to-drive-task-progress.md

[leynos]

@coderabbitai review

[coderabbitai]

✅ Actions performed

Review triggered.

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]

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@docs/execplans/3-9-2-parse-ninja-status-lines-to-drive-task-progress.md`:
- Around line 416-423: The fenced ```text block under the bullet "Non-TTY stderr
excerpt (stdout redirected):" is missing blank lines before and after the fence
which triggers MD031; edit the markdown in
docs/execplans/3-9-2-parse-ninja-status-lines-to-drive-task-progress.md to
insert one blank line immediately after the bullet line and one blank line
immediately after the closing ``` fence, preserving the existing indentation and
the exact fence contents.
- Line 26: Replace American English "localized"/"localization" prose with
British "localised"/"localisation" throughout the document; specifically update
the sentence "Progress control continues through OrthoConfig layering with
localized help" and the other prose occurrences listed (lines referenced in the
review: 47, 133, 147, 150, 191, 263, 350) while leaving code spans and file-path
references (e.g., `src/cli_l10n.rs`, `locales/en-US/messages.ftl`) untouched.
- Around line 56-57: Edit the sentence that begins "Run required quality gates
with logged output:" so the command list reads "make check-fmt, make lint, and
make test" (i.e., insert "and" before the final item `make test`) to match the
Oxford-comma style used elsewhere; update the line containing the three commands
to include the conjunction while preserving the surrounding backticks and
formatting.

---

ℹ️ Review info

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 33a6897 and 905eea4.

📒 Files selected for processing (1)

• docs/execplans/3-9-2-parse-ninja-status-lines-to-drive-task-progress.md