Skip to content

feat(optimization): adopt official methods across knowledge surfaces#78

Merged
drewstone merged 6 commits into
mainfrom
fix/explicit-retrieval-method
Jul 24, 2026
Merged

feat(optimization): adopt official methods across knowledge surfaces#78
drewstone merged 6 commits into
mainfrom
fix/explicit-retrieval-method

Conversation

@drewstone

Copy link
Copy Markdown
Contributor

What changed

  • removes the local bounded retrieval search and requires a complete OptimizationMethod
  • runs retrieval, RAG, KB policy, and memory candidates through Agent Eval with separate train, selection, and final cases
  • adds immutable execution and memory candidate identities for correct resume behavior
  • blocks unsafe RAG and memory activation on regression, incomplete cost, or unobserved external package identity
  • verifies packed installs against official GEPA and Microsoft SkillOpt at pinned source revisions
  • simplifies the public docs and removes obsolete API claims

Checks

  • AGENT_KNOWLEDGE_NETWORK_TESTS=1 pnpm test: 452 passed, 12 skipped
  • pnpm lint
  • pnpm typecheck
  • pnpm build
  • pnpm verify:package
  • AGENT_EVAL_TEST_PYTHON=... pnpm verify:official-optimizers: 2 passed

tangletools
tangletools previously approved these changes Jul 24, 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 — b0f33334

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.

tangletools · auto-approval · reason: drewstone_author · 2026-07-24T16:49:31Z

@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
Concerns 1 (1 weak-concern)
Heuristic 0.0s
Duplication 0.0s
Interrogation 153.9s (2 bridge agents)
Total 153.9s

💰 Value — sound

Removes a maintained-in-parallel grid-search optimizer and routes all knowledge optimization through official agent-eval methods with immutable identity refs and fail-closed activation gates — coherent and in the codebase's grain.

  • What it does: Deletes the local bounded-retrieval grid search (boundedRetrievalConfigMethod, buildBoundedRetrievalConfigs, ~300 lines) so every optimization surface (runRetrievalImprovementLoop, runRagOptimization, optimizeKnowledgeBasePolicy, runAgentMemoryImprovement) requires a complete OptimizationMethod and flows through the existing runSerializedKnowledgeOptimization adapter (src/optimization.ts). Adds as
  • Goals it achieves: (1) Single source of truth for candidate search/selection: stop maintaining a parallel optimizer and its resume/cost/concurrency bugs; let agent-eval own it. (2) Make optimization results non-silently-reusable across incompatible implementations (immutable refs + persisted candidate identity) so a resumed run cannot trust stale evidence. (3) Make promoting a winner into production memory/KB safe-b
  • Assessment: Good change, built in the grain of the codebase. The deletion is the core value: boundedRetrievalConfigMethod duplicated what the dependency owns (campaign batching, cost-ledger wiring, completeness checks, concurrency) and carried its own edge cases; removing it shrinks the maintenance surface and removes a second optimization path. All four surfaces already shared the runSerializedKnowledgeOptim
  • Better / existing approach: Searched src and the installed @tangle-network/agent-interface@0.32.0 dist: the shared package already exports sha256DigestSchema and gitObjectSchema (agent-candidate-schema-common.d.ts:3-4), and src/agent-candidate.ts:41 already consumes sha256DigestSchema. assertImmutableRef re-derives both patterns as local regex instead of composing the shared zod schemas into one assertion guard. This is cosm
  • Model: opencode/zai-coding-plan/glm-5.2
  • Bridge attempts: 2
  • Bridge warning: opencode/kimi-for-coding/k2p7: bridge stream ended without value-audit content

🎯 Usefulness — sound

