Plan 215: Audit AST-walking rules and rewrite the ones that only need f.Lines - #424
Merged
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files
☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Phase one of plan 215. Adds internal/integration/rule_walk_audit_test.go, which classifies every registered rule A / B / ast-required / hybrid via two runtime probes plus a go/packages static scan, and checks the result into testdata/rule_walk_audit.json. TestRuleWalkAuditManifest doubles as the phase-three regression gate: a converted rule that regrows an f.AST read flips its static signal and the manifest comparison fails until the JSON is regenerated (MDSMITH_REGEN_WALK_AUDIT=1). The probe drives each rule against its OWN bad/ fixtures rather than one shared fixture. A shared compliant fixture leaves the nil-AST and code-block probes comparing empty-to-empty (most rules emit nothing on any given document), which proves nothing; a rule that fires on no bad fixture is recorded inconclusive-not-fired so the probe never mislabels a non-firing opt-in rule as Lines-only. Promotes golang.org/x/tools to a direct dependency (go/packages). Flips the plan status to in-progress and checks off task 1 / AC 1. https://claude.ai/code/session_01HPVF7Tzt319PyDrFMohJqw
Phase zero of plan 215. ProseRanges projects the byte ranges of File.Source that fall inside prose nodes (paragraph, heading, list-item, blockquote text), excluding fenced/indented code blocks, HTML blocks, inline code spans, autolinks, inline raw-HTML tags, and PI directive blocks. A Lines-only prose rule scans these ranges instead of re-walking the AST to rediscover the same code-skipping filter; the walk runs once per File and is shared. Derived from f.AST by recursive descent (no ast.Walk closure box, matching collectCodeBlockLines) and memoized via the atomic.Bool + mutex pattern the codebase uses for per-File caches. Visible text wrapped by inline-HTML tags stays prose (CommonMark renders it as words); only the tag bytes and the autolink/link destination are excluded. https://claude.ai/code/session_01HPVF7Tzt319PyDrFMohJqw
The landed manifest shows Category A holds no AST-walking rules: prior perf plans (175/195/196) already converted the substring rules (trailing space, hard tabs, line length, BOM) to direct f.Lines scans. The live lever is Category B — standalone-Check rules that walk the AST only to rediscover code-skipping ranges. Updates tasks 3/4 to name the real targets (MDS047, MDS054) and flags MDS050 as a hybrid (opt-in code/HTML scan plus an AST-based Fix keep it on the tree). https://claude.ai/code/session_01HPVF7Tzt319PyDrFMohJqw
Quantified MDS054's contribution to BenchmarkCheckCorpusLarge: ~11 ms wall and ~0.3% of allocs on the 600-file corpus. Combined with Category A being already-converted and the Category B prose rules being opt-in (absent from the default benchmark), no available conversion can move the gate 5%. Records this in the plan Risk section so the spec matches the evidence; ProseRanges still landed as the correct home for the code-skipping filter. https://claude.ai/code/session_01HPVF7Tzt319PyDrFMohJqw
Adds direct unit tests for appendProseRange's adjacent/overlapping/contained paths and a code-only document yielding no prose ranges, taking proserange.go to 100% and clearing the codecov/patch gate. https://claude.ai/code/session_01HPVF7Tzt319PyDrFMohJqw
Adds internal/integration/perrule_bench_test.go: a per-opt-in-rule regression suite that sits alongside TestPerRuleAllocBudget and the engine BenchmarkCheckCorpus* gates without replacing either. - optInRules enumerates opt-in rules programmatically (implements rule.Defaultable && !EnabledByDefault), never hardcoded. - BenchmarkOptInRule: one sub-benchmark per opt-in rule timing r.Check directly (isolated, no engine), reporting ns/op + allocs/op. - TestPerRuleBenchBudget: pins both a parse-subtracted allocs/op ceiling (deterministic, ~+20%) and a total parse+Check ns/op ceiling (~5x headroom). Gates total parse+Check time because the parse-subtracted Check delta is too noisy (Check << parse); parse is constant so a Check regression still trips. Each rule is its own subtest; a missing budget entry fails. - TestPerRuleBenchDocCompliant guards that the larger representative doc stays diagnostic-free under every rule, so the gate measures base scan cost not per-violation overhead. - Skipped under -short and -race, mirroring TestPerRuleAllocBudget. Covers 26 opt-in rules (MDS024/029/033-037/041-058/063/067/068). https://claude.ai/code/session_01HPVF7Tzt319PyDrFMohJqw
Adds Task 7's guidance to docs/development/high-performance-go.md: - The per-rule gate layer (perrule_bench_test.go) alongside the flat alloc ceiling and corpus benches, plus how to pin a new opt-in rule's perRuleBenchBudget row (Time ~5x baseline, Allocs ~+20%). - Why the gate times parse+Check together. - Category A (Lines-only) vs B (AST-walking via ProseRanges) vs AST-required, with a pointer to rule_walk_audit.json. Condensed adjacent prose (intro cross-refs, PGO list, Patterns-to- avoid table, Tooling) net-zero so the file stays under the MDS022 300-line budget without touching .mdsmith.yml. https://claude.ai/code/session_01HPVF7Tzt319PyDrFMohJqw
…rk done - Rewrite Tasks 3-4: the cleanly-convertible Category-A AND Category-B sets are exhaustively empty (manifest + standalone-AST finding), so no rule conversions ship. - Task 5: the substitute deliverable is the per-opt-in-rule isolated benchmark + alloc+time gate (perrule_bench_test.go). - Task 6/7: regression gate (TestRuleWalkAuditManifest + TestPerRuleBenchBudget) and perf-guide convention recorded. - Add a compact per-opt-in-rule baseline table (ns/op + allocs/op) as evidence for all 26 opt-in rules. - Rewrite ACs to mark the conversion/wall-time criteria superseded (with rationale) and check off the rest. - Condense now-historical Background/Approach prose to keep the plan under the 300-line budget. - Flip plan front-matter status and PLAN.md row to done. https://claude.ai/code/session_01HPVF7Tzt319PyDrFMohJqw
The per-rule gate measured files named check.md/b.md (absent from the in-memory FS) while TestPerRuleBenchDocCompliant verified compliance on doc.md (the FS-resident name). A filename- or FS-presence-sensitive rule (e.g. directory-structure) could diverge between the compliance guard and the measured baseline. Use doc.md everywhere and drop the now-unused name parameter. https://claude.ai/code/session_01HPVF7Tzt319PyDrFMohJqw
diagsEqual built its comparison key with string(rune(d.Line))/string(rune(d.Column)) — the string(rune(int)) antipattern: fragile (a NUL-encoding value could blur the field separators) and obscuring correctness. strconv.Itoa is unambiguous and collision-free. The key is internal-only, so classification is unchanged — TestRuleWalkAuditManifest passes with no drift. https://claude.ai/code/session_01HPVF7Tzt319PyDrFMohJqw
jeduden
marked this pull request as ready for review
May 30, 2026 07:17
jeduden
added
the
queue:active
Owner
Author
|
🟢 Merge Queue — picked up This PR is in the queue and will be batched with other Next: No action needed — you'll get another comment when CI starts on the batch. View merge queue run. |
Owner
Author
|
🔵 Merge Queue — CI running Merged into batch branch Next: No action needed — you'll be notified when CI completes. |
jeduden
removed
the
queue:active
Owner
Author
|
✅ Merge Queue — merged This PR landed on Next: Done — nothing more to do here. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Draft PR for plan/215_lines-only-rule-audit.md.
Status will move 🔲 → 🔳 in PLAN.md once the first real commit lands. Marking ready for review when the acceptance criteria are checked off.
https://claude.ai/code/session_01HPVF7Tzt319PyDrFMohJqw
Generated by Claude Code