Skip to content

refactor(surface): delete twelve modules stranded by the barrel tiering - #643

Merged
drewstone merged 2 commits into
mainfrom
refactor/simplify-20260818
Aug 18, 2026
Merged

refactor(surface): delete twelve modules stranded by the barrel tiering#643
drewstone merged 2 commits into
mainfrom
refactor/simplify-20260818

Conversation

@drewstone

@drewstone drewstone commented Aug 18, 2026

Copy link
Copy Markdown
Contributor

What

0.145.3 tiered the root barrel to consumer-imported symbols and documented front doors. Twelve modules lost their last export path in that pass and kept their source files. This deletes them with their tests, and cuts 0.148.0.

reviewer, analyst/knowledge-capture, multi-toolchain-layer, dual-agent-bench, workspace-inspector, golden-matcher, ui-finding, slo, adapters/langchain, judge-runner, cost-report, worker-driver-seed.

This completes a pass this repository already ran: #581's sibling commit deleted 30 zero-importer modules outright. These twelve have zero importers and were left behind.

The public export surface does not move

A build of the parent commit (0.148.0, 99ed9b8) and a build of this branch each declare 3,447 export entries, and the two lists are identical.

base 0.148.0 (99ed9b8) export entries: 3447
branch 0.149.0 export entries:         3447
=== DIFF ===
IDENTICAL — public export surface unchanged

dist loses 753 bytes across the same 107 files — all of it doc-comment text that named a deleted symbol. The modules themselves were never in dist.

Evidence for the deletion

Each module is unreachable from all 28 build entry points, so none has shipped in dist since 0.145.3 — three published versions. Verified directly against published tarballs: every listed symbol is present in dist for 0.135.1, 0.140.1 and 0.143.0, and absent for 0.145.3 and 0.147.0.

The only importer each module retained was its own test.

Canonical checkouts. 106 under ~/code with .git as a directory, 98 after deduplicating by origin remote. Each fetched, then grepped at refs/remotes/origin/HEAD on word boundaries. 84 clean, 12 hits, 1 self, 0 unresolved failures. One repository initially failed to fetch (persona-labs-sdk); it has no origin remote and no package.json, and its sibling fetched clean.

Eleven of the twelve hits define the name locally or import it from their own module — blueprint-agent has its own ./shot-reviewer/context and ./toolchain-adapters, agent-runtime defines its own UI_LENSES, supervisor-lab its own matchGoldens.

Published dependents. 11 tarballs at current latest, extracted and grepped, 0 extract failures. One hit.

The one real finding

starter-foundry imports runAssertions, WorkspaceAssertion, WorkspaceSnapshot and fileExists from this package and pins it to exactly 0.135.1 (in devDependencies). At that pin those symbols still exist, so it builds today and this PR does not change that.

The constraint it already carries is narrower: it cannot move past 0.145.3 without porting them, and that predates this PR by four releases. Recorded in the CHANGELOG migration note.

(Corrected during review — an earlier revision of this description said starter-foundry declared no dependency at all. That check read only dependencies and peerDependencies and missed the devDependency.)

Release

Cuts 0.149.0. (An earlier revision of this branch cut 0.148.0; #641 took that number while this PR was in review, so the branch was rebased onto it and the release renumbered. The deletion commit cherry-picked clean — no code changed in the rebase.)

Proof

typecheck   tsc --noEmit — clean
lint        biome check src — Checked 723 files, no fixes applied
test        374 passed | 2 skipped (376 files)
            5278 passed | 3 skipped (5281 tests)
build       Build complete; OpenAPI 3.1 spec written
verify:pkg  EXIT 0 — publint "All good!", attw, exports verified

Version locked across package.json, clients/python/pyproject.toml, clients/python/src/agent_eval_rpc/__init__.py, clients/python/uv.lock. The analyst dependency-lock digest is repinned (all four manifests it covers moved); the implementation digest does not move, because the deletion touched no analyst benchmark file.

@tangletools tangletools left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Auto-approved drewstone PR — 7a96f33e

This PR was opened by the trusted drewstone account.
The full PR reviewer audit still runs separately and will publish findings if it detects issues.

This approval is provisional. It rests on the audit running. If the audit cannot run — for example the CLI bridge rejects it — this approval is dismissed rather than left standing, so an unrun check never reads as a passing one.

tangletools · auto-approval · reason: drewstone_author · 2026-08-18T20:06:39Z

@drewstone

Copy link
Copy Markdown
Contributor Author

@tangletools review now

tangletools
tangletools previously approved these changes Aug 18, 2026

@tangletools tangletools left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Auto-approved drewstone PR — 7a96f33e

This PR was opened by the trusted drewstone account.
The full PR reviewer audit still runs separately and will publish findings if it detects issues.

This approval is provisional. It rests on the audit running. If the audit cannot run — for example the CLI bridge rejects it — this approval is dismissed rather than left standing, so an unrun check never reads as a passing one.

tangletools · auto-approval · reason: drewstone_author · 2026-08-18T20:06:58Z

0.145.3 tiered the root barrel to consumer-imported symbols and
documented front doors. Twelve modules lost their last export path in
that pass and kept their source files. Each one is unreachable from all
28 build entry points, so none has shipped in dist for three published
versions, and the only importer each retains is its own test.

Deleted with their tests: reviewer, analyst/knowledge-capture,
multi-toolchain-layer, dual-agent-bench, workspace-inspector,
golden-matcher, ui-finding, slo, adapters/langchain, judge-runner,
cost-report, worker-driver-seed.

The public export surface does not move: a build of the parent commit
and a build of this commit both declare 3,447 export entries, and the
two lists are identical. dist loses 753 bytes, all of it doc-comment
text that named a deleted symbol.

Four doc comments named a deleted symbol and now describe the surviving
behaviour instead: multi-layer-verifier (twice), fuzz/types,
contract/self-improve, plus the docs/feature-guide feature map.
fuzz/explorer-cost asserts the ledger total directly rather than
through the deleted cost-report projection.

Evidence for the deletion: 106 canonical checkouts under ~/code, 98
after deduplicating by origin remote, each fetched and grepped at
refs/remotes/origin/HEAD on word boundaries. 84 clean, 12 hits, 1 self,
0 unresolved failures. Eleven of the twelve hits define the name
locally or import it from their own module. The twelfth, starter-foundry,
imports runAssertions, WorkspaceAssertion and WorkspaceSnapshot from
this package while declaring no dependency on it, and has been
incompatible since 0.145.3 removed them from dist. Eleven published
dependent tarballs at current latest were extracted and grepped with 0
extract failures; starter-foundry is the only hit.
tangletools
tangletools previously approved these changes Aug 18, 2026

@tangletools tangletools left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Auto-approved drewstone PR — 7fd1dfec

This PR was opened by the trusted drewstone account.
The full PR reviewer audit still runs separately and will publish findings if it detects issues.

This approval is provisional. It rests on the audit running. If the audit cannot run — for example the CLI bridge rejects it — this approval is dismissed rather than left standing, so an unrun check never reads as a passing one.

tangletools · auto-approval · reason: drewstone_author · 2026-08-18T20:20:46Z

Version locked across package.json, clients/python/pyproject.toml,
clients/python/src/agent_eval_rpc/__init__.py and clients/python/uv.lock.

The analyst dependency-lock digest is repinned because all four manifests
it covers moved. The implementation digest does not move: the deletion
touched no file in the analyst benchmark manifest.
@drewstone

Copy link
Copy Markdown
Contributor Author

@tangletools review now

@tangletools

Copy link
Copy Markdown
Contributor

✅ No Blockers — 7a96f33e

Review health 100/100 · Reviewer score 83/100 · Confidence 95/100 · 4 findings (4 low)

glm: Correctness 83 · Security 83 · Testing 83 · Architecture 83

Reviewer score is advisory once the run is complete and the verdict has no blockers.

Full multi-shot audit completed 8/8 planned shots over 11 changed files. Global verifier still owns final merge decision.

🟡 LOW Quantitative export-surface claims not reproducible from the diff alone — CHANGELOG.md

The entry asserts '3,447 export entries' and 'identical lists' and '28 build entry points'. These are strong consumer-facing guarantees produced by verify-package-exports; if CI runs that script on the release commit the claim is machine-checked, otherwise it rests on the author's local run. Low impact for a changelog, but consider citing the verification command in the entry or its PR body so a future auditor can reproduce it.

🟡 LOW Fallback version is hand-synced with pyproject with no automated check — clients/python/src/agent_eval_rpc/__init__.py

The except PackageNotFoundError branch hardcodes "0.148.0" as a string duplicate of pyproject.toml's version. Every release must edit both files by hand; a miss produces a misleading version in dev/uninstalled contexts. It is in sync in this diff, so no action required for this PR. Optional hardening: a one-line test asserting version == tomllib.load(pyproject)['project']['version'], or derive the fallback from the pyproject at build time.

🟡 LOW Doc says "wrap llmJudge" but llmJudge() returns JudgeConfig directly — src/contract/self-improve.ts

llmJudge(name, prompt, opts) returns JudgeConfig<TArtifact, TScenario> (src/llm-judge.ts:97-99), the exact type of the judge field, so callers can pass it directly: judge: llmJudge('rubric', PROMPT, { chat }). The word "wrap" implies an extra adapter layer that is not needed. Impact: cosmetic precision only in published doc text; no behavior change. Fix: "Bring your own, or use llmJudge." Optional nit — not blocking.

🟡 LOW Doc advertises summary() integration that no test asserts — src/fuzz/types.ts

The rewritten doc says 'CostLedger.summary() counts fuzz spend alongside judge/analyst spend', but the test suite only asserts ledger.list() entries (explorer-cost.test.ts:88-97). The claim is mechanically true (summary() rolls up the same settled records), yet one line asserting ledger.summary().totalCostUsd === 1 or byChannel containing 'agent' would pin the advertised integration. Nit; fix optional.


tangletools · 2026-08-18T20:36:23Z · trace

@tangletools tangletools left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Approved — 4 non-blocking findings — 7a96f33e

Full multi-shot audit completed 8/8 planned shots over 11 changed files. Global verifier still owns final merge decision.

Full immutable report for this review: trace

Summary comment for this run: full summary


tangletools · 2026-08-18T20:36:23Z · immutable trace

@tangletools tangletools left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Auto-approved drewstone PR — 347d9345

This PR was opened by the trusted drewstone account.
The full PR reviewer audit still runs separately and will publish findings if it detects issues.

This approval is provisional. It rests on the audit running. If the audit cannot run — for example the CLI bridge rejects it — this approval is dismissed rather than left standing, so an unrun check never reads as a passing one.

tangletools · auto-approval · reason: drewstone_author · 2026-08-18T20:42:39Z

@tangletools

Copy link
Copy Markdown
Contributor

✅ No Blockers — 347d9345

Review health 100/100 · Reviewer score 70/100 · Confidence 95/100 · 6 findings (1 medium, 5 low)

glm: Correctness 70 · Security 70 · Testing 70 · Architecture 70

Reviewer score is advisory once the run is complete and the verdict has no blockers.

Full multi-shot audit completed 8/8 planned shots over 11 changed files. Global verifier still owns final merge decision.

🟠 MEDIUM Changelog omits the rotated published dependency-lock digest and overstates 'all doc-comment text' — CHANGELOG.md

The '### Changed' section says the only non-deletion changes are four doc comments plus the feature-guide map, and that 'dist loses 753 bytes, all of it that doc-comment text'. The PR also changes src/analyst/benchmark-implementation.ts:12-13 — ANALYST_BENCHMARK_DEPENDENCY_LOCK_SHA256 moves from 5c7e5561... to 3e17b679... — and that constant is re-exported through the public './analyst' subpath (src/analyst/index.ts), so it ships in dist and is part of the published surface. The rotation is mechanically forced (the hashed manifests package.json, clients/python/pyproject.toml, clients/python/uv.lock all carry the 0.149.0 bump, and scripts/check-analyst-benchmark-implementation.mjs gates the build on it), so the change itself is correct — but a consumer or auditor diffing 0.148.0 vs 0.149.0

🟡 LOW 'Only importer each retained was its own test' is inaccurate for cost-report — CHANGELOG.md

The Removed section states each deleted module's 'only importer each retained was its own test'. cost-report also retained src/fuzz/explorer-cost.test.ts as an importer (the diff removes import { costReport } from '../cost-report' there), which is not cost-report's own test, and that test's assertion was rewritten (perModel shape check replaced by a sum check). Tests are not build entry points, so the dist/export-surface claim is unaffected, but the sentence is factually wrong as written. Fix: 'the only importers each retained were tests' or name both test files.

🟡 LOW Minor bump where patch would suffice is conservative but defensible — package.json

CHANGELOG states the public export surface does not move (identical 3,447 export entries vs 0.148.0) and no deleted symbol has been importable since 0.145.3, so strictly a patch bump (0.148.1) would also be accurate. The chosen minor bump is not wrong — 0.x minor signals possible breaking and this release deletes source — and erring high is safe for consumers. No action required; noted only for release-hygiene consistency.

🟡 LOW Doc says 'wrap llmJudge' but llmJudge returns JudgeConfig directly — src/contract/self-improve.ts

The new comment reads 'Bring your own, or wrap llmJudge'. Evidence: src/llm-judge.ts:97-100 declares llmJudge(name, prompt, opts): JudgeConfig<TArtifact, TScenario>, which is exactly the type of the judge field (src/campaign/types.ts:116), so callers can pass llmJudge(...) directly with no wrapper. Impact: purely cosmetic — the wording suggests an adapter step that the API does not require and may send users to write an unnecessary indirection layer. Fix: 'Bring your own, or use llmJudge.' Not blocking.

🟡 LOW Docstring advertises CostLedger.summary() but the test never asserts it — src/fuzz/explorer-cost.test.ts

The removed assertion exercised costReport(ledger).perModel — the aggregation API the old docstring pointed to. The replacement sums raw entries (entries.reduce(...costUsd)===1), while the updated ExploreOptions.ledger docstring (src/fuzz/types.ts:309) now tells users the payoff is 'CostLedger.summary() counts fuzz spend'. That advertised path is untested here. Fix: add expect(ledger.summary().totalCostUsd).toBe(1) (and optionally byChannel contains {channel:'agent',costUsd:1}) next to the existing reduce to restore doc/test parity. Coverage-only; no behavior risk.

🟡 LOW Finding.detail doc example cites a payload shape with no remaining producer — src/multi-layer-verifier.ts

The rewritten comment still gives '{ adapter: 'pnpm' }' as the toolchain example for Finding.detail, but the only module that attached that shape (multiToolchainLayer, deleted in this PR) is gone; git grep at head shows no remaining producer of that detail shape. Impact: none at runtime — the field is intentionally free-form and the sentence is generic guidance — but the concrete example now describes dead usage. Fix: either drop the parenthetical example or replace it with a shape some current layer actually attaches.


tangletools · 2026-08-18T21:22:05Z · trace

@tangletools tangletools left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Approved — 6 non-blocking findings — 347d9345

Full multi-shot audit completed 8/8 planned shots over 11 changed files. Global verifier still owns final merge decision.

Full immutable report for this review: trace

Summary comment for this run: full summary


tangletools · 2026-08-18T21:22:05Z · immutable trace

@drewstone
drewstone merged commit 64ee404 into main Aug 18, 2026
2 checks passed

@tangletools tangletools left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Value Audit — sound

Verdict sound
Coverage 1 of 2 lenses (usefulness)
Concerns 1 (1 weak-concern)
Heuristic 0.0s
Duplication 0.0s
Interrogation 903.6s (2 bridge agents)
Total 903.6s

⚠️ Partial audit — the verdict covers only usefulness. value: agent returned no usable verdict. Treat the missing lens as unexamined, not as clear.

💰 Value — error

value agent produced no parseable value-audit JSON.

  • Model: opencode/zai-coding-plan/glm-5.2
  • Bridge attempts: 2
  • Bridge error: opencode/kimi-for-coding/k2p7: opencode: opencode error

🎯 Usefulness — sound

A verified-clean deletion of twelve zero-importer modules that completes a pass this repo already ran, with the export surface provably unmoved and every collateral edit (doc fixups, test rewrite, lock digest) consistent with the deletion and version bump.

  • Integration: Deletion-only change; nothing new to integrate, and I verified reachability of the removal itself. All twelve modules have zero remaining importers: grep for module paths and exported symbols across src/, tests/, clients/ returns clean (the 30 word-hits for 'reviewer' are generic prose, none are imports). None of the deleted files appear in the 28 build entries (tsdown config), the root barrel (sr
  • Fit with existing patterns: Fits the codebase's established grain exactly: the PR body cites #581's sibling commit deleting 30 zero-importer modules in the same barrel-tiering pass, and this finishes the set. The four surviving-file edits follow that same pattern — they strip doc-comment references to deleted symbols (src/contract/self-improve.ts:144, src/fuzz/types.ts:309, src/multi-layer-verifier.ts) rather than leaving da
  • Real-world viability: Verified against the real artifacts: pnpm typecheck passes; pnpm build completes (needed RAYON_NUM_THREADS=1/GOMAXPROCS=1 only because this audit sandbox caps OS threads — an environment limit, not a code issue); the PR-rewritten src/fuzz/explorer-cost.test.ts passes; the two failures in benchmark-implementation.test.ts are the same sandbox thread cap hitting their tsgo subprocesses, unrelated to
  • Model: opencode/zai-coding-plan/glm-5.2
  • Bridge attempts: 1

🎯 Usefulness Audit

🟡 explorer-cost test rewrite asserts strictly less than the deleted costReport assertion [robustness] ``

src/fuzz/explorer-cost.test.ts:94-98 replaced expect(costReport(ledger).perModel).toEqual([{ model: 'gpt-4o', usd: 1, entries: 2, unpriced: false }]) with an inline sum(entry.costUsd) === 1. The old form also pinned per-model rollup, entry count, and the unpriced flag; the new one only checks the total. Coverage nit only — CostLedger itself retains its own tests — and it does not gate shipping. Also note the PR-body claim 'the only importer each module retained was its own test' is slightly


What this audit checks

It judges the change on its merits — not whether it was tasked out in an issue. Unticketed, fast-moving work is fine; the question is whether the change is good and whether a better or existing approach should be used instead.

Pass What it asks
Heuristic Vague title? Whitespace-only or cruft-bearing diff? (content signals only)
Duplication Do added function/class names already exist elsewhere in the repo?
Value Audit What does it do? What goal does it achieve? Is it good? Better architecture or already-exists?
Usefulness Audit Does it integrate and fit? Will it hold up in real use and actually get used?

Findings are concerns, not blocks — the human reviewer decides what to do with them.

value-audit · 20260818T212823Z

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants