[mache-ec1a06] feat(smell_rules): Severity+Tags schema + CLI gate flags + 3 drift_doc_* placeholder rules (ADR-0018 PR 1-3 bundle)#403
Merged
Conversation
…s []string to SmellRule
ADR-0018 PR 1: schema-only change. Adds two fields to the SmellRule
struct per the prior-art research (mache-ec1a06):
- Severity Severity ("off" | "warn" | "error") — ESLint vocabulary,
three tiers, zero value defaults to warn via Effective() so the
existing observability contract is preserved without per-rule
backfill. Rules opt INTO blocking by declaring Severity=error;
observability mode unchanged for all existing rules.
- Tags []string — free-form classification for CLI selection via
--tags=foo,bar (added in PR 2). Capped at 3-5 per rule (soft test
warning at 5, hard fail at 8) to avoid the clippy 'restriction'
group explosion. Stages (pre-commit, ci, nightly) emerge as
(--tags × --fail-on) profiles at invocation, NOT as schema fields.
ADR-0018 amended in-place (status: Proposed → Accepted) with the
schema-vocabulary pivot from the original draft documented in the
header. The rest of the ADR (PR roadmap, scope, deferred federation,
SDD framing) stands as-is.
Tests (cmd/smell_rules_test.go, new file):
- TestSmellRule_Severity_Effective — pins zero-value → warn (the
load-bearing default that preserves the observability contract)
- TestSmellRule_AllRegisteredRulesHaveDefaultableSeverity — every
built-in rule's Effective() resolves to a canonical tier
- TestSmellRule_Tags_NoExplosion — taxonomy guard (soft 5, hard 8)
The MCP/CLI rule listing payload (rulesListing in serve_find_smells.go)
now emits 'severity' and 'tags' so agents can discover them. Severity
always present (defaults to 'warn'); tags omitted when empty.
No CLI flags or new rules in this PR — those are PR 2 + PR 3 on the
same branch.
…ting visibility (ADR-0018 PR 3)
Adds three v1 placeholder rules to smellRegistry:
- drift_doc_dead_symbol_reference (Severity: warn, Tags: docs,drift)
- drift_doc_broken_internal_link (Severity: warn, Tags: docs,drift,links)
- drift_doc_outdated_count (Severity: warn, Tags: docs,drift,counts)
Each rule ships the full metadata — ID, Languages, Severity, Tags,
Requires, ScopeColumn, Description — but the Query is a no-op
(WHERE 1=0) returning zero findings. This unblocks three downstream
PRs without overreaching PR 3's scope:
1. mache find_smells (no rule) now lists all three — discovery
surface is complete.
2. PR 2's --rule 'drift_doc_*' glob has registry entries to match.
3. PR 4's pre-commit hook can wire the rule pack today.
Each rule's actual firing logic has an honest dependency on plumbing
that doesn't belong in PR 3:
- dead_symbol_reference needs a backtick-token extractor (either a
derived _doc_tokens view or a SQLite regex extension).
- broken_internal_link needs host-side filesystem stat — pure SQL
can't validate paths; requires a runSmellRule post-filter hook.
- outdated_count needs a .mache/drift-counts.toml loader + regex
extractor + per-claim SQL execution loop.
Follow-up beads filed for each (mache-fa12b3, mache-fa569a, mache-faacf3)
referencing parent mache-e1b6c8 (ADR-0018) and mache-ec1a06 (schema).
Tests (cmd/serve_find_smells_test.go): for each rule, three tests pin
the v1 placeholder contract — _Registered (metadata in registry),
_ListingExposesIt (severity + tags reach the JSON listing surface),
and _PlaceholderQueryReturnsZeroFindings (pre-flight passes + SQL
executes cleanly + zero findings). Nine tests total; all pass.
…ags (ADR-0018 PR 2)
Extends `find-smells` CLI with the gating + selection surface ADR-0018
PR 2 specifies, without touching any rule logic (PR 1 owns the schema;
PR 3 owns new rules).
Flags:
--fail-on=<none|warn|error> default `error`. `none` preserves the
legacy observability contract; `warn` is the clippy -D warnings
analogue; `error` only escalates on rules with Severity=error
(zero today, by design — opt-in per-rule).
--rule=GLOB filepath.Match; runs every matching
rule (was: first-match exact). Discovery mode unchanged.
--tags=tag1,tag2 union filter against rule.Tags; composes
with --rule glob.
--format=ci gh/ripgrep/vale-shaped line per finding.
Refactor: cliExit -> runFindSmells(cmd, args) (int, error). RunE wraps
and dispatches to a swappable exitFunc package var; tests override it
to capture exit codes without crashing the binary. Legacy cliExit
removed (no internal callers, no exported users).
Exit codes:
0 success; 1 gate fired; 2 pre-flight; 3 unknown rule (msg now names
the glob); 4 db error. Unknown rule error includes the requested
--rule glob and --tags filter so debug is one read.
Tests (cmd/find_smells_cli_test.go, all pass):
- TestFindSmellsCLI_FailOn_None_NeverFails
- TestFindSmellsCLI_FailOn_Warn_FailsOnWarnRule
- TestFindSmellsCLI_FailOn_Error_OnlyFailsOnErrorRule (warn passes / error fails)
- TestFindSmellsCLI_Rule_GlobMatchesMultiple
- TestFindSmellsCLI_Rule_GlobMatchesNone_Exit3
- TestFindSmellsCLI_Tags_FiltersToMatchingRules
- TestFindSmellsCLI_Format_CI_OutputShape
Existing CLI tests + full cmd suite remain green.
find_smells (advisory)Scoped to files changed in this PR. Rules below run on the standalone (no-LLO) cross-ref tables; fan_out_skew — 2 finding(s) in changed files
Rules: see |
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
1 participant
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.
Summary
Implements ADR-0018 PR 1–3 in a single bundle. Three commits, one bead, three logical changes — bundled because they're tightly coupled (PR 2 + PR 3 depend on PR 1's schema, and dogfooding the gate requires all three).
What ships
PR 1 (commit
6c0c780) —SmellRuleschemaSeverity Severityfield (off | warn | errorper ESLint precedent)Tags []stringfield (free-form, capped 3-5 per rule)Effective()method: zero value →warn(preserves existing observability contract; all 11 pre-existing rules keep current behavior)rulesListing) now emitsseverity+tagsso MCP/CLI consumers see themPR 2 (commit
f297db1) — CLI gate flags--fail-on=<none|warn|error>(defaulterror) — exit-code gate, decoupled from rule severity (pylint precedent)--rulesupportsfilepath.Matchglobs (e.g.'drift_doc_*'); runs every match--format=ci—file:line:col: severity: msg [rule-id]lines (gh / ripgrep / vale convention)--tags=tag1,tag2— union filter againstSmellRule.Tags; composes with--rulecliExit→runFindSmells(cmd, args) (int, error)+exitFunchook so tests capture exit codes without crashing the test binaryPR 3 (commit
b5f4ddc) — placeholder doc-drift rulesdrift_doc_dead_symbol_reference— markdown backtick-fenced tokens vsnode_defsdrift_doc_broken_internal_link— markdown internal links vs filesystemdrift_doc_outdated_count— markdown numeric claims vs SQL ground-truth queriesAll three ship as registered metadata with
WHERE 1=0placeholder queries so they appear inmache find_smells --listand PR 2's--rule 'drift_doc_*'glob has something to match. Honest-degradation: each rule has a real plumbing dependency that exceeded PR 3 scope; tracking as follow-up beads:mache-fa12b3— backtick-token extraction (preprocessor or SQL regex)mache-fa569a— host-side filesystem link validationmache-faacf3—.mache/drift-counts.tomlparser + per-claim SQL executionWhy this scope
ADR-0018 (PR #402) framed doc-drift as spec-driven development applied to maintenance — existing prose docs ARE the spec; rules turn claims into executable verifications. This PR ships the substrate (schema + CLI gate) and visible placeholders. Subsequent PRs (via the follow-up beads) implement the actual firing logic.
The schema decisions (Severity vocabulary, Tags vs Stages) follow prior-art research (
mache-ec1a06) across ruff/pylint/eslint/clippy/semgrep. ADR-0018 was amended in-place (PR 1 commit) with the vocabulary pivots from the original draft.Tests
cmd/smell_rules_test.go(Severity/Tags schema)cmd/find_smells_cli_test.go(CLI flags + gate)cmd/serve_find_smells_test.go(3 per drift rule: registered, listing-exposed, placeholder returns 0)go test -count=1 -short ./cmd/ ./internal/graph/exits 0 (≈100s)Subagent provenance
This PR was assembled via the
/evolvepattern:/tmp/prior-art-rule-classification.md(informs schema)agent-ab59be19e61f5a7dc) → commit6a2ec51agent-aeb262f10019c5933) → commitb5f4ddcfeat/drift-rules-implementation; verify; push.Beads
Closes:
mache-ec1a06(schema decision)Progress on:
mache-e1b6c8(ADR-0018 rollout — placeholder rules ship; follow-ups remain viamache-fa12b3,mache-fa569a,mache-faacf3)Pairs with:
mache-96341d(categories — Severity addresses value-axis),mache-966e22(rule pack distribution)Test plan
go test -short ./cmd/...passes on the branch