Defame1297/holocron
0
0
Fork 0
Code Issues 14 Pull Requests Actions Packages Projects Releases 3 Wiki Activity

docs: record the finding 5 and 16 salvage decisions in the audit

Browse Source 
Why: the 2026-09-16 grill decided the two salvage options the verified
notes for findings 5 and 16 had left open.

Implementation Notes: finding 5's differential-suite speed-up is closed
as not proceeding, with the per-suite timings that decided it. Finding
16's resolver-sourcing option is recorded as done in ef27c97, with the
line delta and the output-identity check.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Defame1297
2026-09-16 13:25:29 +00:00
parent ef27c9751a
commit df64475ff6
1 changed files with 4 additions and 0 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
SIMPLIFICATION-AUDIT.md
View File

@@ -134,6 +134,8 @@ This is the area you named as hardest to understand and slowest. Root cause: mos
5. [x] **`skill-size-check.sh` has six test files totalling 3,589 lines for one 1,497-line script**, split by ADR section rather than behaviour. `test-adr0020-differential.sh` is 452 lines for 12 assertions. Merge to two files. Effort M. 5. [x] **`skill-size-check.sh` has six test files totalling 3,589 lines for one 1,497-line script**, split by ADR section rather than behaviour. `test-adr0020-differential.sh` is 452 lines for 12 assertions. Merge to two files. Effort M.
> **Not proceeding (2026-09-14):** premise doesn't hold, in the same way finding 3's did not. The six suites are **not** split by ADR section — they are split by failure class, and five of the six headers name the incident they guard. (The exception is `tests/test-skill-size-check.sh`, whose header names no incident: it describes the two gate families the script must not conflate and flags the constant-agreement block as the load-bearing part.) `test-adr0020-contract.sh` defends *structural* claims that "each one fails silently": that the resolver block copied verbatim into three scripts has not drifted, that both interpreter preflights still exist, that `verbose: true` is still set on the hook (the entire delivery mechanism for the SUGGESTION tier). It records that the `validate-provenance.sh` pair "had already drifted" once. `test-adr0020-differential.sh` compares *verdicts* between `skill-size-check.sh` and `validate.sh` on real files, and its header states that constant-agreement is "necessary but demonstrably not sufficient — a previous review found the two scripts disagreeing on real files while every constant matched perfectly", with two ceilings excluded "until a real divergence shipped behind the exclusion". The suites also do not cover the same scripts: `contract` reaches `validate-provenance.sh` (`tests/test-adr0020-contract.sh:115-116` byte-compares both copies of it). Merging by subject would delete exactly the guards against silent drift between hand-duplicated validators. Re-measured at HEAD: **3,619 lines** across six suites against a **1,517-line** script, not 3,589/1,497. That ratio is the cost of the duplication, not an independent defect — it is deleted by **finding 16**, which removes the thing being differentially compared. **#5 is downstream of #16 and should be reconsidered only after it.** The one salvageable part is a performance change, not a coverage change: `test-adr0020-differential.sh` spends 29 s of every push re-running two validators over the live corpus, and could be sped up with no coverage loss. That is a different finding than the one written here. > **Not proceeding (2026-09-14):** premise doesn't hold, in the same way finding 3's did not. The six suites are **not** split by ADR section — they are split by failure class, and five of the six headers name the incident they guard. (The exception is `tests/test-skill-size-check.sh`, whose header names no incident: it describes the two gate families the script must not conflate and flags the constant-agreement block as the load-bearing part.) `test-adr0020-contract.sh` defends *structural* claims that "each one fails silently": that the resolver block copied verbatim into three scripts has not drifted, that both interpreter preflights still exist, that `verbose: true` is still set on the hook (the entire delivery mechanism for the SUGGESTION tier). It records that the `validate-provenance.sh` pair "had already drifted" once. `test-adr0020-differential.sh` compares *verdicts* between `skill-size-check.sh` and `validate.sh` on real files, and its header states that constant-agreement is "necessary but demonstrably not sufficient — a previous review found the two scripts disagreeing on real files while every constant matched perfectly", with two ceilings excluded "until a real divergence shipped behind the exclusion". The suites also do not cover the same scripts: `contract` reaches `validate-provenance.sh` (`tests/test-adr0020-contract.sh:115-116` byte-compares both copies of it). Merging by subject would delete exactly the guards against silent drift between hand-duplicated validators. Re-measured at HEAD: **3,619 lines** across six suites against a **1,517-line** script, not 3,589/1,497. That ratio is the cost of the duplication, not an independent defect — it is deleted by **finding 16**, which removes the thing being differentially compared. **#5 is downstream of #16 and should be reconsidered only after it.** The one salvageable part is a performance change, not a coverage change: `test-adr0020-differential.sh` spends 29 s of every push re-running two validators over the live corpus, and could be sped up with no coverage loss. That is a different finding than the one written here.
>
> **Salvage closed (2026-09-16, grill): not proceeding.** Timed one suite at a time on this 4-core machine, `test-adr0020-differential.sh` takes **34.3 s** of **213 s** total suite time, behind bats (70.7 s) and ahead of `test-vale-wrap.sh` (29.4 s). Its cost is about 90 validator runs, one after another, at 0.1–0.15 s each. Even deleting it outright would take at most 34 s off a pre-push measured at 3.5–5 min, and inside `run-tests` a parallel rewrite would compete for the same four cores, so a standalone speed-up is too small to be worth another change to a regression suite. Pre-push `run-tests` wall time is a separate question, pending a possible issue.
6. [x] ~~**Prose-grep tests.** `test-governance-layer.sh` and `test-instructions-and-docs.sh` (583 lines) grep markdown for phrases, including a one-shot "issue 0015 refactor incomplete" assertion made permanent and an assertion that `docs/notes/` exists. Delete both.~~ `check-apm-agents-valid.sh` (~~161 + 264 test lines~~ → **167 + 282**, re-measured 2026-09-16 at HEAD) is a loop plus fail-closed guards around `validate.sh`; it folds into the merged audit skill's own tests (finding 14). Effort S. 6. [x] ~~**Prose-grep tests.** `test-governance-layer.sh` and `test-instructions-and-docs.sh` (583 lines) grep markdown for phrases, including a one-shot "issue 0015 refactor incomplete" assertion made permanent and an assertion that `docs/notes/` exists. Delete both.~~ `check-apm-agents-valid.sh` (~~161 + 264 test lines~~ → **167 + 282**, re-measured 2026-09-16 at HEAD) is a loop plus fail-closed guards around `validate.sh`; it folds into the merged audit skill's own tests (finding 14). Effort S.
> **Done (2026-09-12):** see commit `5f9f2b3` on `docs/simplification-audit`. Deleted `tests/test-governance-layer.sh` (270 lines) and `tests/test-instructions-and-docs.sh` (313 lines); no other file referenced either. `check-apm-agents-valid.sh` was left untouched — its fate is tied to the separate, out-of-scope skill-merge finding 14. > **Done (2026-09-12):** see commit `5f9f2b3` on `docs/simplification-audit`. Deleted `tests/test-governance-layer.sh` (270 lines) and `tests/test-instructions-and-docs.sh` (313 lines); no other file referenced either. `check-apm-agents-valid.sh` was left untouched — its fate is tied to the separate, out-of-scope skill-merge finding 14.
@@ -233,6 +235,8 @@ The shared pattern: per-skill `README.md` files no model reads, a `docs/research
> **The `vale-wrap` half is wrong on its conclusion.** `|` literal blocks do fix the folding case — the script says so and deliberately no-ops on them — but the wrapper handles **four** affected scalar forms (folded `>`, bare plain, double- and single-quoted continuation lines), and **277 of its 526 lines are argv handling unrelated to folding** (cwd-relative absolutization, the `is_builtin_output` guard, scratch-tree mirroring, path relativization), each with its own incident record. Decisively, `.pre-commit-hooks.yaml` exports these hooks to external consumer repos whose scalar style this repo cannot dictate. Converting the 40 in-repo descriptions to `|` is a fine independent change; **it does not shrink the wrapper.** > **The `vale-wrap` half is wrong on its conclusion.** `|` literal blocks do fix the folding case — the script says so and deliberately no-ops on them — but the wrapper handles **four** affected scalar forms (folded `>`, bare plain, double- and single-quoted continuation lines), and **277 of its 526 lines are argv handling unrelated to folding** (cwd-relative absolutization, the `is_builtin_output` guard, scratch-tree mirroring, path relativization), each with its own incident record. Decisively, `.pre-commit-hooks.yaml` exports these hooks to external consumer repos whose scalar style this repo cannot dictate. Converting the 40 in-repo descriptions to `|` is a fine independent change; **it does not shrink the wrapper.**
> >
> Where the savings actually are: **merge skill-audit + agent-audit (finding 14) → ~~−1,587 lines~~ → landed 2026-09-15 at −2,934 lines, zero coverage loss.** A second option — sourcing the resolver into `scripts/skill-size-check.sh` rather than embedding it (−1,061) — is technically possible but couples the root hook to plugin layout and dismantles the byte-identity contract test's design; needs a decision, not an assumption. > Where the savings actually are: **merge skill-audit + agent-audit (finding 14) → ~~−1,587 lines~~ → landed 2026-09-15 at −2,934 lines, zero coverage loss.** A second option — sourcing the resolver into `scripts/skill-size-check.sh` rather than embedding it (−1,061) — is technically possible but couples the root hook to plugin layout and dismantles the byte-identity contract test's design; needs a decision, not an assumption.
>
> **Decided and done (2026-09-16, grill; `ef27c97`).** The coupling objection went away with finding 36: `4de5b6b` retired `.pre-commit-hooks.yaml`, so `skill-size-check.sh` runs only inside this repo, where the plugin path always exists. The hook now sources `factory-audit/scripts/lib-boundary-resolver.sh` and fails closed without it. `scripts/skill-size-check.sh` went from **1,524** to **509** lines (`wc -l`, −1,015), and the change is −957 lines net across 9 files. The hook's stdout, stderr and exit code are identical before and after over every corpus `SKILL.md` and the 26 differential-suite fixtures. The contract test's byte-identity hash became single-copy assertions (27 → 29 passes), and ADR-0020 and ADR-0025 carry dated amendments.
17. [x] **Fold `forge` and `apm-install`.** `forge` is a four-row routing table plus 207 lines of references explaining fork vs inline; it should be 25 lines with no references. `apm-install` (53 lines + 17-line sources) becomes a sixth dispatch row in `apm-workflow`. Effort S. 17. [x] **Fold `forge` and `apm-install`.** `forge` is a four-row routing table plus 207 lines of references explaining fork vs inline; it should be 25 lines with no references. `apm-install` (53 lines + 17-line sources) becomes a sixth dispatch row in `apm-workflow`. Effort S.
> **Decision (2026-09-16):** Not proceeding — the human declined this finding. `forge` and `apm-install` stay as separate skills. > **Decision (2026-09-16):** Not proceeding — the human declined this finding. `forge` and `apm-install` stay as separate skills.