refactor: honest single-walk + dedupe presets + version pragma#21
Merged
Conversation
Follow-up PR that honestly delivers what the earlier roadmap PRs oversold. Everything is behind broadened parity goldens so any drift is a build failure. ## 1. True single-walk rule engine (fixes spec 005 dishonesty) The prior "rule engine" PR (#17) shipped an architectural wrapper but kept the seven per-rule filepath.Walk calls untouched — every rule still walked the tree via the old Validate* method it was wrapping. Net cost: 8 walks per run instead of the previous 7. This commit does the actual conversion: - Each rule iterates ctx.Tree.Entries in walk order. - dirStructureRule tracks a skipTracker to preserve filepath.SkipDir semantics from the legacy walk (disallowed subtree suppresses descendant checks). - requiredPathsRule and requiredGroupsRule keep their os.Stat / glob helpers — the legacy quirk of seeing through ignored directories is a parity requirement. - placementRule and boundariesRule preserve their Successes counter quirks (once per (file, rule) pair even on violations). - The exported Validate* methods stay as thin wrappers over runSingleRule(path, rule) so root validator_test.go and any external callers keep working. Broadened parity fixtures cover boundaries, requiredGroups, and disallowed-subtree SkipDir semantics — three cases the earlier goldens didn't touch. 21 parity subtests, all byte-identical. ## 2. Init templates now live in one place (fixes template dup) Deletes the 220-line projectTemplates map in internal/cli/init.go. `init --type <T>` now reads the same embedded preset files that `extends: <T>-standard` resolves against, via config.ReadPreset, and prepends a one-line project-typed header comment. Two ways to reach the same rules; one source of truth. - init.go: 322 → 168 lines. - test/init_presets_test.go proves init output body is byte-identical to the corresponding preset AND that init --type go and extends: go-standard produce equivalent validation on the same tree. ## 3. Version pragma actually enforced (fixes docs-only mitigation) The prior mitigation for "old binary + new config with `extends`" was just a suggestion to add `# requires structlint >= vX.Y` in comments. Nothing enforced it — users saw the raw yaml `field not found` error. New internal/config/versioncheck.go parses the pragma from the raw bytes BEFORE strict parsing and prints an actionable message when the running binary is older than required. Dev builds (Version=="dev") and malformed pragmas skip the check. Tests: 10 in-package unit tests over parse/compare edge cases, plus 2 binary-based end-to-end tests that build pinned binaries with ldflags-injected versions and prove the error surfaces cleanly. ## 4. Deeper SKILL.md validation skill_contract_test.go had shallow checks (file exists, frontmatter delimiter present). skill_deep_test.go adds: - Frontmatter parses as real YAML with the expected shape. - Trigger phrases from the description survive block-scalar folding (parses YAML first, then normalizes whitespace before searching). - Every subcommand referenced in setup recipes actually boots via app.Run(--help) — catches renames. - JSON v1 contract mentioned + all four exit codes documented. ## Numbers 167 tests before this PR, 191 tests after. Every existing parity golden byte-identical. Self-validation still passes.
StructLint — All checks passed139 rules validated against
|
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
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
Follow-up PR that honestly delivers what the roadmap PRs oversold. Everything gated by broadened parity goldens — 21 subtests across 6 fixtures, all byte-identical.
What actually changed
1. True single-walk rule engine (the fix for spec 005)
The prior "rule engine" (#17) shipped an architectural wrapper but kept the 7 per-rule `filepath.Walk` calls — every rule wrapper called the old `Validate*` method which walked the tree itself. Net cost: 8 walks per run instead of the previous 7. The commit title lied.
This does the actual conversion:
Broadened parity fixtures: `boundaries` (Go import parsing, module resolution), `required-groups` (`eachDirMatching` + `oneOf`), `disallowed-subtree` (SkipDir semantics with nested subtrees). Three cases the earlier goldens didn't touch at all.
2. Init templates deduped
Deletes the 220-line `projectTemplates` map in `internal/cli/init.go`. `init --type ` now reads the same embedded preset files that `extends: -standard` resolves against, via a new `config.ReadPreset` API, and prepends a one-line project-typed header.
3. Version pragma is actually enforced
The prior "mitigation" for old-binary/new-config was docs suggesting `# requires structlint >= vX.Y`. Nothing checked it. Users saw the raw yaml `field not found` error.
New `internal/config/versioncheck.go` parses the pragma from raw bytes before strict parsing and prints:
```
.structlint.yaml requires structlint >= v0.6.0, but running version is v0.5.0.
Upgrade the binary (go install github.com/AxeForging/structlint/cmd/structlint@latest) …
```
Because the check runs before strict parsing, it works on binaries too old to know about `extends`. Dev builds (unstamped) skip the check.
4. Deeper SKILL.md validation
`skill_contract_test.go` had shallow checks. `skill_deep_test.go` adds:
Numbers
Test plan