Redact JSON catalogue version (#569) - #611
Conversation
Reuse the generator-version snapshot filter for JSON help catalogues so release version bumps do not update an otherwise unchanged fixture.
|
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:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (4)
🔗 Linked repositories identifiedCodeRabbit considers these linked repositories for cross-repo context during reviews:
Included review availability: 4 reviews are currently available. Your included PR review attempts over the past 7 days set your current allowance at 5 reviews per hour. Summary
Resolves Validation
WalkthroughCentralise JSON snapshot version redaction. Pass snapshot settings into catalogue rendering. Reuse shared text rendering for text-based help snapshots. Keep text catalogue settings unfiltered. ChangesSnapshot settings
Suggested labels: Poem
Merge Risk: ⚪ Minimal · up to The PR changes snapshot-test redaction and documentation behavior without introducing an actionable merge-blocking risk; it is merge-ready after normal checks and review. 🚥 Pre-merge checks | ✅ 20✅ Passed checks (20 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 12 functions across 2 files. (3 skipped: 3 unsupported.) Full details: Testing (Overall)Explanation Mark Testing (Overall) as PASS. The JSON catalogue test renders the real Full details: User-Facing DocumentationExplanation Pass this check. The pull request changes test-only snapshot infrastructure and the expected JSON snapshot. Full details: Developer DocumentationExplanation PASS — The pull request documents the changed snapshot-test APIs and boundary. Full details: Module-Level DocumentationExplanation Pass the module-level documentation check. The two changed Rust modules have clear Full details: Testing (Unit And Behavioural)Explanation Pass the testing check. The PR adds a property-based invariant test for SemVer variants and preservation of unrelated version fields, and it parses the filtered result as JSON. The JSON catalogue snapshot exercises the Full details: Testing (Property / Proof)Explanation Pass. The changed code introduces a range-based invariant: redact only Netsuke's generator SemVer while preserving JSON structure and unrelated version fields. Full details: Testing (Compile-Time / Ui)Explanation Pass the testing check. The pull request adds snapshot coverage for the structured JSON help catalogue and preserves the three focused text snapshots. The JSON snapshot retains Full details: Unit ArchitectureExplanation PASS. The PR changes only Full details: Domain ArchitectureExplanation Pass the Domain Architecture check. The complete diff from the PR predecessor changes only snapshot tests/support, snapshot fixtures, a development dependency, and documentation. Full details: ObservabilityExplanation Classify this change as test-only. The pull-request range changes Full details: Security And PrivacyExplanation The pull request only changes test snapshot settings, test helpers, a development-only Full details: Performance And Resource UseExplanation Accept the change. The executable changes are limited to snapshot-test helpers and property tests. The text helper preserves the existing one render per test, and the JSON filter runs only during one snapshot bind. Property-test inputs are bounded: prerelease and build identifiers are at most 32 characters, unrelated versions are at most 64 characters, and each case uses a fixed small JSON document. The regex dependency is dev-only, and no new production loops, unbounded collections, blocking I/O, retries, or hot-path work were introduced. Full details: Concurrency And StateExplanation Classify this check as PASS. The PR adds only per-call Full details: Architectural Complexity And MaintainabilityExplanation Keep the change. The new abstractions have immediate, concrete reuse: Full details: Rust Compiler Lint IntegrityExplanation Pass this check. The pull-request diff adds no broad ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Reviewer's GuideThe PR makes JSON snapshot tests resilient to Netsuke generator version bumps by sharing the existing generator-scoped redaction filter, applying it only to diagnostic JSON and JSON help-catalogue snapshots, and updating the JSON fixture accordingly; text catalogue snapshots remain unchanged and fully versioned. 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 Please suggest a fix for this issue and supply a prompt for an AI coding agent to enable it to apply the fix. Include the file and symbol names indicated in the issue at the head of your response. Ensure that this is validated against the current version of the codegraph. If further refinement to address this finding would be deleterious, please supply a clear explanatory one to two paragraph markdown message in a code block that I can paste into the CodeScene web ui's diagnostic suppression function so this diagnostic can be silenced. Comment on lines +156 to +166 catalogue_snapshot(
"en-US",
"text_catalogue",
&snapshot_settings("help_targets"),
|manifest| {
Ok(normalize_fluent_isolates(&render_text(
&build_catalogue(manifest),
theme_prefs(ThemePreference::Unicode),
)))
},
)❌ New issue: Code Duplication |
This comment was marked as resolved.
This comment was marked as resolved.
Route each text catalogue snapshot through one themed helper while keeping the JSON catalogue redaction path separate.
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.
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e0412438ff
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| /// | ||
| /// Anchor the filter on the enclosing generator object so unrelated versioned | ||
| /// content remains visible in snapshot diffs. | ||
| fn add_generator_version_filter(settings: &mut Settings) { |
There was a problem hiding this comment.
The new reusable add_generator_version_filter abstraction is introduced without updating the contributor documentation that defines its ownership and reuse policy. docs/snapshot-testing-in-netsuke-using-insta.md still describes generator-version filtering as diagnostic-JSON-only, leaving the new help-target settings path undiscoverable; document the abstraction's scope, permitted callers, and composition rules in the relevant guide.
AGENTS.md reference: AGENTS.md:L111-L119
Useful? React with 👍 / 👎.
| /// Render and assert a text catalogue snapshot with a selected theme. | ||
| fn text_catalogue_snapshot_with_theme( |
There was a problem hiding this comment.
This helper refactors three existing text snapshot tests but is bundled with the functional JSON-version redaction fix. That prevents the behavioural fix and unrelated deduplication from being reviewed, reverted, or bisected independently; move this refactor to a separate follow-up commit after the fix.
AGENTS.md reference: AGENTS.md:L120-L128
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Exercise the shared filter across generated SemVer variants while preserving unrelated version fields and valid JSON output.
Define the generator-version filter ownership and the separate JSON and text snapshot-settings contracts for contributors.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
|
@coderabbitai Review reconciliation for the current head:
|
|
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 head and named commits ---'
git rev-parse --short HEAD
git log --oneline --decorate -30
for commit in e0412438 0046af7c 05773f33 3598d2c6; do
printf '\ncommit %s\n' "$commit"
git show -s --format='%H%n%P%n%s' "$commit"
done
printf '%s\n' '--- relevant Rust declarations ---'
ast-grep outline src/runner/help_tests.rs --items all --type function --view expanded
ast-grep outline src/snapshot_test_support.rs --items all --view expanded
printf '%s\n' '--- catalogue helpers and test entry points ---'
rg -n -A38 -B6 \
'fn (catalogue_snapshot|text_catalogue_snapshot_with_theme|text_catalogue_snapshot|accessible_catalogue_snapshot|localized_catalogue_snapshot|json_catalogue_snapshot)\s*\(' \
src/runner/help_tests.rs
printf '%s\n' '--- JSON redaction constants, settings, and property tests ---'
rg -n -A32 -B8 \
'(VERSION|version|filter|help_targets_json_snapshot_settings|diagnostic_json_snapshot_settings|proptest|prop_compose)' \
src/snapshot_test_support.rs src \
-g '*.rs'
printf '%s\n' '--- documentation references ---'
rg -n -i -A10 -B4 \
'(snapshot|redact|redaction|generator.version|help.targets|json builder|unfiltered text)' \
README.md docs .github \
-g '*.md' -g '*.mdx' -g '*.rst' 2>/dev/null || true
printf '%s\n' '--- fixture version placeholders ---'
rg -n -A5 -B5 '"version"[[:space:]]*:[[:space:]]*"(\\[version\\]|[0-9])' \
. -g '*.snap' -g '*.json' -g '*.txt' 2>/dev/null || trueLength of output: 2132 🏁 Script executed: #!/bin/bash
set -u
printf '%s\n' '--- PR `#611` commits from GitHub ---'
gh pr view 611 --repo leynos/netsuke --json commits \
--jq '.commits[] | [.oid[0:8], .messageHeadline] | `@tsv`' || true
printf '%s\n' '--- help test function outline ---'
ast-grep outline src/runner/help_tests.rs --items all --type function --view expanded || true
printf '%s\n' '--- text and JSON catalogue test implementations ---'
rg -n -A42 -B6 \
'fn (catalogue_snapshot|text_catalogue_snapshot_with_theme|text_catalogue_snapshot|accessible_catalogue_snapshot|localized_catalogue_snapshot|json_catalogue_snapshot)\s*\(' \
src/runner/help_tests.rs || true
printf '%s\n' '--- snapshot redaction settings and constants ---'
ast-grep outline src/snapshot_test_support.rs --items all --view expanded || true
rg -n -A36 -B8 \
'(VERSION|version|filter|help_targets_json_snapshot_settings|diagnostic_json_snapshot_settings|proptest|prop_compose)' \
src/snapshot_test_support.rs -g '*.rs' || true
printf '%s\n' '--- all snapshot-related property-test definitions ---'
rg -n -A36 -B8 \
'(proptest!|#[[:space:]]*test|SemVer|Version|version)' \
src -g '*.rs' || true
printf '%s\n' '--- tracked documentation files that reference redaction or snapshots ---'
git ls-files '*.md' '*.mdx' '*.rst' | while IFS= read -r file; do
rg -n -i -A10 -B4 \
'(snapshot|redact|redaction|generator version|help targets|JSON builder|unfiltered text)' \
"$file" && printf '%s\n' "--- file: $file ---"
done || true
printf '%s\n' '--- tracked snapshot and JSON fixture version values ---'
git ls-files '*.snap' '*.json' '*.txt' | while IFS= read -r file; do
rg -n -A5 -B5 '"version"[[:space:]]*:[[:space:]]*"(\\[version\\]|[0-9])' \
"$file" && printf '%s\n' "--- file: $file ---"
done || trueLength of output: 137788 🏁 Script executed: #!/bin/bash
set -u
printf '%s\n' '--- PR `#611` commit order ---'
gh api "repos/leynos/netsuke/pulls/611/commits?per_page=100" \
--jq '.[] | [.sha[0:8], .commit.message | split("\n")[0]] | `@tsv`' || true
printf '%s\n' '--- help test symbol locations ---'
rg -n \
'^(fn (catalogue_snapshot|text_catalogue_snapshot_with_theme|text_catalogue_snapshot|accessible_catalogue_snapshot|localized_catalogue_snapshot|json_catalogue_snapshot)|use .*snapshot_test_support)' \
src/runner/help_tests.rs || true
printf '%s\n' '--- bounded help test implementation ---'
sed -n '100,240p' src/runner/help_tests.rs || true
printf '%s\n' '--- snapshot support symbol locations ---'
rg -n \
'^(const |fn |#\[cfg\(test\)\]|mod tests|.*(GENERATOR|generator|VERSION|version|filter|help_targets_json_snapshot_settings|diagnostic_json_snapshot_settings))' \
src/snapshot_test_support.rs || true
printf '%s\n' '--- bounded snapshot support implementation and tests ---'
sed -n '1,280p' src/snapshot_test_support.rs || true
printf '%s\n' '--- candidate help snapshot paths ---'
git ls-files | rg -i '(help.*target|target.*help|catalogue).*\.(snap|json|txt)$' || true
printf '%s\n' '--- exact version fields in candidate help snapshots ---'
git ls-files | rg -i '(help.*target|target.*help|catalogue).*\.(snap|json|txt)$' \
| while IFS= read -r file; do
printf '\n--- %s ---\n' "$file"
rg -n -A3 -B3 '"version"' "$file" || true
doneLength of output: 15842
Mark the reconciliation complete. Verify the commit order in PR
Verify the current implementation:
Treat the dynamic validation results as maintainer-provided evidence. This environment performed static inspection only. You are interacting with an AI system. |
Mark the SemVer term as code so the test-module documentation satisfies Clippy's Markdown convention.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
|
@coderabbitai review |
✅ Action performedReview finished.
|
2 participants
Summary
Closes #569.
This branch redacts Netsuke's generator version in the JSON help catalogue
snapshot, so release-version bumps do not update an otherwise unchanged
fixture. It retains the complete JSON assertion, gives the text catalogue
refactor its own follow-up commit, property-tests the redaction range, and
records the contributor contract for JSON and text snapshot settings.
Review walkthrough
[version]while retaining every other assertion.Validation
make check-fmt: passedmake lint: passedmake doc-coverage: passed at 98.98%make test: 2,410 non-doctests and all doctests passedmake markdownlint: passedmake nixie: passedBacklinks
References
Summary by Sourcery
Stabilize JSON catalogue snapshots by redacting release-dependent generator versions while retaining complete behavioural assertions.
Enhancements:
Documentation:
Tests: