Skip to content

Regression coverage for conditional action dependency manifests (3.14.5) - #387

Merged
leynos merged 34 commits into
mainfrom
3-14-5-regression-coverage-for-conditional-action-dependency-manifests
Aug 31, 2026
Merged

Regression coverage for conditional action dependency manifests (3.14.5)#387
leynos merged 34 commits into
mainfrom
3-14-5-regression-coverage-for-conditional-action-dependency-manifests

Conversation

@leynos

@leynos leynos commented Jun 15, 2026

Copy link
Copy Markdown
Owner

Summary

Adds regression coverage for roadmap item 3.14.5 — conditional action
dependency manifests
. The change is test-and-documentation only; production
behaviour is unchanged.

The suite now proves:

  1. Action-level when and foreach work together.
  2. Real command_available(...) resolution selects exactly one nextest or
    legacy action in deterministic present and absent tool worlds.
  3. Absent-command fallback performs no impure stdlib operation during
    selection, with a shell-in-when control proving the observable is live.
  4. A conditionally selected action's deps lower to IR implicit_deps and
    Ninja | inputs, including {{ item }} substitution and || order-only
    dependencies.

Coverage spans confined in-crate googletest/rstest cases, IR integration tests,
an Insta snapshot validated with real Ninja, and one combined BDD scenario.
The users' guide, developers' guide, design document, roadmap, and living
ExecPlan have been updated.

Validation

  • make fmt
  • make markdownlint
  • make nixie
  • make check-fmt
  • make typecheck
  • make lint
  • INSTA_UPDATE=no make test
  • coderabbit review --agent after every major milestone and for the final
    branch diff: zero actionable findings

Each new regression layer also passed a targeted sabotage check that produced
the expected assertion-level failure before the production line was restored.

References

Summary by Sourcery

Strengthen regression coverage and documentation for conditional action dependency manifests while preserving compatibility at the time-parsing and template-formatting boundaries.

Bug Fixes:

  • Preserve documented time-offset validation by rejecting absolute offsets of 24 hours or more, including both signs.
  • Restore lowercase Boolean interpolation and retain normal formatting for non-Boolean values.

Enhancements:

  • Add deterministic regression coverage for conditional action selection, including real command discovery, action-level filtering and fan-out, and selection without impure stdlib helpers.
  • Verify that dependencies on selected conditional actions and targets retain explicit, implicit, and order-only classifications through IR lowering and generated Ninja files.
  • Add end-to-end Ninja and BDD coverage for conditionally selected dependency manifests.

Build:

  • Refresh the lockfile and align documentation-coverage command configuration with explicit toolchain and threshold options.

CI:

  • Improve workflow validation handling for Markdown formatter installation and checks.

Documentation:

  • Document conditional action dependencies, deterministic command-availability testing practices, selection-time purity boundaries, Boolean formatting, and time-offset constraints.
  • Mark roadmap item 3.14.5 complete and add the associated execution plan and design guidance.

Tests:

  • Add unit, integration, IR, snapshot, real-Ninja, BDD, and property-based regression coverage for conditional manifests, dependency lowering, Ninja emission, and time-offset boundaries.

Chores:

  • Apply repository-wide formatting and diagnostic-message clean-ups.

@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 Jun 15, 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

  • Add regression coverage for conditional action dependency manifests.
  • Verify action-level when and foreach, deterministic command_available(...) resolution, selection-time purity, and dependency lowering to IR and Ninja.
  • Add in-crate, IR, BDD, snapshot, and real-Ninja integration tests.
  • Add the conditional_action_deps.yml fixture and validate explicit, implicit, order-only, and phony dependency edges.
  • Preserve lowercase Boolean formatting in MiniJinja and reject offsets of 24 hours or more.
  • Add property-based tests for UTC offset boundaries.
  • Add Markdown format checking, pinned mdtablefix installation, and CI contract tests.
  • Update the users’ guide, developers’ guide, design document, roadmap, and the completed ExecPlan at docs/execplans/3-14-5-regression-coverage-for-conditional-action-dependency-manifests.md.
  • Apply formatting, diagnostic, path-display, and test-helper clean-ups across the codebase.

Follow-up

  • Resolve the remaining review findings for doc comment coverage, now(offset=...) documentation, ExecPlan alignment, self.root borrowing, and executable-fixture reuse.

Walkthrough

The pull request adds conditional manifest regression coverage, documents command discovery and Boolean formatting, updates the standard library, tightens time-offset validation, adds Markdown formatting validation, and modernizes test diagnostics.

Changes

Netsuke regression coverage

Layer / File(s) Summary
Standard-library behaviour and documentation
src/stdlib/register.rs, src/stdlib/time/*, docs/developers-guide.md, docs/netsuke-design.md, docs/users-guide.md
Register lowercase Boolean formatting and reject offsets of 24 hours or more. Document these behaviours and related contracts.
Conditional action selection tests
src/manifest/expand_test_cases/*, src/manifest/expand_tests.rs, tests/data/conditional_action_deps.yml, docs/execplans/..., docs/roadmap.md
Test deterministic command discovery, fallback selection, directive removal, dependency selection, and impurity tracking.
Dependency lowering and Ninja validation
tests/ir_from_manifest_tests.rs, tests/features/ninja.feature, tests/ninja_snapshot_tests*
Verify explicit, implicit, order-only, and phony dependencies through the IR, generated Ninja output, snapshots, execution, and no-op checks.
Markdown formatting validation
scripts/check-markdown-format.sh, scripts/tests/test_check_markdown_format.py, Makefile, .github/workflows/ci.yml, AGENTS.md
Add staged Markdown checks with LF/CRLF handling, formatter version pinning, CI installation, and workflow contract tests.
Documentation and diagnostic maintenance
docs/*, src/**/*tests.rs, tests/**/*
Update guidance, roadmap status, assertion formatting, filesystem-path rendering, and test comments. Runtime behaviour remains unchanged for these maintenance edits.

Suggested labels: Roadmap, Issue

Poem

Conditional branches choose their way,
Ninja records each edge in play.
Booleans speak in lowercase light,
Bad time offsets lose their flight.
Markdown checks keep pages right.

Merge Risk: 🟡 Moderate · up to ae9ad

Although this change is limited to regression coverage and documentation, the current head is not merge-ready because its CI changes can fail on Windows and non-Unix test builds, while test fixtures can diverge from the repository’s shared contract. The remaining issues are bounded but require follow-up before merging.


Caution

Pre-merge checks failed

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

  • Ignore

❌ Failed checks (1 error, 2 warnings)

Check name Status Explanation Resolution
Testing (Overall) ❌ Error The core Rust tests are substantive: the offset properties cover both sides of the 24-hour boundary, existing command_available tests assert lowercase Boolean output, and the conditional manifest te… Add a Makefile contract or process-boundary test that reads the check-fmt recipe and fails when the Markdown file discovery and scripts/check-markdown-format.sh invocation are absent. Ensure the test covers the batch invocation rather t…
User-Facing Documentation ⚠️ Warning The PR changes user-visible Boolean interpolation in src/stdlib/register.rs by installing a global formatter that renders Boolean values as lowercase true or false. docs/developers-guide.md do… Add a user-facing section to docs/users-guide.md in the templating or standard-library section. State that Boolean values interpolated into string fields render as lowercase true and false, and show a short manifest example. Explain t…
Developer Documentation ⚠️ Warning The developer guide documents the new formatter boundary, Markdown tooling, and conditional-test seams. The roadmap item is checked off, and the design document records the conditional-selection and f… Update docs/execplans/3-14-5-regression-coverage-for-conditional-action-dependency-manifests.md with an addendum or revision entry for register_legacy_boolean_formatter. Record its reason, registration-wide scope, lowercase Boolean beha…
✅ Passed checks (17 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the regression-coverage work and references roadmap item 3.14.5, as required.
Description check ✅ Passed The description clearly relates to the changeset and summarises the conditional action dependency coverage, documentation updates, validation, and time-offset compatibility change.
Docstring Coverage ✅ Passed Docstring coverage is 81.32% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 91 functions across 42 files. (8 skipped: 8…
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.
Module-Level Documentation ✅ Passed Pass this check. The three new Rust modules begin with //! documentation: the two manifest expansion modules describe their selection-coverage purpose, and the conditional Ninja module explains both…
Testing (Unit And Behavioural) ✅ Passed Accept the testing coverage. The new expansion tests exercise complementary command_available branches, real executable resolution, foreach interaction, and impurity boundaries. The IR tests and B…
Testing (Property / Proof) ✅ Passed The pull request introduces a range invariant for parse_offset: signed ISO 8601 offsets with absolute hours below 24 must be accepted, and offsets at or above 24 hours must be rejected. It adds subs…
Testing (Compile-Time / Ui) ✅ Passed Mark this check as passed. The PR introduces no compile-time API or language behaviour that requires a trybuild-style test. The changed runtime behaviour is covered by focused assertions and property …
Unit Architecture ✅ Passed Pass Unit Architecture. The PR adds no new hidden side-effect path. The only production changes are an explicitly named formatter registration in register_with_config and validation in `parse_offset…
Domain Architecture ✅ Passed PASS. The pull request does not introduce a domain-boundary violation. The only production changes are in src/stdlib/register.rs and src/stdlib/time/mod.rs. register_legacy_boolean_formatter is …
Observability ✅ Passed Pass the Observability check. The committed production diff changes only Boolean template formatting and parse_offset validation. The conditional-manifest work adds tests and documentation; it does …
Security And Privacy ✅ Passed No stated security or privacy failure is introduced. The diff adds no secrets, credentials, authentication checks, permissions, or sensitive data. The only non-test runtime changes register lowercase …
Performance And Resource Use ✅ Passed Pass the performance and resource-use check. The production diff adds only a constant-time Boolean formatter branch per rendered value and one bounded offset validation; it adds no blocking I/O, hot-p…
Concurrency And State ✅ Passed Pass this check. The diff against main adds no async tasks, spawned workers, channels, locks, globals, or new shared mutable state. The existing StdlibState Arc<AtomicBool> is unchanged. The new…
Architectural Complexity And Maintainability ✅ Passed Pass this check. The change adds focused test fixtures, assertion records, and small temporary-workspace helpers with immediate use in the new regression tests. It adds no new traits, production layer…
Rust Compiler Lint Integrity ✅ Passed Accept this check. The main-to-PR diff adds no #[allow(...)] or #[expect(...)] suppression for dead_code, unused_imports, or unused. It adds no clone operation. New helpers and fields have a…
Full details: Docstring Coverage

Explanation

Docstring coverage is 81.32% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 91 functions across 42 files. (8 skipped: 8 unsupported.)

Full details: Testing (Overall)

Explanation

The core Rust tests are substantive: the offset properties cover both sides of the 24-hour boundary, existing command_available tests assert lowercase Boolean output, and the conditional manifest tests assert selected actions and explicit, implicit, and order-only Ninja dependencies. The Markdown checker tests also exercise the script process boundary and non-canonical input. However, the new check-fmt Makefile behaviour is not guarded. The checker tests invoke scripts/check-markdown-format.sh directly, while the existing Makefile contract checks assert only the Ruff command. Removing @$(MD_FILES_FIND) | xargs -0 -r scripts/check-markdown-format.sh from check-fmt would leave the new tests passing. The new CI installer contract is also only fragment-based and does not assert that the install command itself uses the pinned version.

Resolution

Add a Makefile contract or process-boundary test that reads the check-fmt recipe and fails when the Markdown file discovery and scripts/check-markdown-format.sh invocation are absent. Ensure the test covers the batch invocation rather than only the standalone script. Strengthen the CI workflow contract so both installer steps assert the pinned install argument, such as mdtablefix@${MDTABLEFIX_VERSION} or an equivalent exact-version option, in addition to checking the post-install version.

Full details: User-Facing Documentation

Explanation

The PR changes user-visible Boolean interpolation in src/stdlib/register.rs by installing a global formatter that renders Boolean values as lowercase true or false. docs/developers-guide.md documents this change, but docs/users-guide.md does not. Its only Boolean reference states that manifests can provide booleans to Jinja; it does not explain their rendered output.

Resolution

Add a user-facing section to docs/users-guide.md in the templating or standard-library section. State that Boolean values interpolated into string fields render as lowercase true and false, and show a short manifest example. Explain that non-Boolean values retain the normal escaping behaviour. Keep the existing now(offset=...) and conditional-action dependency documentation.

Full details: Developer Documentation

Explanation

The developer guide documents the new formatter boundary, Markdown tooling, and conditional-test seams. The roadmap item is checked off, and the design document records the conditional-selection and formatter boundaries. However, the new ExecPlan is not up to date with the changed code. src/stdlib/register.rs adds register_legacy_boolean_formatter, which changes production rendering behaviour and supports the conditional-command tests. The completed ExecPlan instead states that parse_offset is the sole production-beaviour boundary and that no other production semantics changed. Those claims conflict with the pull-request diff.

Resolution

Update docs/execplans/3-14-5-regression-coverage-for-conditional-action-dependency-manifests.md with an addendum or revision entry for register_legacy_boolean_formatter. Record its reason, registration-wide scope, lowercase Boolean behaviour, escape_formatter fallback, affected validation, and the fact that this is an additional production change. Correct the statements that identify parse_offset as the sole production-behaviour boundary and that runtime behaviour is unchanged. Re-run the documented documentation and validation gates.

Full details: Module-Level Documentation

Explanation

Pass this check. The three new Rust modules begin with //! documentation: the two manifest expansion modules describe their selection-coverage purpose, and the conditional Ninja module explains both its coverage and its relationship to the parent runner. All changed Rust modules retain module-level //! documentation. The new Python test module also begins with a module docstring, and the new shell script has a descriptive header comment. No changed module lacks purpose documentation.

Full details: Testing (Unit And Behavioural)

Explanation

Accept the testing coverage. The new expansion tests exercise complementary command_available branches, real executable resolution, foreach interaction, and impurity boundaries. The IR tests and BDD scenario verify explicit, implicit, and order-only dependency lowering, while the Ninja snapshot test validates generated Ninja syntax and execution when Ninja is available. The time tests cover accepted and rejected offset boundaries with property-based cases. The Markdown checker tests run the script as a process and cover argument errors, missing tools, batching, CRLF handling, mixed endings, and source immutability. Existing tests/stdlib_which_tests.rs cases also verify lowercase Boolean rendering through the registered stdlib. No explicit testing failure condition is introduced.

Full details: Testing (Property / Proof)

Explanation

The pull request introduces a range invariant for parse_offset: signed ISO 8601 offsets with absolute hours below 24 must be accepted, and offsets at or above 24 hours must be rejected. It adds substantive Rust proptest coverage in src/stdlib/time/tests.rs, generating both signs, all hour values 0–23 with all valid minute and second values, and boundary/out-of-range hour values 24–48 with all valid minute and second values. The implementation change and the property tests are present in the pull-request diff, and proptest is declared as a dev-dependency. No explicit property-testing failure condition is present.

Full details: Testing (Compile-Time / Ui)

Explanation

Mark this check as passed. The PR introduces no compile-time API or language behaviour that requires a trybuild-style test. The changed runtime behaviour is covered by focused assertions and property tests in src/stdlib/time/tests.rs and existing Boolean-rendering tests. The structured, UI-adjacent Ninja output has a focused Insta snapshot in tests/ninja_snapshot_tests/conditional_action_deps.rs with semantic assertions for explicit, implicit, order-only, and filtered dependencies. The snapshot contains deterministic SHA-256 action-content identifiers, not floating commit, path, timestamp, version, or secret data. The Markdown checker tests use stable semantic assertions because snapshotting temporary paths and diagnostic variants would add brittleness.

Full details: Unit Architecture

Explanation

Pass Unit Architecture. The PR adds no new hidden side-effect path. The only production changes are an explicitly named formatter registration in register_with_config and validation in parse_offset; the existing wall-clock call in now remains unchanged. New expansion tests inject isolated workspace, PATH, and PATHEXT configuration, and they expose fallible setup through Result. Ninja and Markdown checks isolate writes to temporary copies and use explicit process/file-operation boundaries. The new tests also verify the intended purity boundary for conditional selection and the impurity signal for shell execution.

Full details: Domain Architecture

Explanation

PASS. The pull request does not introduce a domain-boundary violation. The only production changes are in src/stdlib/register.rs and src/stdlib/time/mod.rs. register_legacy_boolean_formatter is placed in the MiniJinja registration adapter and delegates non-Boolean values to MiniJinja. parse_offset translates template input into UtcOffset and applies the documented ISO 8601 bound at the standard-library adapter boundary. Conditional-manifest changes are test-only under #[cfg(test)], or integration tests and infrastructure scripts. No core manifest, IR, persistence, transport, or domain model code gains adapter dependencies. No command is newly named as enforcing an invariant that it does not enforce.

Full details: Observability

Explanation

Pass the Observability check. The committed production diff changes only Boolean template formatting and parse_offset validation. The conditional-manifest work adds tests and documentation; it does not alter the manifest, IR, or Ninja runtime paths. The new ±24:00 rejection returns the existing InvalidOperation diagnostic, includes the supplied offset and expected syntax, and reaches human stderr or the JSON diagnostic path through the existing runner boundary. The Boolean change affects rendered text only and introduces no new failure mode, latency, throughput, resource, retry, cache, or cross-process behaviour. No new alert, metric, or trace is required by the stated conditions.

Full details: Security And Privacy

Explanation

No stated security or privacy failure is introduced. The diff adds no secrets, credentials, authentication checks, permissions, or sensitive data. The only non-test runtime changes register lowercase Boolean formatting while retaining MiniJinja escaping, and reject parsed offsets of 24 hours or more. The new formatter script quotes file and tool arguments, uses cp --, stages files in a temporary directory, and does not use eval or shell concatenation. New tests use fixed commands, temporary directories, static fake executables, and Command::arg calls. CI keeps contents: read, pins the formatter version, and adds no new secret exposure. Credential-pattern scans found no matches, and bash -n accepted the new script.

Full details: Performance And Resource Use

Explanation

Pass the performance and resource-use check. The production diff adds only a constant-time Boolean formatter branch per rendered value and one bounded offset validation; it adds no blocking I/O, hot-path process execution, unbounded collection, or new production loop. The new Markdown checker processes an explicit xargs batch once, copies files to temporary storage once, and compares each file once, giving linear work with bounded batch arguments and cleanup via trap. New integration and property tests use finite fixtures, bounded ranges, and a small fixed number of Ninja invocations. No avoidable quadratic behaviour or material allocation regression is introduced.

Full details: Concurrency And State

Explanation

Pass this check. The diff against main adds no async tasks, spawned workers, channels, locks, globals, or new shared mutable state. The existing StdlibState Arc&lt;AtomicBool&gt; is unchanged. The new resolver and Ninja tests own separate TempDir instances, use synchronous process calls, and release resources at test end. The Markdown checker uses a per-invocation mktemp -d directory with trap cleanup. The added stderr lock covers only one diagnostic write. Formatter registration mutates the caller-owned MiniJinja Environment through &amp;mut; it does not create process-wide state. The dependency tests validate explicit, implicit, and order-only ordering without introducing an application concurrency model. No stated concurrency or state failure condition is introduced.

Full details: Architectural Complexity And Maintainability

Explanation

Pass this check. The change adds focused test fixtures, assertion records, and small temporary-workspace helpers with immediate use in the new regression tests. It adds no new traits, production layers, registries, global state, module dependency cycles, or dependency declarations. The Boolean formatter is a private compatibility helper that preserves an existing manifest contract and delegates all other values to MiniJinja's existing escape_formatter. The executable tests reuse the existing test_support::write_exec_with_content helper. The Markdown checker is a direct script for a formatter that has no check-only mode, and its temporary-copy contract is documented and tested. The lockfile changes refresh resolved versions but do not add a new dependency manifest entry.

Full details: Rust Compiler Lint Integrity

Explanation

Accept this check. The main-to-PR diff adds no #[allow(...)] or #[expect(...)] suppression for dead_code, unused_imports, or unused. It adds no clone operation. New helpers and fields have active callers, and the new test modules use explicit, narrow path and cfg(unix) boundaries. The added formatter helper is called from register_with_config, and the added imports support live code. Existing #[expect] attributes in touched files concern Clippy rules, remain unchanged, and do not suppress unused-code lints.

✨ Finishing Touches 💡 2
⚔️ Resolve merge conflicts 💡
  • Resolve merge conflict in branch 3-14-5-regression-coverage-for-conditional-action-dependency-manifests
🛠️ Fix failing CI checks 💡
  • Create stacked PR
  • Commit on current branch
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch 3-14-5-regression-coverage-for-conditional-action-dependency-manifests

Warning

Your free Security trial is over. An organization admin can activate Security or dismiss this notice.


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

@leynos leynos changed the title Draft execplan: regression coverage for conditional action dependency manifests (3.14.5) Plan: Regression coverage for conditional action dependency manifests (3.14.5) Jun 19, 2026
@lodyai
lodyai Bot force-pushed the 3-14-5-regression-coverage-for-conditional-action-dependency-manifests branch from f4e21b1 to d591a6d Compare July 28, 2026 21:57
codescene-access[bot]

This comment was marked as outdated.

@lodyai lodyai Bot changed the title Plan: Regression coverage for conditional action dependency manifests (3.14.5) Regression coverage for conditional action dependency manifests (3.14.5) Jul 28, 2026
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.

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.

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.

codescene-access[bot]

This comment was marked as outdated.

codescene-access[bot]

This comment was marked as outdated.

@leynos
leynos force-pushed the 3-14-5-regression-coverage-for-conditional-action-dependency-manifests branch from 87a0993 to cbad3ac Compare August 14, 2026 21:59
codescene-access[bot]

This comment was marked as outdated.

codescene-access[bot]

This comment was marked as outdated.

@leynos
leynos force-pushed the 3-14-5-regression-coverage-for-conditional-action-dependency-manifests branch from cbad3ac to a857fde Compare August 16, 2026 22:25
codescene-access[bot]

This comment was marked as outdated.

codescene-access[bot]

This comment was marked as outdated.

leynos added 15 commits August 31, 2026 09:38
Regenerate `Cargo.lock` from the merged manifest and keep the refreshed
toolchain's diagnostic assertions lint-clean.
Keep the real-Ninja regression workflow intact while isolating workspace,
command, selection, timestamp, and no-op responsibilities.
Regenerate `Cargo.lock` from the merged manifest and retain `main`'s
Unix-only policy for native Ninja execution while preserving the
conditional dependency regression coverage.
Use the path display representation in the upstream ladder assertion so
the target's strict Clippy configuration accepts the rebased test suite.
Preserve the existing UTF-8 conversion context while rendering invalid
temporary paths with their display representation for strict Clippy.
Regenerate `Cargo.lock` from the merged manifest and update the upstream
runner test diagnostic for the target branch's strict Clippy policy.
Format non-UTF-8 temporary paths through `Display` and compile the
executable-permission helper only where the test needs it.
Rebuild the dependency lockfile from the main-side manifest after the
rebase so it records the current compatible resolver output.
Expect MiniJinja's rendered Boolean spelling while retaining the assertion
that the excluded command is unavailable.
Preserve the path-conversion errors while satisfying Clippy's portable
formatting requirement after the rebase.
Keep existing manifests stable across the MiniJinja formatter change and
make the Windows command resolver fixture independent of host PATHEXT.
Apply the enforced Markdown table and paragraph formatting to the
overlapping documentation changes after rebasing onto the new PR base.
Preserve the conditional manifest contracts while removing redundant
coverage, reusing shared test fixtures, and documenting the offset boundary
and Ninja generation behaviour.
Record the target formatter contract and the target-baseline lockfile
regeneration so the completed ExecPlan remains reproducible.
Document the reviewed compatibility boundaries and add focused UTC
shorthand coverage. Keep executable fixtures shared, preserve lossless
diagnostics, and make workflow validation structurally typed.
@leynos
leynos force-pushed the 3-14-5-regression-coverage-for-conditional-action-dependency-manifests branch from ae9adec to a396543 Compare August 31, 2026 08:01
codescene-access[bot]

This comment was marked as outdated.

codescene-access[bot]

This comment was marked as outdated.

@leynos

This comment was marked as resolved.

@coderabbitai

This comment was marked as resolved.

Keep the Makefile contract from silently losing Markdown discovery or
format-checker batching.
codescene-access[bot]

This comment was marked as outdated.

@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.

@coderabbitai

coderabbitai Bot commented Aug 31, 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' '--- PR metadata and current checkout ---'
git rev-parse HEAD
gh pr view 387 --repo leynos/netsuke \
  --json url,baseRefOid,headRefOid,headRefName \
  --jq '{url,headRefName,baseRefOid,headRefOid}'

printf '%s\n' '--- current PR checks ---'
gh pr checks 387 --repo leynos/netsuke --json name,state,workflow,link \
  --jq '.[] | [.name,.state,.workflow,.link] | `@tsv`' || true

printf '%s\n' '--- relevant file outlines ---'
ast-grep outline tests/workflow_contracts/ci_lint_test.py --items all || true
ast-grep outline src/stdlib/register.rs --items all || true

printf '%s\n' '--- Makefile recipe and contract test ---'
rg -n -A35 -B8 \
  '^(check-fmt|test-markdown-format|MD_FILES_FIND):|check-markdown-format|def test_makefile_check_fmt_runs_markdown_format_checker' \
  Makefile tests/workflow_contracts/ci_lint_test.py

printf '%s\n' '--- boolean formatting and user guide ---'
rg -n -i -A24 -B8 \
  'register_legacy_boolean_formatter|boolean interpolation|lowercase.*(true|false)|true.*false|template' \
  src/stdlib/register.rs docs/users-guide.md

printf '%s\n' '--- ExecPlan boolean formatter entries ---'
rg -n -i -A28 -B10 \
  'register_legacy_boolean_formatter|boolean|lowercase|formatter|revision' \
  docs/execplans/3-14-5-regression-coverage-for-conditional-action-dependency-manifests.md

printf '%s\n' '--- changes against PR base in affected paths ---'
base=$(gh pr view 387 --repo leynos/netsuke --json baseRefOid --jq .baseRefOid)
git diff --unified=6 "$base" HEAD -- \
  Makefile tests/workflow_contracts/ci_lint_test.py src/stdlib/register.rs \
  docs/users-guide.md \
  docs/execplans/3-14-5-regression-coverage-for-conditional-action-dependency-manifests.md

printf '%s\n' '--- existing related issues ---'
gh issue list --repo leynos/netsuke --state all --limit 100 \
  --search '"check-fmt" "check-markdown-format" OR "boolean formatter"' \
  --json number,title,state,url

Length of output: 63251


@leynos Files: Makefile, tests/workflow_contracts/ci_lint_test.py, docs/users-guide.md, docs/execplans/3-14-5-regression-coverage-for-conditional-action-dependency-manifests.md
Symbols: check-fmt, test_makefile_check_fmt_runs_markdown_format_checker, register_legacy_boolean_formatter

Treat all three reported checks as resolved at PR head a2e9cd8f6c7e2afc7e0728c680dbafc2715b40c5.

Check Status Current evidence
Testing (Overall) Resolved tests/workflow_contracts/ci_lint_test.py::test_makefile_check_fmt_runs_markdown_format_checker now reads only the check-fmt recipe. It requires $(MD_FILES_FIND), xargs -0 -r, and scripts/check-markdown-format.sh. It also requires exactly one occurrence of @$(MD_FILES_FIND) | xargs -0 -r scripts/check-markdown-format.sh.
User-Facing Documentation Resolved docs/users-guide.md states that Boolean interpolation into string fields produces lowercase true and false. It includes a YAML manifest example that writes true to status.txt.
Developer Documentation Resolved The ExecPlan records register_legacy_boolean_formatter, its registration-wide scope, its compatibility purpose, lowercase Boolean rendering, and its revision history.

Current CI also reports success for build-test, build-test-windows, CodeScene Code Health Review, CodeScene Code Coverage, and CodeRabbit.

Do not create a follow-up GitHub issue. No further AI coding-agent prompt is required.

The checkout lacks the PR base object, so I could not generate a local diff against 784bf911954b7b3108ab413db277e3ac90ca4f3a. This limitation does not affect the direct current-codegraph inspection or the reported successful checks.

You are interacting with an AI system.

@leynos

leynos commented Aug 31, 2026

Copy link
Copy Markdown
Owner Author

@coderabbitai Have the following now been resolved?

docs/netsuke-design.md (1)

2390-2394: 📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy lift
Consolidate the Ninja-generation design into one canonical §5.4.
Remove the duplicated direct and serial copies, or split the material into

uniquely numbered subsections. Lines 2392-2526 repeat the generation contract

already described earlier in this document. The repeated sections now contain

different detail levels, such as control-character rejection, so they create

multiple sources of truth for one boundary.
Triage: [type:docstyle]

🤖 Detailed instructions

Use a wyvern agent team to verify each finding against the current code and only fix it if needed. Fix any documentation issues using a scribe agent team. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. Use a scrutineer agent for execution of tests and summarizing the results.

In @docs/netsuke-design.md around lines 2390 - 2394, Consolidate the duplicated
Ninja-generation material under one canonical §5.4: remove the repeated direct
and serial copies or assign each distinct topic a unique subsection number.
Ensure details such as control-character rejection appear in only one
authoritative section, preserving all necessary generation-contract content
without conflicting sources of truth.

Sources: Coding guidelines, Path instructions

@leynos

leynos commented Aug 31, 2026

Copy link
Copy Markdown
Owner Author

@coderabbitai Have the following now been resolved?

AGENTS.md (1)

221-221: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Document the executed doc-coverage command.
Add RUSTDOCFLAGS, --toolchain "$DOC_COVERAGE_TOOLCHAIN", and

--threshold "$DOC_COVERAGE_THRESHOLD" to this example. The Makefile passes

these values, but the documented command omits them.

🤖 Detailed instructions

Use a wyvern agent team to verify each finding against the current code and only fix it if needed. Fix any documentation issues using a scribe agent team. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. Use a scrutineer agent for execution of tests and summarizing the results.

In @AGENTS.md at line 221, Update the documented doc-coverage command to include
the RUSTDOCFLAGS environment setting, the --toolchain "$DOC_COVERAGE_TOOLCHAIN"
option, and the --threshold "$DOC_COVERAGE_THRESHOLD" option, matching the
values passed by the Makefile.

@coderabbitai

coderabbitai Bot commented Aug 31, 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 revision ---'
git rev-parse HEAD
gh pr view 387 --repo leynos/netsuke \
  --json url,baseRefOid,headRefOid,headRefName \
  --jq '{url,headRefName,baseRefOid,headRefOid}'

printf '%s\n' '--- current PR checks ---'
gh pr checks 387 --repo leynos/netsuke --json name,state,workflow,link \
  --jq '.[] | [.name,.state,.workflow,.link] | `@tsv`' || true

printf '%s\n' '--- Markdown heading map ---'
rg -n '^(#{1,6})[[:space:]]+' docs/netsuke-design.md

printf '%s\n' '--- reported section, with context ---'
sed -n '2310,2560p' docs/netsuke-design.md

printf '%s\n' '--- Ninja generation and control-character contract occurrences ---'
rg -n -i -C 5 \
  'ninja (generation|synthesis|emission|file)|serial depend|direct depend|control.character|control character|reject.*character|character.*reject|build edge|order.only' \
  docs/netsuke-design.md

printf '%s\n' '--- document changes in this PR ---'
base=$(gh pr view 387 --repo leynos/netsuke --json baseRefOid --jq .baseRefOid)
git diff --unified=8 "$base" HEAD -- docs/netsuke-design.md

Length of output: 41480


@leynos File: docs/netsuke-design.md
Symbols: §5.4 Ninja file synthesis, Serial dependency ordering and generated bundles

Mark this finding as resolved.

The current PR head is a2e9cd8f6c7e2afc7e0728c680dbafc2715b40c5. The document has one canonical Ninja-generation section: ### 5.4 Ninja file synthesis (src/ninja_gen/mod.rs) at Line 2117. It contains the authoritative backend conversion contract, including rejection of newline, carriage-return, and NUL characters at Lines 2141-2148.

#### Serial dependency ordering and generated bundles at Line 2185 is a distinct subsection of §5.4. It describes the exceptional serial-dependency lowering path. It does not duplicate the direct Ninja-generation contract.

The reported Lines 2390-2394 now occur in Section 6.1. This section describes Ninja process invocation. It does not define a second Ninja-generation contract. Do not merge or remove that material. Such a change would combine generation and process-execution boundaries and reduce document clarity.

Required checks are successful at the current head. This includes build-test, build-test-windows, CodeScene Code Health Review, and CodeScene Code Coverage. No AI coding-agent prompt is required.

You are interacting with an AI system.

@coderabbitai

coderabbitai Bot commented Aug 31, 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' '--- PR revision ---'
git rev-parse HEAD
gh pr view 387 --repo leynos/netsuke --json headRefOid,baseRefOid,url \
  --jq '{url,baseRefOid,headRefOid}'

printf '%s\n' '--- AGENTS.md doc-coverage documentation ---'
rg -n -A12 -B12 'doc-coverage|DOC_COVERAGE_TOOLCHAIN|DOC_COVERAGE_THRESHOLD|RUSTDOCFLAGS' AGENTS.md

printf '%s\n' '--- Makefile doc-coverage variables and recipe ---'
rg -n -A24 -B12 '^(doc-coverage|DOC_COVERAGE_TOOLCHAIN|DOC_COVERAGE_THRESHOLD|RUSTDOCFLAGS)\b|cargo.*doc.*coverage' Makefile

printf '%s\n' '--- current PR checks ---'
gh pr checks 387 --repo leynos/netsuke --json name,state,workflow,link \
  --jq '.[] | [.name,.state,.workflow,.link] | `@tsv`' || true

printf '%s\n' '--- relevant diff against PR base ---'
base=$(gh pr view 387 --repo leynos/netsuke --json baseRefOid --jq .baseRefOid)
git diff --unified=12 "$base" HEAD -- AGENTS.md Makefile || true

Length of output: 14945


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '--- RUSTDOC_FLAGS definitions and uses ---'
rg -n -A4 -B4 '^\s*RUSTDOC_FLAGS\s*[:?+]?=|RUSTDOC_FLAGS|RUSTDOCFLAGS' Makefile

printf '%s\n' '--- exact documented and executed commands ---'
sed -n '217,225p' AGENTS.md
sed -n '181,186p' Makefile

Length of output: 2894


@leynos File: AGENTS.md
Symbol: make doc-coverage command documentation

Treat this finding as resolved at PR head a2e9cd8f6c7e2afc7e0728c680dbafc2715b40c5.

AGENTS.md Lines 220-223 now documents:

RUSTDOCFLAGS="--cfg docsrs -D warnings" \
uv run --no-project --python $(PYTHON_BASELINE) \
  scripts/doc-coverage.py --toolchain "$DOC_COVERAGE_TOOLCHAIN" \
  --threshold "$DOC_COVERAGE_THRESHOLD"

The current Makefile recipe passes the same required inputs:

  • RUSTDOCFLAGS="$${RUSTDOC_FLAGS}"
  • --toolchain "$$DOC_COVERAGE_TOOLCHAIN"
  • --threshold "$$DOC_COVERAGE_THRESHOLD"

Makefile Line 125 defines the default RUSTDOC_FLAGS value as --cfg docsrs -D warnings, so the documented command matches the executed default command. The Makefile supports an override, but that does not invalidate the documented default.

Keep the current documentation. Do not create a GitHub issue.

You are interacting with an AI system.

@leynos
leynos merged commit 53de154 into main Aug 31, 2026
18 checks passed
@leynos
leynos deleted the 3-14-5-regression-coverage-for-conditional-action-dependency-manifests branch August 31, 2026 15:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants