feat: fledge spec lint β quality gate for the spec itself (#429) - #505
feat: fledge spec lint β quality gate for the spec itself (#429)#5050xGaspar wants to merge 1 commit into
Conversation
`fledge spec check` validates code against the spec. Nothing validated the
spec. A thin, aspirational, or poisoned spec passes structurally while being
wrong β and the spec is the one input to an agent pipeline that nothing else
gates.
`fledge spec lint [target]` closes that in two layers, per the design
constraint in the issue's follow-up comment ("the quality judgment should be
agent-graded, not a static linter"):
Layer 1 β structural, always runs, offline and deterministic:
- every required section present *and non-empty* (comments, empty table
rows, and bare bullets are scaffolding, not content)
- no TODO / TBD / FIXME placeholder in Purpose or Public API (prose only β
a backticked token is a citation, so a spec documenting the check does
not fail it)
- `version:` is an integer (spec-sync convention) or a semver string
- every `files:` entry exists on disk
- at least one acceptance signal and one rejection signal (non-empty
Behavioral Examples / Error Cases, or `accepts:` / `rejects:` frontmatter)
Layer 2 β model-graded, opt-in via `--ai`, same provider plumbing as
`fledge review`: falsifiable purpose, load-bearing vs decorative invariants,
discriminating acceptance/rejection signals, internal consistency. It is
opt-in so the default invocation stays safe as a pre-commit hook and CI gate;
when requested it fails fast with a clear error rather than hanging, and a
provider or parse failure becomes a `model_pass_failed` error β a gate that
could not run never reads as a pass.
`--ignore <check>` is the human override (validated against a closed check
vocabulary). `--strict` promotes warnings to errors. `--json` emits
`{schema_version: 1, action: "spec_lint", strict, model_pass, specs, totals,
passed}`; errors go to stderr as plain text; exit code is the contract.
Reuses the existing spec parser (new `split_frontmatter` /
`extract_section_bodies` helpers) rather than adding a second one, and hoists
the seven-section default into a shared `DEFAULT_REQUIRED_SECTIONS`.
Specs: `spec` v14, `main` v14. SDD change CHG-0007 (draft β definition
approval is a human gate).
Closes #429
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KuhwrJF2XFDVHhX7qzDuyy
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
0xLeif
left a comment
There was a problem hiding this comment.
Review β REQUEST CHANGES
Feature design: strong
This is a well-shaped answer to #429:
- Two layers with the right default: layer 1 always offline; layer 2 opt-in via
--ai, with--no-aiwinning so a future default flip is non-breaking - Fail-closed model pass (
model_pass_failederror, never silent skip) - Fail-fast provider probe before the first prompt (keyed providers by key presence; keyless Ollama gets one 3s
/api/tagsprobe) - Closed check vocabulary +
--ignorevalidation against it - Placeholder check on prose only (fenced/inline code stripped) so the lint module's own spec can cite
TODO - Integer or semver
version:is the correct call given fledge's monotonicu32frontmatter --jsonviaenvelope::action/action: "spec_lint"matches the action dialect- Tests use
StubLlmProviderfor layer 2 β no network LLM in CI - Specs bumped (
spec/mainβ v14), AGENTS.md row added, introspect snapshot updated
The maintainer call-outs in the PR body (lane wiring, layer-2 default, empty-section vs spec new) are the right open decisions; leaving them un-wired is fine for v1 of the command.
Blocker: SpecSync contract / active change coverage
spec-check and trust fail. Relevant excerpts:
error: meaningful changed paths are not covered by an active change:
src/snapshots/fledge__introspect__tests__introspect_schema.snap,
src/spec/lint.rs, src/spec/mod.rs, src/spec/parse.rs,
src/spec/tests.rs, tests/spec.rs
error: CHG-0001-β¦: accepted change verification is stale β¦ AGENTS.md changed after acceptance
error: CHG-0003-β¦: β¦ specs/main/main.spec.md changed after acceptance β¦
CHG-0007 is present but still state: draft. Draft does not count as an active change for path coverage, so the new lint surface is uncovered for the trust gate. Tasks.md already lists the human gate:
- approve this change definition (
specsync change approve)
Please:
- Get
CHG-0007approved / activated so it covers the listed paths (and extendaffected_pathsif anything is still missing after approve), - Reopen (or successor) the staled accepted changes that locked
AGENTS.mdandspecs/main/main.spec.md, - Confirm
spec-check+ trust are green on the PR.
Non-blocking nits (not required for re-approval)
- Placeholder match is case-sensitive (
prose.contains("TODO")).todo/Todoin Purpose would pass. Probably intentional for noise control; if you want a stricter gate later, word-boundary + case-insensitive would be better. prose.contains("TODO")also hitsTODOS/ similar substrings β rare in Purpose/Public API; fine for v1.- Site docs deferred β acceptable as called out in CHG docs.md; consider a follow-up issue so it is not lost.
- Agree with leaving
spec lintout ofpre-commit/cilanes until maintainers decide.
Verdict
I would approve the implementation once the SpecSync lifecycle gate is green (activate CHG-0007 + reopen stales). No code redesign requested β this is primarily a process/CI gate hold.
There was a problem hiding this comment.
β Corvin says...
_
<(;\ .oO(oh no...)
|/(\
\(\\
" "\\
"Caw... validation failed..."
CI Summary
| Check | Status |
|---|---|
| Dependency Audit | β Passed |
| Integration (3 OS) | β Passed |
| Lint (fmt + clippy) | β Passed |
| Spec Validation | β failure |
| Tests (3 OS) | β Passed |
Powered by corvid-pet
Implements #429 β a quality gate for the spec itself, not just code-vs-spec.
Design
Two layers, per the maintainer's follow-up on the issue ("agent-graded, not a static linter", with cheap structural checks kept as a fast pre-pass):
Layer 1 β structural, always runs, fully offline. Check ids:
frontmatter,version_format,missing_section,empty_section,placeholder_text,missing_file,no_acceptance_signal,no_rejection_signal.Layer 2 β model-graded,
--aionly. Reuses the existing provider plumbing. Ids:purpose_not_falsifiable,decorative_invariants,weak_acceptance_signal,weak_rejection_signal,underspecified_error_cases,api_drift,aspirational_claim,quality_other, plusmodel_pass_failed.CLI
--jsonuses the action dialect viaenvelope::action,action: "spec_lint",schema_version: 1. Exit 0 clean / 1 on any error (or any warning under--strict); errors to stderr as plain text even under--json.No provider configured: layer 2 is unreachable without
--ai. When requested,ensure_provider_availableruns before the first prompt β a keyed provider without a key errors naming the env var; a keyless Ollama gets one 3sGET /api/tagsprobe, so CI fails fast rather than hanging.Verification
cargo test(999 unit + all integration),cargo clippy --all-targets -- -D warnings,cargo fmt --checkall green. 24 new unit tests + 6 CLI tests; no test touches a network LLM (layer 2 uses the existingStubLlmProvider).fledge spec lintover this repo: 33 specs, 0 findings.Specs updated:
specv13βv14,mainv13βv14, plus an AGENTS.md--jsontable row.Decisions worth your call
--ai), not opt-out β the biggest judgment call. The issue wants this usable as a pre-commit hook and CI gate; defaulting on means network + spend, and fledge's keyless-local-Ollama default would block on a connect timeout in CI.--no-aiwins over--aiso the default can flip later without breaking callers.version:accepts an integer or semver. The issue says "valid semver", but spec-sync frontmatter uses a monotonicu32β rejecting integers would fail all 33 fledge specs. Lint rejects what is neither (v3,1.2,draft).spec new-scaffolded module fails lint (there's a test asserting this). Intentional, but people will notice.`TODO`is a citation. Without this thespecmodule's own spec failed its own check.spec lintis NOT wired intofledge.toml'spre-commit/cilanes. All 33 specs pass so it would be safe, but adding a gate is a maintainer decision.Note on the SDD record
This branch adds
CHG-0007indraftβ definition approval is an explicit human gate. Be aware that allocating it bumps.specsync/change-sequence.json, which under the current SpecSync behaviour cascades staleness onto previously accepted changes (CorvidLabs/spec-sync#481).fledge spec checkis already red onmainfor that same reason.π€ Generated with Claude Code
https://claude.ai/code/session_01KuhwrJF2XFDVHhX7qzDuyy