A clean major-version consolidation that deletes a local retrieval sweep in favor of official agent-eval optimizers, and hardens resume/activation safety with immutable identity refs threaded through every optimization surface.

  • Integration: Fully wired and reachable. The four public entry points (runRetrievalImprovementLoop, runRagOptimization, optimizeKnowledgeBasePolicy, runSerializedKnowledgeOptimization at src/index.ts:45/39/34; runAgentMemoryImprovement via /memory) all route through runSerializedKnowledgeOptimization (src/optimization.ts:75), which now requires and validates executionRef. runRagKnowledgeImprovementLoop consumes
  • Fit with existing patterns: Fits the codebase grain precisely. AGENTS.md layering states agent-knowledge imports agent-eval and agent-interface downward; this PR moves JsonValue to AgentCandidateJsonValue from @tangle-network/agent-interface (src/rag-optimization.ts:6, src/retrieval-eval.ts:7, src/optimization.ts:13) and pins agent-eval to an exact 0.126.2. The deleted boundedRetrievalConfigMethod/buildBoundedRetrievalConfig
  • Real-world viability: Hardened for realistic use. assertImmutableRef (src/immutable-ref.ts:4) enforces sha256:<64 hex> or git:<40 hex> on every identity-carrying option: executionRef (src/optimization.ts:83), policyApplicationRef (src/kb-improvement/optimization.ts:108), implementationRef/activation.ref/executeStepRef (src/memory/improvement/validation.ts:18,31,40), and candidate.ref (src/memory/improvement/candidate.t
  • Model: opencode/zai-coding-plan/glm-5.2
  • Bridge attempts: 1

💰 Value Audit

🟡 assertImmutableRef re-derives shared digest schemas [duplication] ``

src/immutable-ref.ts:1-8 hardcodes /^sha256:[a-f0-9]{64}$/ and /^git:[a-f0-9]{40}$/ locally, while @tangle-network/agent-interface (already a dependency, already used at src/agent-candidate.ts:41 via sha256DigestSchema) exports both sha256DigestSchema and gitObjectSchema (confirmed in agent-interface@0.32.0 dist/agent-candidate-schema-common.d.ts:3-4). If gitObjectSchema's accepted format ever widens (e.g. abbreviated/object hashes, case rules), the local copy won't track it. Consider composing


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 · 20260724T170522Z

@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 — b033f182

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.

tangletools · auto-approval · reason: drewstone_author · 2026-07-24T17:07:53Z

@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
Concerns 0 (none)
Heuristic 0.0s
Duplication 0.0s
Interrogation 199.1s (2 bridge agents)
Total 199.1s

💰 Value — sound

Consolidates all four optimization surfaces (retrieval, RAG, KB policy, memory) on official agent-eval methods with immutable content-addressed identity and safety gates; removes ~200 lines of duplicated grid-search code — a clean, well-verified major version.

  • What it does: This is a breaking major version (4.1.0→5.0.0) that: (1) removes the built-in boundedRetrievalConfigMethod (a hand-rolled exhaustive retrieval grid search with ~200 lines of helpers: buildBoundedRetrievalConfigs, measureBoundedRetrievalSurface, setConfigPath, campaignIsComplete, etc.) and requires callers to pass a complete OptimizationMethod from agent-eval for all optimization surfaces; (2) adds
  • Goals it achieves: (1) Eliminate the duplicated local grid-search code by routing all optimization through the single official agent-eval OptimizationMethod funnel — the bounded method was a parallel reimplementation of candidate search that agent-eval already owns. (2) Make resume correctness verifiable: immutable content-addressed identity means if implementation, config, or external dependencies change, the ident
  • Assessment: Sound and well-structured. The single biggest win is removing ~200 lines of hand-rolled grid-search logic (boundedRetrievalConfigMethod and its helpers) that duplicated agent-eval's OptimizationMethod abstraction — the old code ran its own runCampaign loops, cost ledgers, concurrency batching, and completion checks in parallel to the official path. Requiring a complete method aligns with AGENTS.md
  • Better / existing approach: none — this is the right approach. Searched for existing equivalents: the codebase already has sha256DigestSchema from @tangle-network/agent-interface (used in src/agent-candidate.ts:3 and src/kb-improvement/activation.ts:286) but that only validates sha256 digests, not git commit hashes — assertImmutableRef correctly covers both formats for execution identity, so it is not a duplicate. The reject
  • Model: opencode/zai-coding-plan/glm-5.2
  • Bridge attempts: 2
  • Bridge warning: opencode/kimi-for-coding/k2p7: bridge stream ended without value-audit content

🎯 Usefulness — sound

Consolidates all knowledge optimization surfaces onto one complete OptimizationMethod primitive, removes the local bounded-retrieval sweep, and verifies the packed package against pinned official GEPA/SkillOpt — coherent, reachable, and in the grain of the codebase.

  • Integration: runSerializedKnowledgeOptimization (src/optimization.ts:75) is the hub, publicly exported at root (src/index.ts:34) and required by verify-package.mjs:26. Four public wrappers consume it: runRetrievalImprovementLoop (src/retrieval-optimization.ts:85), runRagOptimization (src/rag-optimization.ts:75), optimizeKnowledgeBasePolicy (src/kb-improvement/optimization.ts:116), and runAgentMemoryImprovement
  • Fit with existing patterns: Aligns with AGENTS.md:78 ('Use a complete OptimizationMethod from @tangle-network/agent-eval'). Removes the competing local sweep (boundedRetrievalConfigMethod/buildBoundedRetrievalConfigs/buildRetrievalParameterCandidates/retrievalParameterSweepProposer — confirmed absent from src/tests/docs/skills, and verify-package.mjs:29 asserts their absence). The immutable executionRef/policyApplicationRef/
  • Real-world viability: rejectUnsafePromotionEvidence (src/rag-improvement-loop.ts:391) blocks promotion on missing evidence, incomplete cost accounting, unobserved package identity, negative lift-CI lower bound, or cost-ceiling exceedance before the caller hook runs. Memory activation (src/memory/improvement/activation.ts:65) uses compare-and-set plus a re-read of observed config, with an append-only journal that detect
  • Model: opencode/zai-coding-plan/glm-5.2
  • Bridge attempts: 1

No concerns — sound change, no better or existing approach found. ✅


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 · 20260724T172629Z

@tangletools

Copy link
Copy Markdown
Contributor

✅ No Blockers — b033f182

Review health 100/100 · Reviewer score 48/100 · Confidence 95/100 · 11 findings (2 medium, 9 low)

glm: Correctness 48 · Security 48 · Testing 48 · Architecture 48

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 50 changed files. Global verifier still owns final merge decision.

🟠 MEDIUM Strict key validation on StoredMemoryArtifact and ActivationEvent hard-fails old-format records on resume — src/memory/improvement/evaluation.ts

hasExactKeys(record, ['surfaceHash','candidateRef','sequenceFingerprint','sequenceId','rep','seed','artifact']) rejects any record with an unexpected key. Old artifacts written before this PR had schema: 1 instead of candidateRef, so hasExactKeys returns false and the function THROWS 'does not match its evaluation cell' — it does not return undefined for re-evaluation. The same pattern appears in activation.ts:195-207 (expectedKeys exact match rejects old schema: 2 activation events). Any run directory with old-format artifacts or activation journals becomes unresumable after upgrade with no migration path or helpful error. Impact: users with in-flight improvement runs must delete the run directory and restart, losing cached evaluation results and paying to re-run them. This is a bre

🟠 MEDIUM dispatchCandidate race window breaks pending-map dedup, risking duplicate paid evaluations — src/memory/improvement/run.ts

The old code was fully synchronous from pending.get(key) to pending.set(key, operation) — no race was possible. The new code inserts const builtCandidate = await candidateFor(candidate, candidateSurfaceHash) between the if (!operation) check and pending.set(key, operation). If two concurrent dispatches for the same artifact key (same surfaceHash+scenario+rep+seed) enter before candidateFor resolves, both pass the !operation guard, both await the same memoized candidateFor promise, and when it resolves both create separate evaluateMemoryCandidate operations — each calling runPaidCall, each writing to the same artifact path. This can cause double-charging on paid LLM calls. The candidateFor Map itself is safe (it sets the promise synchronously before awaiting), but the pending Map is

🟡 LOW Removing schema version field leaves no explicit format discriminator for future migrations — src/benchmarks/memory-recovery.ts

The schema: 3 field served as an explicit event-format version. Its removal is safe TODAY because the schema:1->3 bump (commit c3cc7b4) co-introduced the cost fields, so the field-level validation at lines 402-410 (adapterCreationCostUsd/costUsdPerCase/recoveryCostUsdPerAttempt required as finite non-negative numbers) still rejects any legacy schema:1 events on disk. However, if a future change alters the event shape by adding/removing/renaming fields, there is no version sentinel to gate on - migration must rely entirely on field-presence checks, which are fragile against type-widening or field-rename changes. Acceptable given the current compreh

🟡 LOW candidateExecutionRef binding invariant is undocumented — src/kb-improvement/evaluation.ts

candidateExecutionRef hashes executionRef+candidateHash to derive a per-candidate execution identity. The non-obvious invariant — why both inputs are bound (so two candidates sharing the same base executionRef get distinct storage/cache keys, preventing cross-candidate result contamination in agent-eval) — is not stated. Per repo convention (comments explain non-obvious invariants/risk boundaries), a one-line comment would help future readers avoid weakening this to a constant or dropping the candidateHash term. Not blocking; behavior is correct and tested.

🟡 LOW Inconsistent schema-field migration across event types — src/memory/experiment/recovery.ts

The schema field was removed from RecoveryAttemptEvent (attempt-log.ts:8) and AgentMemoryAttemptEvent (experiment/types.ts:200), and their parsers simply dropped the schema check without adding strict key validation — old events with schema: 1 or schema: 2 are silently accepted (extra key ignored). But StoredMemoryArtifact (evaluation.ts:293) and AgentMemoryActivationEvent (activation.ts:195) had strict hasExactKeys/expectedKeys validation ADDED, which rejects old records containing the schema field. This means within the same run directory, some old-format records are accepted and others throw. The migration strategy should be consistent: either all parsers reject extra keys, or all tolerate them.

🟡 LOW Search-phase cost ceiling removed from optimizationRunOptions, removing protected final-comparison budget — src/memory/improvement/run.ts

The old code set costCeiling: options.maxOptimizationCostUsd ?? 0 inside optimizationRunOptions (bounding the method's search-phase spend) and costCeiling: options.maxFinalCostUsd ?? 0 at the comparison level (bounding the final comparison). The new code sets only costCeiling: options.maxTotalCostUsd ?? 0 at the comparison level, with no per-phase ceiling in optimizationRunOptions. This means the search phase can now consume the entire budget, leaving nothing for the final comparison. decidePromotion catches this via totalCost.totalCostUsd > maxTotalCostUsd and blocks promotion, so it fails safe — but the guarantee that the final comparison always has reserved budget is gone. This is an intentional design change (consolidated to maxTotalCostUsd), but callers relying on the old two-

🟡 LOW Import-swap has no regression guard; broken import reached this PR unguarded — src/rag-eval/contracts.ts

The base branch compiled contracts.ts against an export (JsonValue from @tangle-network/agent-eval/campaign) that does not exist in the pinned agent-eval@0.126.2 declaration — I reproduced error TS2305 by checking out the base file and running tsc --noEmit. This fix is correct, but a typecheck on install/resolution (or a minimal compile-only test touching the re-exported types) would have caught the broken import at the dependency bump instead of here. Consider adding contracts.ts (or the package entry) to a typecheck smoke step that runs against the locked dep versions. No action required for this PR; the fix itself is right.

🟡 LOW Removed schema: 2 from fixtures relies on parser's extra-key tolerance — tests/memory/experiment-recovery.test.ts

Six manually-written JSONL attempt events in this file had schema: 2 removed to match the new AgentMemoryAttemptEvent shape (src/memory/experiment/types.ts:201, recovery.ts:380,431). parseMemoryAttemptEvent (recovery.ts:422-453) does NOT do exact-key validation, so both old (with schema) and new (without schema) fixtures would parse — meaning the edits are stylistically consistent but not strictly required, and they do not test that legacy on-disk journals (from deployed prior versions that still emit schema:2) are accepted. The migration safety claim rests entirely on parser tolerance that no test in this file pins. If a future change tightens parseMemoryAttemptEvent to reject unknown keys, every deployed in-flight attempt log becomes unreadable. Consider adding one fixture variant that

🟡 LOW Unsafe casts bypass RunAgentMemoryImprovementOptions shape — tests/memory/graphiti.test.ts

const options = { createCandidate: ... } as RunAgentMemoryImprovementOptions<typeof config> constructs a minimal object and casts away every other required field (experimentId, baselineConfig, method, trainSequences, selectionSequences, finalSequences, implementationRef, runDir, controllerMode, significance). buildCandidate only reads options.createCandidate at runtime today, so the test passes; but the cast means any future field accessed by buildCandidate would silently be undefined at runtime instead of failing tsc. Same pattern at tests/memory/mem0.test.ts:304 and tests/memory/improvement.test.ts:286 (the negative test there intentionally omits required cost fields via cast). Prefer constructing a real minimal options via a shared helper or factoring buildCandidate's input to a narro

🟡 LOW Resume +2 candidate-construction assertion documents an implicit createCandidate side-effect contract — tests/memory/improvement.test.ts

Assertion changed from toBe(constructionsAfterFirstRun) to toBe(constructionsAfterFirstRun + 2). This is correct for the new code: buildRegisteredCandidate re-invokes options.createCandidate on resume for baseline+winner surface hashes to detect identity drift (verified against candidate.ts:36-65 and run.ts:105-113, 181-184). But the assertion encodes a new implicit contract — createCandidate MUST be idempotent and side-effect-free on resume, because resume now always calls it at least twice even when all artifacts are cached on disk. A createCandidate that provisions a fresh provider account per call would double-charge or double-provision on every resume. The companion test 'rejects a changed candidate identity before reusing a resumed result' ([line 136](https://github.com/tangle-ne

🟡 LOW Missing-method test omits executionRef, relying on validation order — tests/retrieval-eval.test.ts

The options object at line 233-245 omits both method and executionRef. The test only asserts the method-completeness error message fires. This passes today because src/optimization.ts:82 runs assertCompleteOptimizationMethod before src/optimization.ts:83 runs assertImmutableRef. If that ordering is ever swapped, the test would still pass (it would throw a different substring match). Not a bug, but the test does not lock the intended 'method-first' ordering invariant — the dedicated 'requires an immutable execution identity' test at [line 252](https://github.com/tangle-network/agent-knowledge/blob/b033f182041b3b2d4f199110182a781df2f3edb9/tests/retriev


tangletools · 2026-07-24T17:52:24Z · trace

tangletools
tangletools previously approved these changes Jul 24, 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.

✅ Approved — 11 non-blocking findings — b033f182

Full multi-shot audit completed 8/8 planned shots over 50 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-07-24T17:52:24Z · 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 — fda8e254

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.

tangletools · auto-approval · reason: drewstone_author · 2026-07-24T18:27:47Z

@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
Concerns 1 (1 weak-concern)
Heuristic 0.0s
Duplication 0.0s
Interrogation 218.6s (2 bridge agents)
Total 218.6s

💰 Value — sound

Major-version refactor that deletes the local retrieval/search campaign code and routes all four optimization surfaces (retrieval, RAG, KB policy, memory) through agent-eval's compareOptimizationMethods, adding an immutable execution-identity layer that composes into agent-eval's dispatchRef plus fa

  • What it does: Removes ~300 lines of locally-reimplemented campaign machinery (runCampaign/fsCampaignStorage/createRunCostLedger/campaignMeanComposite/costFromLedgerSummary) from src/retrieval-optimization.ts and src/rag-optimization.ts; both now delegate to runSerializedKnowledgeOptimization -> compareOptimizationMethods (agent-eval). Deletes the local boundedRetrievalConfigMethod/buildBoundedRetrievalConfigs s
  • Goals it achieves: Statistical integrity of optimization runs: strict train/selection/final partitioning with content-fingerprint overlap detection (src/optimization.ts:305, src/memory/improvement/validation.ts:108), final cases touched exactly once, and no resuming a run whose callbacks/models/indexes changed. Fail-loud safety before activation: no promoting on regressing, unaccounted, over-budget, or unobserved-id
  • Assessment: This is a strong change on its merits. The central move - deleting runCampaign/fsCampaignStorage/createRunCostLedger/campaignMeanComposite/costFromLedgerSummary and routing through compareOptimizationMethods - is the correct de-duplication: agent-eval already owns candidate search, resume, and final scoring, and the knowledge package had been parallel-reimplementing it. The identity layer is corre
  • Better / existing approach: Searched for existing equivalents before answering. agent-eval already exposes dispatchRef (the resume-identity primitive) and provenance/accountingComplete/ComparisonCost; this change composes into those rather than replacing them, so no existing capability is being reinvented. The only adjacent duplication is agent-eval's internal const SHA256 = /^sha256:[a-f0-9]{64}$/ in src/campaign/surface-
  • Model: opencode/zai-coding-plan/glm-5.2
  • Bridge attempts: 2
  • Bridge warning: opencode/kimi-for-coding/k2p7: bridge stream ended without value-audit content

🎯 Usefulness — sound

Consolidates four knowledge-optimization surfaces onto one serialized runner backed by official agent-eval methods, removing a local bounded-retrieval reinvention and adding durable immutable-identity and final-case isolation boundaries — all proven end-to-end via packed-install verification.

  • Integration: Fully wired and reachable. Five public surfaces (runSerializedKnowledgeOptimization at src/optimization.ts:75, runRagOptimization at src/rag-optimization.ts:62, runRetrievalImprovementLoop at src/retrieval-optimization.ts:64, optimizeKnowledgeBasePolicy at src/kb-improvement/optimization.ts:88, runAgentMemoryImprovement at src/memory/improvement/run.ts:47) all funnel through one core runner requir
  • Fit with existing patterns: Exactly in the grain. AGENTS.md already documents this as the integration boundary: 'Use a complete OptimizationMethod from @tangle-network/agent-eval with runRetrievalImprovementLoop(), runRagOptimization(), optimizeKnowledgeBasePolicy(), or runAgentMemoryImprovement().' Package layering is respected (knowledge imports eval, never vice versa). The serialized-candidate + codec pattern is consisten
  • Real-world viability: Holds up well past the happy path. assertImmutableRef (src/immutable-ref.ts:6) enforces sha256/git hex identity, preventing resume-state corruption across implementation changes — validated at every entry point (validation.ts:18, evaluation.ts:49, optimization.ts:83). The final-evaluation boundary (finalEvaluationStartedAt ledger event + one-way gate in evaluation.ts:140-160) prevents final-case r
  • Model: opencode/zai-coding-plan/glm-5.2
  • Bridge attempts: 1

💰 Value Audit

🟡 exceedsCostCeiling duplicated verbatim across two modules [duplication] ``

function exceedsCostCeiling(totalCostUsd, costCeiling) with the identical epsilon-tolerance body exists in both src/rag-answer-evidence.ts:76 and src/rag-improvement-loop.ts (same epsilonmax8 formula). The non-negative-finite cost-ceiling validation (assertOptionalCostCeiling) also repeats the Number.isFinite(value) || value < 0 pattern in src/memory/improvement/validation.ts:59 and src/memory/improvement/candidate.ts. These are small (3 lines each) and in the same RAG-evidence domain, so


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 · 20260724T183144Z

@drewstone
drewstone merged commit 6195131 into main Jul 24, 2026
2 checks passed
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