Narrow the build script's module graph (#513) - #514
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
Validation
WalkthroughThe PR separates CLI command, preference, and validation responsibilities, narrows ChangesCLI command and validation
Build-script module slice
Host matching
CLI schema tests
CLI layout documentation
Sequence Diagram(s)sequenceDiagram
participant BuildScript
participant CliCommand
participant CliConfig
participant CliValidation
BuildScript->>CliCommand: construct Cli command schema
CliCommand->>CliConfig: resolve configuration types
CliConfig->>CliValidation: apply validation policies
CliCommand-->>BuildScript: return command data
BuildScript->>BuildScript: generate man page
Suggested labels: Poem
Merge Risk: 🟡 Moderate · up to The PR narrows build-script compilation without changing the intended CLI schema, but merge observation still runs from a query path and can cause externally visible side effects for read-only callers; a platform-sensitive boundary test also needs hardening. Merge should wait for these bounded issues to be fixed or explicitly accepted. Caution Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional.
❌ Failed checks (1 error, 4 warnings)
✅ Passed checks (15 passed)
Full details: Linked Issues checkExplanation The changes satisfy issue Full details: Docstring CoverageExplanation Docstring coverage is 92.50% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 40 functions across 24 files. (2 skipped: 2 unsupported.) Full details: Testing (Overall)Explanation The added tests substantially cover command parsing, host matching, validation, preference accessors, and the supported/unsupported build-module slice. However, the pull request also changes the observable Cargo rerun contract in Resolution Add a focused build-script contract test for the exact Full details: User-Facing DocumentationExplanation Fail: the PR introduces user-facing host-matching behaviour without updating Resolution Update the “Configure network access” section in Full details: Developer DocumentationExplanation Mark this check PASS. Full details: Module-Level DocumentationExplanation Pass the module-level documentation check. Every added or modified Rust module has a module docstring. The new Full details: Testing (Unit And Behavioural)Explanation The added tests cover the main behaviour: command parsing uses the public parser boundary, host matching has edge and property tests, validation has unit tests, and the build-slice fixture compiles production modules and rejects Resolution Normalize the contents read from Full details: Testing (Property / Proof)Explanation Pass this check. The introduced hostname-matching rules use substantive Full details: Testing (Compile-Time / Ui)Explanation The PR adds a direct- Resolution Normalise Full details: Unit ArchitectureExplanation PASS: The pull request improves separation. The actual diff moves the Clap schema into Full details: Domain ArchitectureExplanation Keep the new boundaries. Full details: ObservabilityExplanation Instrument the changed network-policy decision path. Resolution Either remove the terminal-dot normalisation if this behaviour is not intended, or add bounded observability at the fetch policy boundary. Emit a trace event for allowed and rejected evaluations with stable fields such as Full details: Security And PrivacyExplanation PASS. The committed diff contains no secrets, credentials, tokens, certificates, or sensitive fixture data. The CLI types and Full details: Performance And Resource UseExplanation PASS. The pull request does not introduce a performance or resource-use failure. The production matching path remains a linear scan over the existing host-pattern lists, with one ASCII lowercase allocation per candidate as before; the new terminal-dot check is constant-time. The CLI refactor moves schema and preference code without adding hot-path loops, retries, blocking I/O, caches, or unbounded collections. Build-script work is reduced by compiling a narrower module slice. New test loops and generated inputs have explicit small bounds, and the direct Cargo/rustc invocations run once for two fixed fixtures rather than in a runtime path. Full details: Concurrency And StateExplanation Pass the check. The pull request narrows the Full details: Architectural Complexity And MaintainabilityExplanation Accept the change. The new Full details: Rust Compiler Lint IntegrityExplanation PASS. The PR removes the five broad build-script ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning Your free Security trial is over. An organization admin can activate Security or dismiss this notice. Comment |
Reviewer's GuideRefactors the CLI and host pattern modules to carve out a minimal, self-contained slice that the build script recompiles for man-page and localization audits, removing broad dead-code suppressions while keeping behavior and public API unchanged. File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![codescene-access[bot]](https://avatars.githubusercontent.com/in/3755875?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
|
@coderabbitai review |
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@build.rs`:
- Around line 31-35: Update the boundary documentation in build.rs lines 31-35
to state that src/cli/command.rs contains command-schema and default-command
behavior, including Cli::with_default_command; do not move the method.
Synchronize the corresponding boundary statement in docs/developers-guide.md
lines 391-393 with the same wording and scope.
In `@docs/developers-guide.md`:
- Around line 382-384: Reconcile the build-script description in
docs/developers-guide.md with the generate_man_page call and the statement that
build.rs only performs localization auditing. Explicitly state whether build.rs
stages a man page or cargo-orthohelp is the sole generator, then update the
related maintenance guidance so it presents one consistent rule and preserves
docs/ as the source of truth.
In `@src/cli/command.rs`:
- Around line 117-124: Add Rustdoc usage and outcome examples for each affected
public/shared function: in src/cli/command.rs lines 117-124, document
with_default_command() selecting Commands::Build when command is None; in
src/cli/preferences.rs lines 14-44, document each policy-to-preference mapping
with examples; and in src/cli/validation.rs lines 15-20, describe the produced
OrthoError::Validation and show caller context.
In `@src/host_matching.rs`:
- Around line 21-23: Expand the documentation for HostPattern::matches with a #
Examples section demonstrating an exact host match, a wildcard subdomain match,
and rejection of the wildcard apex; show the expected boolean outcomes for each
case while preserving the existing implementation.
- Around line 23-35: Update HostPattern::matches to remove one trailing DNS dot
from the lowercased candidate hostname before applying exact or wildcard
matching. Preserve the existing wildcard subdomain-only behavior after
normalization, and add regression coverage for trailing-dot hosts against both
exact and wildcard patterns.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 74c58235-8cae-4681-8a68-6c1908f2098a
📒 Files selected for processing (17)
build.rsdocs/developers-guide.mddocs/netsuke-design.mdsrc/cli/command.rssrc/cli/config.rssrc/cli/diag.rssrc/cli/discovery.rssrc/cli/merge.rssrc/cli/mod.rssrc/cli/parser.rssrc/cli/parsing.rssrc/cli/preferences.rssrc/cli/validation.rssrc/host_matching.rssrc/host_pattern.rssrc/lib.rssrc/stdlib/network/policy/mod.rs
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
leynos/rstest-bdd(auto-detected)leynos/ortho-config(auto-detected)leynos/shared-actions(auto-detected)
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
af77331 to
e9c8a02
Compare
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
e9c8a02 to
0c5157f
Compare
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
|
@coderabbitai review |
✅ Action performedReview finished.
|
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
0816a76 to
a7992b1
Compare
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
8ae7d1c to
1997684
Compare
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
|
@coderabbitai review |
|
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
|
@coderabbitai Have the following now been resolved? src/cli/merge.rs (1)
🤖 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 failed checks now been resolved? If further work is required, please provide an AI agent prompt for the remaining work to be done to address these failures. Do not treat warnings as optional or aspirational. Where a change is out of scope for this PR, propose a GitHub issue unless one exists already. (Treat o11y, code safety, documentation and validation coverage as in scope). ❌ Failed checks (1 error, 4 warnings)
|
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '=== PR head and relevant changed files ==='
gh pr view 514 --repo leynos/netsuke --json headRefOid,updatedAt \
--jq '{headRefOid, updatedAt}'
gh api --paginate 'repos/leynos/netsuke/pulls/514/files?per_page=100' \
--jq '.[] | select(.filename | test("(^|/)(merge|merge_observability|merge_input|cli|developer).*\\.rs$|^docs/")) | [.status,.filename,.additions,.deletions] | `@tsv`'
echo '=== Repository guidance candidates ==='
fd -HI -t f . | rg '(^|/)(AGENTS|CONTRIBUTING|CODING|PATH.*INSTRUCTIONS|README)\.(md|txt)$|coderabbit.*ya?ml$' | while IFS= read -r f; do
echo "--- $f"
sed -n '1,240p' "$f"
done
echo '=== Source outlines ==='
for f in src/cli/merge.rs src/cli/merge_observability.rs src/cli/merge_input.rs; do
if [ -f "$f" ]; then
echo "--- $f"
ast-grep outline "$f" --items all
fi
done
echo '=== Merge implementation and observer interfaces ==='
rg -n -C 8 --glob '*.rs' \
'trait\s+MergeObserver\b|struct\s+\w*Merge\w*Observer\b|enum\s+\w*Merge\w*Event\b|fn\s+merge_with_cached_file_layers_with_observer\b|\.observe\s*\(|observe_validation_rejection\b|push_(defaults|discovered_file|environment|cli)_layer\b' \
src tests
echo '=== Call sites of the merge entry points ==='
rg -n -C 6 --glob '*.rs' \
'\bmerge_with_cached_file_layers_with_observer\s*\(|\bmerge_with_cached_file_layers\s*\(' \
src tests
echo '=== Relevant PR diff ==='
base=$(gh pr view 514 --repo leynos/netsuke --json baseRefName --jq .baseRefName)
git diff --unified=40 "origin/${base}...HEAD" -- \
src/cli/merge.rs src/cli/merge_observability.rs src/cli/merge_input.rs tests docs || trueLength of output: 55610 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '=== Application adapter replay ==='
nl -ba src/config_load.rs | sed -n '135,220p'
echo '=== Merge query and event collection ==='
nl -ba src/cli/merge.rs | sed -n '45,235p'
echo '=== Observer contract ==='
nl -ba src/cli/merge_observability.rs | sed -n '24,185p'
echo '=== Behavioural coverage for replay, ordering, and rejection ==='
ast-grep outline tests/cli_tests/merge_logging.rs --items all
rg -n -C 10 --glob '*.rs' \
'MergeEvent|EventCollector|event.*order|order.*event|ValidationRejected|TracingMergeObserver|merge_and_capture|merge_with_cached_file_layers_with_observer' \
tests/cli_tests/merge_logging.rs tests/ui/config_cached_discovery_embedder_pass.rs src/cli/merge.rs
echo '=== GitHub patch for merge-related PR files ==='
gh api --paginate 'repos/leynos/netsuke/pulls/514/files?per_page=100' \
--jq '.[] | select(.filename == "src/cli/merge.rs" or .filename == "src/config_load.rs" or .filename == "src/cli/merge_observability.rs" or .filename == "tests/cli_tests/merge_logging.rs") | "FILE: \(.filename)\nPATCH:\n\(.patch // "<binary or omitted>")\n---"'
echo '=== Check status ==='
gh pr view 514 --repo leynos/netsuke --json statusCheckRollup \
--jq '[.statusCheckRollup[] | {name, status, conclusion}]'Length of output: 1680 🏁 Script executed: #!/bin/bash
set -euo pipefail
show_range() {
local file="$1"
local start="$2"
local end="$3"
echo "=== ${file}:${start}-${end} ==="
sed -n "${start},${end}p" "$file"
}
show_range src/config_load.rs 135 220
show_range src/cli/merge.rs 45 235
show_range src/cli/merge_observability.rs 24 185
echo '=== Merge logging test outline ==='
ast-grep outline tests/cli_tests/merge_logging.rs --items all
echo '=== Behavioural coverage for replay, ordering, and rejection ==='
rg -n -C 12 --glob '*.rs' \
'MergeEvent|EventCollector|event.*order|order.*event|ValidationRejected|TracingMergeObserver|merge_and_capture|merge_with_cached_file_layers_with_observer' \
tests/cli_tests/merge_logging.rs tests/ui/config_cached_discovery_embedder_pass.rs src/cli/merge.rs
echo '=== GitHub patch for merge-related PR files ==='
gh api --paginate 'repos/leynos/netsuke/pulls/514/files?per_page=100' \
--jq '.[] | select(.filename == "src/cli/merge.rs" or .filename == "src/config_load.rs" or .filename == "src/cli/merge_observability.rs" or .filename == "tests/cli_tests/merge_logging.rs") | "FILE: \(.filename)\nPATCH:\n\(.patch // "<binary or omitted>")\n---"'
echo '=== Check status ==='
gh pr view 514 --repo leynos/netsuke --json statusCheckRollup \
--jq '[.statusCheckRollup[] | {name, status, conclusion}]'Length of output: 67633
(OrthoResult<Cli>, Vec<MergeEvent>)The merge query does not accept a
The tests support this boundary:
The PR checks are successful, including 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
echo '=== PR head and checks ==='
gh pr view 514 --repo leynos/netsuke \
--json headRefOid,updatedAt,statusCheckRollup \
--jq '{headRefOid,updatedAt,checks:[.statusCheckRollup[]|{name,status,conclusion,detailsUrl}]}' || true
echo '=== Changed files ==='
base=$(gh pr view 514 --repo leynos/netsuke --json baseRefName --jq '.baseRefName')
git diff --name-status "origin/${base}...HEAD"
echo '=== Relevant source outlines ==='
for f in build.rs tests/build_module_slice_ui_tests.rs src/host_matching.rs src/host_pattern.rs src/stdlib/network/policy/mod.rs docs/users-guide.md; do
if [ -f "$f" ]; then
echo "--- $f"
ast-grep outline "$f" --items all 2>/dev/null || true
fi
done
echo '=== Build rerun directives and their tests ==='
rg -n -C 5 --glob '*.rs' \
'rerun-if-changed|rerun.*changed|generate_man_page|fn main\s*\(' \
build.rs tests src || true
echo '=== UI assertion normalisation and regression coverage ==='
rg -n -C 5 --glob '*.rs' \
'assert_fixture_matches_build|replace\("\\r\\n"|CRLF|crlf|widened.*boundary|discovery' \
tests/build_module_slice_ui_tests.rs tests/ui || true
echo '=== Host matching and fetch-policy observability ==='
rg -n -C 5 --glob '*.rs' \
'trim_end_matches|strip_suffix|terminal.*dot|HostCandidate|\.matches\(|NetworkPolicy::evaluate|fn evaluate|tracing::|trace!' \
src/host_matching.rs src/host_pattern.rs src/stdlib/network/policy || true
echo '=== User documentation for network matching ==='
rg -n -C 5 \
'Configure network access|host pattern|wildcard|terminal DNS|terminal.*dot|example\.com' \
docs/users-guide.md || true
echo '=== Difference from origin/main for affected production files ==='
git diff --unified=25 "origin/${base}...HEAD" -- \
build.rs src/host_matching.rs src/host_pattern.rs src/stdlib/network/policy/mod.rs docs/users-guide.md \
tests/build_module_slice_ui_tests.rs || trueLength of output: 5779 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '=== PR file list ==='
gh api --paginate 'repos/leynos/netsuke/pulls/514/files?per_page=100' \
--jq '.[] | [.filename, .status, .additions, .deletions] | `@tsv`'
echo '=== Current relevant sources ==='
for f in \
build.rs \
tests/build_module_slice_ui_tests.rs \
src/host_matching.rs \
src/host_pattern.rs \
src/stdlib/network/policy/mod.rs \
docs/users-guide.md
do
echo "--- $f"
nl -ba "$f" | sed -n '1,360p'
done
echo '=== Focused directive, UI, and observability references ==='
rg -n -C 8 --glob '*.rs' \
'rerun-if-changed|rerun_directive|rerun.*path|assert_fixture_matches_build|build_module_slice_source|CRLF|crlf|NetworkPolicy|fn evaluate|trace!\s*\(|event!\s*\(|debug!\s*\(' \
build.rs tests src || true
echo '=== Base-to-head GitHub patches for affected files ==='
gh api --paginate 'repos/leynos/netsuke/pulls/514/files?per_page=100' --jq \
'.[] | select(.filename == "build.rs" or .filename == "tests/build_module_slice_ui_tests.rs" or .filename == "src/host_matching.rs" or .filename == "src/host_pattern.rs" or .filename == "src/stdlib/network/policy/mod.rs" or .filename == "docs/users-guide.md") | "FILE: \(.filename)\nPATCH:\n\(.patch // "<no textual patch>")\n---"'Length of output: 3449 🏁 Script executed: #!/bin/bash
set -euo pipefail
show() {
file="$1"
echo "=== ${file} ==="
sed -n '1,420p' "$file"
}
show build.rs
show tests/build_module_slice_ui_tests.rs
show src/host_matching.rs
show src/stdlib/network/mod.rs
show src/stdlib/network/observability_tests.rs
show docs/users-guide.md
echo '=== Relevant source and test matches ==='
rg -n -C 7 --glob '*.rs' \
'rerun-if-changed|rerun.*changed|RERUN|assert_fixture_matches_build|replace\("\\r\\n"|CRLF|crlf|NetworkPolicy|fn evaluate|trace!\s*\(|event!\s*\(|debug!\s*\(|operation.?=.?fetch|allowed|rejected' \
build.rs tests src || true
echo '=== PR patches for the affected findings ==='
gh api --paginate 'repos/leynos/netsuke/pulls/514/files?per_page=100' --jq \
'.[] | select(.filename == "build.rs" or .filename == "tests/build_module_slice_ui_tests.rs" or .filename == "docs/users-guide.md" or .filename == "src/stdlib/network/mod.rs" or .filename == "src/stdlib/network/observability_tests.rs" or .filename == "src/host_matching.rs") | "FILE: \(.filename)\nPATCH:\n\(.patch // "<no textual patch>")\n---"'Length of output: 148363
The PR now addresses the reported validation, documentation, portability, UI-boundary, and observability requirements. You are interacting with an AI system. |
|
@coderabbitai Have the following now been resolved? src/cli/merge.rs (1)
🤖 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 |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '=== PR head and checks ==='
gh pr view 514 --repo leynos/netsuke --json headRefOid,statusCheckRollup \
--jq '{headRefOid, checks: [.statusCheckRollup[] | {name, status, conclusion}]}' || true
echo '=== Relevant file map ==='
fd -HI -t f . src tests | rg '(^|/)(merge(\.rs|_observability\.rs|_logging\.rs)|config_load\.rs|config_cached_discovery_embedder_pass\.rs)$' || true
echo '=== Relevant source outlines ==='
for file in src/cli/merge.rs src/cli/merge_observability.rs src/config_load.rs tests/cli_tests/merge_logging.rs tests/ui/config_cached_discovery_embedder_pass.rs; do
if [ -f "$file" ]; then
echo "--- $file"
ast-grep outline "$file" --items all 2>/dev/null || true
fi
done
echo '=== Query, event, observer, and adapter evidence ==='
rg -n -C 10 --glob '*.rs' \
'\bfn\s+merge_with_cached_file_layers_with_observer\b|\btrait\s+MergeObserver\b|\benum\s+MergeEvent\b|\bfn\s+merge_cli_or_exit\b|\.observe\s*\(|\bTracingMergeObserver\b|\bpush_(defaults_layer|discovered_file_layers|environment_layer|cli_layer)\b|\bobserve_validation_rejection\b' \
src tests || true
echo '=== Relevant PR diff ==='
base="$(gh pr view 514 --repo leynos/netsuke --json baseRefName --jq '.baseRefName')"
git diff --unified=25 "origin/${base}...HEAD" -- \
src/cli/merge.rs src/cli/merge_observability.rs src/config_load.rs \
tests/cli_tests/merge_logging.rs tests/ui/config_cached_discovery_embedder_pass.rs || trueLength of output: 35871
The test coverage supports this boundary.
The current You are interacting with an AI system. |
Compile only the schema slice required for `Cli::command()` in `build.rs`. Keep parsing, preferences, validation, host matching, and runtime discovery in sibling modules so build-script dead-code analysis remains meaningful. Preserve the existing `help` command and CLI identity within the schema slice, and document the four schema-only modules that the build script compiles.
Protect the narrowed build-script CLI composition root with focused unit, parser-schema, property, and direct-rustc UI tests. Document the maintained UI boundary so later slice changes update its positive and negative fixtures deliberately.
Bind the direct-rustc UI fixtures to the production CLI paths and verify their declarations still match `build.rs`. Keep the runtime-module rejection meaningful by compiling the same real support graph in the positive and negative fixtures.
Restore the runtime CLI imports that the rebase lost while keeping the four-file build-script slice narrow. Track the help schema file, retain the UI boundary contract, and make command-schema coverage independent per command variant.
Remove the obsolete `build_support` façade and align module and developer documentation with the inline build-script slice. Normalize one terminal DNS dot before policy matching while preserving wildcard apex rejection.
Accept CRLF checkouts before parsing the inline `build.rs` facade while retaining the exact module declarations and count. Add a CRLF regression alongside the direct-rustc boundary contract.
Keep cached configuration merging free of observer callbacks by returning bounded events with the merge result and replaying them in `config_load`. Update callers, documentation and the spelling policy to preserve the established event order and required en-GB prose.
Assert that the static rerun directives match the narrow CLI facade and exclude runtime-only modules. Document the terminal-DNS-dot matching rule at the user-facing network-policy boundary.
Keep policy help metadata within the runtime parser while preserving the four-file build-script facade. Retain direct-schema artefact coverage and Clap-independent policy parsing.
Record bounded allowed and rejected decisions at the fetch boundary without emitting raw URLs or hosts. Keep the network test module within its size contract and correct the remaining build-slice prose.
Retain the main branch's observability example while preserving the cached-merge API contract from this branch. Keep the documentation-test identifier unique across the users' guide.
ae82581 to
60f4003
Compare
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
3 participants
Closes #513
Problem
build.rsrecompiles part of the library so it can callcli::Cli::command()for man-page generation. It declared
src/cli/mod.rs, which pulled the wholeclisubtree — merging, discovery, diagnostics, localized value parsing — pluscli_l10n,host_pattern,output_mode, andtheme. Removing the fivemodule-wide
#[expect(dead_code, ...)]attributes and building produced 110unused-item diagnostics, which is what those attributes were suppressing.
The suppressions also masked genuinely dead code. Appending an unused
pub fntosrc/cli/config.rsonmainproduced no diagnostic from anycompilation unit: the library exports
cli::configpublicly so it is notdead-code linted there, and the build script's module-wide expectation covered
it here.
Change
build.rsnow declares an inlineclifacade naming exactly the three filesthe Clap schema needs, rather than inheriting the subtree:
The library is split along the same seam so that slice is self-contained:
src/cli/command.rsCli,InteractionArgs,BuildArgs,GraphArgs,Commands)src/cli/parser.rs; the schema is all the man page needssrc/cli/preferences.rsClioutput-policy accessorstheme_preferencewas the only reason the build script compiledthemeand, transitively,output_modesrc/cli/validation.rsMAX_JOBS,validation_errorsrc/cli/config.rsstop reaching up intosrc/cli/mod.rssrc/host_matching.rsHostCandidate,HostPattern::matchessrc/host_pattern.rsthe schema does not needsrc/cli/parser.rskeeps the localization-aware parsing entry point;cli_l10n,output_mode, andthemeare no longer declared by the buildscript at all.
Result
cargo check --all-targetsemits no unused-item diagnostics.
mainnow reports: an unusedpub fninsrc/cli/config.rsproduceswarning: function ... is never usedfrom the build-script crate.before and after).
locales/orsrc/localization/touched. Thelocale_catalogues/localizationdeclarations were already correct and are untouched.
src/cli/config.rsat321,
src/host_pattern.rsdown from 344 to 304).docs/developers-guide.mdgains a section recording the slice as a maintainedboundary: widening it reintroduces unreachable items, and a dependency added
outside it surfaces as a build-script compile error.
Gates
cargo fmt -- --checkmake lint-clippy(cargo doc+ clippy)make testmake markdownlintmake nixieTwo gates fail identically on unmodified
origin/mainin this environment andare not caused by this change:
make check-fmtrunscargo fmt --all, which fails resolving thetest_supportpath dependency's workspace.cargo fmt -- --checkon theroot package passes.
make lint's Whitaker pass reportsno_std_fs_operationsagainstbuild.rsandbuild_l10n_audit.rs— 9 findings onmain, 8 after thischange, all in
std::fscalls this PR does not touch.🤖 Generated with Claude Code
Summary by Sourcery
Narrow the build script's compiled module graph while separating CLI schema concerns and modernizing bounded event reporting at application boundaries.
New Features:
Bug Fixes:
Enhancements:
Build:
CI:
Documentation:
Tests:
Chores: