Regression coverage for conditional action dependency manifests (3.14.5) - #387
Conversation
|
Note Reviews pausedIt 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 Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
Summary
Follow-up
WalkthroughThe 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. ChangesNetsuke regression coverage
Suggested labels: Poem
Merge Risk: 🟡 Moderate · up to 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 failedPlease resolve all errors before merging. Addressing warnings is optional.
❌ Failed checks (1 error, 2 warnings)
✅ Passed checks (17 passed)
Full details: Docstring CoverageExplanation 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 Resolution Add a Makefile contract or process-boundary test that reads the Full details: User-Facing DocumentationExplanation The PR changes user-visible Boolean interpolation in Resolution Add a user-facing section to Full details: Developer DocumentationExplanation 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. Resolution Update Full details: Module-Level DocumentationExplanation Pass this check. The three new Rust modules begin with Full details: Testing (Unit And Behavioural)Explanation Accept the testing coverage. The new expansion tests exercise complementary Full details: Testing (Property / Proof)Explanation The pull request introduces a range invariant for 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 Full details: Unit ArchitectureExplanation Pass Unit Architecture. The PR adds no new hidden side-effect path. The only production changes are an explicitly named formatter registration in Full details: Domain ArchitectureExplanation PASS. The pull request does not introduce a domain-boundary violation. The only production changes are in Full details: ObservabilityExplanation Pass the Observability check. The committed production diff changes only Boolean template formatting and Full details: Security And PrivacyExplanation 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 Full details: Performance And Resource UseExplanation 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 Full details: Concurrency And StateExplanation Pass this check. The diff against Full details: Architectural Complexity And MaintainabilityExplanation 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 Full details: Rust Compiler Lint IntegrityExplanation Accept this check. The main-to-PR diff adds no ✨ Finishing Touches 💡 2⚔️ Resolve merge conflicts 💡
🛠️ Fix failing CI checks 💡
📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning Your free Security trial is over. An organization admin can activate Security or dismiss this notice. Comment |
f4e21b1 to
d591a6d
Compare
87a0993 to
cbad3ac
Compare
cbad3ac to
a857fde
Compare
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.
ae9adec to
a396543
Compare
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
Keep the Makefile contract from silently losing Markdown discovery or format-checker batching.
|
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,urlLength of output: 63251
Treat all three reported checks as resolved at PR head
Current CI also reports success for 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 You are interacting with an AI system. |
|
@coderabbitai Have the following now been resolved? docs/netsuke-design.md (1)
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. 🤖 Detailed instructionsUse 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 Sources: Coding guidelines, Path instructions |
|
@coderabbitai Have the following now been resolved? AGENTS.md (1)
these values, but the documented command omits them. 🤖 Detailed instructionsUse 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 |
|
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.mdLength of output: 41480
Mark this finding as resolved. The current PR head is
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 You are interacting with an AI system. |
|
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 || trueLength 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' MakefileLength of output: 2894
Treat this finding as resolved at PR head
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
Keep the current documentation. Do not create a GitHub issue. You are interacting with an AI system. |
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:
whenandforeachwork together.command_available(...)resolution selects exactly one nextest orlegacy action in deterministic present and absent tool worlds.
selection, with a shell-in-
whencontrol proving the observable is live.depslower to IRimplicit_depsandNinja
|inputs, including{{ item }}substitution and||order-onlydependencies.
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 fmtmake markdownlintmake nixiemake check-fmtmake typecheckmake lintINSTA_UPDATE=no make testcoderabbit review --agentafter every major milestone and for the finalbranch 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
docs/execplans/3-14-5-regression-coverage-for-conditional-action-dependency-manifests.mddocs/roadmap.md§3.14.5Summary 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:
Enhancements:
Build:
CI:
Documentation:
Tests:
Chores: