feat(improvement): remeasure selected candidate changes before promotion - #149
Conversation
tangletools
left a comment
There was a problem hiding this comment.
✅ Auto-approved drewstone PR — 2aaf8485
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-17T02:46:28Z
tangletools
left a comment
There was a problem hiding this comment.
✅ Auto-approved drewstone PR — 4d11513c
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-17T03:13:22Z
|
| State | Detail |
|---|---|
| Interrupted | webhook restarted |
No review verdict was produced for this run. Trigger a fresh review on the current PR head if the PR is still open.
tangletools · #149 · model: kimi-for-coding · updated 2026-08-17T03:24:35Z
tangletools
left a comment
There was a problem hiding this comment.
✅ Auto-approved drewstone PR — 4d11513c
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-17T03:27:31Z
tangletools
left a comment
There was a problem hiding this comment.
🟡 Value Audit — sound-with-nits
| Verdict | sound-with-nits |
| Coverage | 2 of 2 lenses (value, usefulness) |
| Concerns | 2 (2 weak-concern) |
| Heuristic | 0.0s |
| Duplication | 0.0s |
| Interrogation | 370.9s (2 bridge agents) |
| Total | 370.9s |
💰 Value — sound-with-nits
Adds improveSelectedKnowledgeCandidate, which remeasures a caller-selected subset of an already-measured candidate through the ordinary improve→promote path so no unmeasured hybrid can ever be promoted; sound and in-grain, with two small duplicate-helper nits.
- What it does: Adds a new exported helper (src/kb-improvement/selected-candidate.ts:143) that takes a previously measured KnowledgeImprovementCandidateRef plus a list of changed file paths, and produces a NEW measured candidate containing only those changes. It resolves the frozen baseline + candidate snapshot via withKnowledgeImprovementComparison (workspace.ts:50), verifies the source plan hash still equals th
- Goals it achieves: Eliminates the correctness hole where a whole candidate passes its evaluators but an arbitrary subset of its changes is promoted anyway (a page losing its supporting source, breaking a wikilink, regressing readiness, or becoming a duplicate). Once merged, every promoted change is a measured candidate: a caller cannot filter an approved plan and silently inherit the whole candidate's evidence. It a
- Assessment: Good. It is coherent and sits squarely in the codebase's grain: it reuses improveKnowledgeBase + promoteKnowledgeCandidate rather than adding a parallel activation path, and follows the exact composition pattern already established by optimizeKnowledgeBasePolicy (src/kb-improvement/optimization.ts:88, which also wraps improveKnowledgeBase with a custom updateKnowledge callback). The integrity disc
- Better / existing approach: No materially better architecture exists. I searched src/ (grep for subset/partial/selectedPaths/selection and read transition.ts, workspace.ts, optimization.ts, activation.ts, evaluation.ts, contracts.ts) and found no existing primitive that re-measures a path-filtered subset of a measured candidate; optimizeKnowledgeBasePolicy materializes a policy, not a file subset, and 'selectionScenarios' el
- Model: opencode/deepseek/deepseek-v4-pro
- Bridge attempts: 3
- Bridge warning: opencode/kimi-for-coding/k2p7: opencode: opencode error; opencode/zai-coding-plan/glm-5.2: opencode: opencode error
🎯 Usefulness — sound
A safe partial-promotion capability that remeasures a caller-selected subset as an ordinary candidate, built entirely from the package's existing measurement and activation machinery with no second promotion path.
- Integration: Exported at the package root (src/kb-improvement.ts:43-49) and composes only established primitives: improveKnowledgeBase with a custom updateKnowledge callback (the same extension point optimizeKnowledgeBasePolicy already uses at src/kb-improvement/optimization.ts:143), withKnowledgeImprovementComparison (src/kb-improvement/workspace.ts:50), the canonical file-transaction/mutation-lock stack, and
- Fit with existing patterns: Fits the codebase's central invariant rather than competing with it: promotion is gated on promotionPlanHash/evidenceHash binding exactly the measured content (assertCandidateEvidence, src/kb-improvement/workspace.ts:209-243; assertCandidateTransitionPlan, src/kb-improvement/transition.ts:557-569). The obvious cheaper design — filtering the activation plan — would violate that invariant, and grepp
- Real-world viability: Holds up beyond the happy path: crash recovery routes through the same resumeTransaction validation the promotion path uses (src/kb-improvement/selected-candidate.ts:251-258), re-invocation is idempotent without re-running evaluators (test at tests/kb-improvement/selected-candidate.test.ts:190-226), source-content drift is caught by re-hashing each file before materialization (selected-candidate.t
- Model: opencode/zai-coding-plan/glm-5.2
- Bridge attempts: 1
💰 Value Audit
🟡 immutableJson re-implements the already-exported immutableJsonValue [duplication] ``
selected-candidate.ts:476 defines a module-private deep-freeze helper identical in effect to immutableJsonValue (activation.ts:294), which is exported and already used by workspace.ts:17. Import and reuse the existing helper instead of a third local deep-freeze copy.
🟡 selectionMutations duplicates knowledgePlanMutations [duplication] ``
selected-candidate.ts:410 (selectionMutations: read file, verify sha256+mode, emit KnowledgeFileMutation) is nearly identical to knowledgePlanMutations in transition.ts:540. They differ only in the error message ('changed before subset materialization' vs 'changed before activation'). Could be unified into one shared 'mutations for a filtered plan' helper, but this is a ~15-line overlap and does not gate shipping.
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.
❌ Needs Work —
|
| opencode GLM 5.2 | opencode DeepSeek v4 Pro | opencode DeepSeek v4 Flash | aggregate | |
|---|---|---|---|---|
| Readiness | 27 | 61 | 26 | 26 |
| Confidence | 75 | 75 | 75 | 75 |
| Correctness | 27 | 61 | 26 | 26 |
| Security | 27 | 61 | 26 | 26 |
| Testing | 27 | 61 | 26 | 26 |
| Architecture | 27 | 61 | 26 | 26 |
Reviewer score is advisory once the run is complete and the verdict has no blockers.
Full multi-shot audit completed 3/3 planned shots over 3 changed files. Global verifier still owns final merge decision. | Full multi-shot audit completed 3/3 planned shots over 3 changed files. Global verifier still owns final merge decision. | Full multi-shot audit completed 3/3 planned shots over 3 changed files. Global verifier still owns final merge decision.
Blocking
🔴 HIGH Failing assertion: error-message regex does not match the thrown error — tests/kb-improvement/selected-candidate.test.ts
expect(() => knowledgeImprovementCandidateRef(selected)).toThrow(/not ready for promotion/) fails. knowledgeImprovementCandidateRef -> candidateRefFor throws
knowledge candidate 'kcand_...' is not ready(src/kb-improvement/workspace.ts:116), which does not contain the substring 'for promotion'. The wording '...not ready for ${action}' exists only in src/kb-improvement/transition.ts:244, a path this call never reaches. Reproduced:pnpm vitest run tests/kb-improvement/selected-candidate.test.ts-> 'Tests 1 failed | 3 passed (4)'. Fix: assert /is not ready/ (or /not ready/), or align the production message with the intended regex. As written, this test is red and blocks CI.
🔴 HIGH Failing assertion: toThrow regex never matches actual error message — tests/kb-improvement/selected-candidate.test.ts
expect(() => knowledgeImprovementCandidateRef(selected)).toThrow(/not ready for promotion/) — actual throw is
knowledge candidate 'kcand_8550c04dd61c0d62' is not readyfrom candidateRefFor (src/kb-improvement/workspace.ts:116, pre-existing, unchanged by this PR). The 'not ready for promotion' wording only exists in src/kb-improvement/transition.ts:244 (knowledge candidate is not ready for ${action}) on the promote/restore path, which this direct call never hits. Verified by executing the suite: assertion fails deterministically across 3 runs (earlier toMatchObject({status:'rejected'}) and toMatchObject({passed:false}) pass, so only the regex is wrong). CI runspnpm test(vitest run) and will be red. Fix: change to .toThrow(/is not ready/) or assert the exact message.
🔴 HIGH Head commit ships a failing test and a lint error (CI red) — tests/kb-improvement/selected-candidate.test.ts
Verified by running
pnpm vitest run tests/kb-improvement/selected-candidate.test.ts --pool=threads --maxWorkers=1: Test Files 1 failed, Tests 1 failed | 3 passed. The failing assertionexpect(() => knowledgeImprovementCandidateRef(selected)).toThrow(/not ready for promotion/)never matches the actual thrown messageknowledge candidate 'kcand_...' is not ready(thrown by candidateRefFor in src/kb-improvement/workspace.ts:116). Product behavior is correct (a rejected candidate refuses promotion), but the test's expected-message regex is wrong, so the PR's test suite is red on head. Additionallypnpm lint(biome check src tests) fails on this same test file (formatter would add a trailing newline after [line 227](https://github.com/tangle-network/agent-knowledge/blob/4d11513c922ac4a68
🔴 HIGH New test fails deterministically at head: wrong error-message regex — tests/kb-improvement/selected-candidate.test.ts
Test asserts expect(() => knowledgeImprovementCandidateRef(selected)).toThrow(/not ready for promotion/), but the exported helper resolves to candidateRefFor in src/kb-improvement/workspace.ts:116 which throws
knowledge candidate '<id>' is not readyfor a rejected candidate. /not ready for promotion/.test("knowledge candidate 'kcand_a255de868da4f9c7' is not ready") is false — verified by a complete vitest run of this file (3 passed, 1 failed, exact assertion diff shown) and by static regex evaluation. Impact: CI is red at PR head; the production behavior itself is correct (rejected candidates correctly refuse a ref). Fix: change the regex to /is not ready/ or equivalent. One-line test fix, but merge-blocking until applied. (Cross-shot evidence: the test file belongs to another shot; repo
🔴 HIGH Test 2 error-message assertion can never pass — tests/kb-improvement/selected-candidate.test.ts
expect(() => knowledgeImprovementCandidateRef(selected)).toThrow(/not ready for promotion/) asserts an error that the implementation does not produce. knowledgeImprovementCandidateRef (workspace.ts:42) calls candidateRefFor, which for a rejected candidate throws
knowledge candidate '<candidateId>' is not ready(workspace.ts:116). RegExp /not ready for promotion/.test('knowledge candidate kimpsel-... is not ready') is false, so vitest's toThrow (chai throws -> message match) fails every run. The test always fails regardless of implementation behavior. Fix: match /not ready$/ or /is not ready/, or assert on selected.candidate.status === 'rejected' instead of the throw message.
Other
🟠 MEDIUM Base-drift precondition is unvalidated; subset derivation fails late with misleading errors — src/kb-improvement/selected-candidate.ts
The helper requires options.root to be byte-identical to the source candidate's frozen baseline, but never verifies
hashKnowledgeBase(options.root) === sourceCandidate.baseHashup front. The proof is in the comparison references: assertExactSelectedChanges (line 265) compares the derived candidate against source.baseline.root, assertSelectionTransaction (line 233) requires the transaction's beforeHash (read from the live-root copy, file-transaction.ts prepareKnowledgeFileTransaction) to equal the SOURCE
🟡 LOW Receipt type exported without its runtime zod schema — src/kb-improvement.ts
The barrel exports MeasuredKnowledgeSelectionReceipt as an inferred type, but the corresponding measuredSelectionReceiptSchema (selected-candidate.ts:66) is not exported as a value — unlike the established contracts pattern where persisted-record schemas are re-exported (KnowledgeImprovementRunStateSchema, KnowledgeImprovementEvidenceSchema, KnowledgeImprovementCandidateRefSchema at lines 32-34). The receipt is persisted to candidates//selection.json and re-parsed internally on resume (selected-candidate.ts:463), so downstream consumers reading that file from disk cannot re-validate it with the canonical schema and must hand-roll a structurally identical one.
🟡 LOW Receipt zod schemas not exported from the public barrel — src/kb-improvement.ts
The new module defines measuredSelectionReceiptSchema/measuredSelectionLifecycleSchema (selected-candidate.ts:53-85), and the receipt type is public (MeasuredKnowledgeSelectionReceipt). But kb-improvement.ts only exports the types and the function, not the schemas. contracts.ts sets the package precedent of exporting its schemas as values (kb-improvement.ts:31-35), and package.json's exports map publishes only the index, so a consumer who reads candidates//selection.json cannot validate it without re-implementing the schema. Fix: add
export { measuredSelectionLifecycleSchema, measuredSelectionReceiptSchema } from './kb-improvement/selected-candidate'to the barrel, consistent with the sibling module.
🟡 LOW DERIVED_KNOWLEDGE_PATHS only lists knowledge/index.md, inconsistent with isScaffoldPath — src/kb-improvement/selected-candidate.ts
DERIVED_KNOWLEDGE_PATHS = new Set(['knowledge/index.md']) is used by notDerivedPath to exclude 'derived' paths from the selectable changed set and from assertExactSelectedChanges. store.ts:isScaffoldPath (lines 56-63) treats knowledge/index.md, knowledge/log.md, AND any nested
/index.md or /log.md as non-searchable scaffold pages. So knowledge/log.md and nested scaffold paths are NOT excluded here: if a source candidate's update/research hook modified such a path, it would be exposed as an ordinary selectable file and carried into the derived subset instead of being treated as derived content. Impact is low (content is still remeasure
🟡 LOW No up-front check that the live root still equals the source candidate's baseline — src/kb-improvement/selected-candidate.ts
Selection is defined against the frozen source baseline, but the derived run copies the LIVE root as its baseline (run.ts:68-69) and nothing compares the two up front. When the live KB has drifted (e.g. another candidate was promoted since), failure arrives late and indirect: drift on a selected path makes prepareKnowledgeFileTransaction's beforeHash (from the live workspace copy) disagree with the plan hash frozen from the source baseline, yielding 'selected knowledge transaction does not match its approved path set' (line 432); drift on unselected paths surfaces as 'selected knowledge candidate changed the wrong files' ([line 446](https://github
🟡 LOW Re-running the helper after the derived candidate is promoted throws an opaque ZodError — src/kb-improvement/selected-candidate.ts
After promoteKnowledgeCandidate promotes the derived candidate, a second improveSelectedKnowledgeCandidate call with identical inputs makes improveKnowledgeBase take its promoted early-return path (src/kb-improvement/run.ts:99-123), which returns no
lifecycleand never invokes updateKnowledge, so in-processlifecycleSelectionis also undefined. measuredSelectionLifecycleSchema.parse(result.lifecycle?.knowledgeUpdate?.metadata?.selection ?? lifecycleSelection) then parses undefined and throws a raw ZodError before the clearer receipt-conflict check in persistSelectionReceipt (line 466-467) can produce 'measured knowledge selection receipt
🟡 LOW Selection receipt does not bind the base the derived candidate was actually measured against — src/kb-improvement/selected-candidate.ts
receiptWithoutHash records sourceCandidate, sourcePlanHash, selectedCandidateHash, selectedPlanHash, selectedEvidenceHash, and derivedCandidateId, but not the derived candidate's baseHash. The derived run snapshots its baseline from the live root at derived-run start (run.ts createBaselineSnapshot), which under the current code only ever equals the source baseHash when the base has not drifted (see the medium finding above). A verifier reading the receipt later cannot confirm that the measured subset was measured against the base named by sourceCandidate, because the derived evidence.baseHash lives only in the candidate's evidence.json. Adding a
derivedBaseHash(or asserting equality with sourceCandidate.baseHash) to the receipt would make the source-base/derived-base relationship audita
🟡 LOW Type-omitted update-driver fields are not stripped at runtime — src/kb-improvement/selected-candidate.ts
improvementOptions destructures out the selection-specific fields but not
acquireKnowledge,step, orknowledgeResearch, which are excluded only by the Omit<> on ImproveSelectedKnowledgeCandidateOptions. A JavaScript oras anycaller passing acquireKnowledge would smuggle a second knowledge-update driver into the derived run (evaluation.ts:73-81 allows one driver; acquireKnowledge would run before the selection hook inside the same update loop, letting unselected content enter the candidate). updateKnowledge, enabledPhases, requiredPhases, root, goal, runId, and implementationRef are safe because explicit keys after the spread ([lines 208-216](
🟡 LOW recordedSelection fallback throws a confusing ZodError when the derived run is already promoted — src/kb-improvement/selected-candidate.ts
recordedSelection = measuredSelectionLifecycleSchema.parse(result.lifecycle?.knowledgeUpdate?.metadata?.selection ?? lifecycleSelection). If a caller reuses an options.runId whose run is already 'promoted', improveKnowledgeBase (run.ts:99-123) returns early with no lifecycle, and updateKnowledge never runs, so both operands are undefined and zod throws a bare 'Invalid input' ZodError with no run context. The subsequent binding check (lines 297-304) never executes. Not a data-integrity bug (fail-closed), but the diagnostic hides the real cause. Fix: guard first, e.g. throw new Error('selected knowledge candidate produced no measured selection
🟡 LOW Missing trailing newline at EOF fails formatter/lint — tests/kb-improvement/selected-candidate.test.ts
The file ends on
})with no trailing newline.pnpm biome check tests/kb-improvement/selected-candidate.test.tsreports 'Formatter would have printed the following content' and exits non-zero. The repo lint script isbiome check src tests, so this file failspnpm lint. Fix: add the trailing newline (biome format --write).
🟡 LOW Missing trailing newline fails pnpm lint (biome check) in CI — tests/kb-improvement/selected-candidate.test.ts
File has no newline at EOF (diff shows '\ No newline at end of file').
npx biome check tests/kb-improvement/selected-candidate.test.tsemits 1 format error ('Found 1 error') proposing to add the trailing blank line. CI runspnpm lint=biome check src tests(ci.yml:26), so this file alone reds the lint gate. Fix:biome format --writeor append newline.
🟡 LOW Missing trailing newline fails repo lint gate — tests/kb-improvement/selected-candidate.test.ts
File ends at '})' with no final newline.
biome check tests/kb-improvement/selected-candidate.test.tsreports a formatter error (verified locally: 'Formatter would have printed...'), sopnpm lint(biome check src tests) fails on this file. Add the terminating newline.
🟡 LOW Test title claims validation happens 'before opening a derived run' but never verifies it — tests/kb-improvement/selected-candidate.test.ts
The three rejects.toThrow assertions verify the error messages, but nothing asserts that no derived run directory was created under the KB root (e.g., checking the runs dir is empty after the throws). The ordering claim in the test name — the safety property that malformed selections cost nothing — is unverified. Add an assertion that no new run dir exists for the selection digest after each rejection.
tangletools · 2026-08-17T04:13:49Z · trace
tangletools
left a comment
There was a problem hiding this comment.
❌ 5 Blocking Findings — 4d11513c
Full multi-shot audit completed 3/3 planned shots over 3 changed files. Global verifier still owns final merge decision. | Full multi-shot audit completed 3/3 planned shots over 3 changed files. Global verifier still owns final merge decision. | Full multi-shot audit completed 3/3 planned shots over 3 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-17T04:13:49Z · immutable trace
…ally throws candidateRefFor throws "knowledge candidate '<id>' is not ready". The assertion expected "not ready for promotion", a wording that exists only on the promote path this call never reaches, so the test failed on every run. Match the thrown message and terminate the file with a newline for biome.
|
@tangletools review now |
tangletools
left a comment
There was a problem hiding this comment.
✅ Auto-approved drewstone PR — 3d5dd9b5
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-17T05:23:06Z
tangletools
left a comment
There was a problem hiding this comment.
⚠️ Value Audit did not run — no verdict
This is not an approval and not a clean bill of health. Neither interrogation lens returned a judgment, so this PR has no value-audit evidence for or against it.
| Status | audit-incomplete (could not run) |
| Why | value: cli-bridge admission rejected (queue saturated); usefulness: cli-bridge admission rejected (queue saturated) |
| Lenses answered | 0 of 2 |
| What to do | re-run once the CLI bridge has capacity: pr-reviewerctl trigger <repo>#<pr> --force |
💰 Value — error
value agent never ran: the CLI bridge refused admission (no model was started).
- Model: opencode/deepseek/deepseek-v4-pro
- Bridge attempts: 4
- Bridge error: opencode/kimi-for-coding/k2p7: Bridge returned 503: bridge at capacity (queue_timeout, lane=reserved): active=20/20 queued=0/48 — no model was started
🎯 Usefulness — error
usefulness agent never ran: the CLI bridge refused admission (no model was started).
- Model: opencode/deepseek/deepseek-v4-pro
- Bridge attempts: 4
- Bridge error: opencode/zai-coding-plan/glm-5.2: Bridge returned 503: bridge at capacity (queue_timeout, lane=reserved): active=20/20 queued=1/48 — no model was started
No concerns are listed because nothing examined the change — absence of findings here is absence of evidence, not a pass.
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 could not run (value: cli-bridge admission rejected (queue saturated); usefulness: cli-bridge admission rejected (queue saturated)). This approval was provisional on that audit running, so it is dismissed. Re-run the reviewer once the CLI bridge has capacity.
tangletools
left a comment
There was a problem hiding this comment.
⚠️ Value Audit did not run — no verdict
This is not an approval and not a clean bill of health. Neither interrogation lens returned a judgment, so this PR has no value-audit evidence for or against it.
| Status | audit-incomplete (could not run) |
| Why | value: cli-bridge admission rejected (queue saturated); usefulness: cli-bridge admission rejected (queue saturated) |
| Lenses answered | 0 of 2 |
| What to do | re-run once the CLI bridge has capacity: pr-reviewerctl trigger <repo>#<pr> --force |
💰 Value — error
value agent never ran: the CLI bridge refused admission (no model was started).
- Model: opencode/deepseek/deepseek-v4-pro
- Bridge attempts: 5
- Bridge error: opencode/kimi-for-coding/k2p7: opencode: opencode error; opencode/zai-coding-plan/glm-5.2: Bridge returned 503: bridge at capacity (queue_timeout, lane=reserved): active=20/20 queued=6/48 — no model was started
🎯 Usefulness — error
usefulness agent never ran: the CLI bridge refused admission (no model was started).
- Model: opencode/deepseek/deepseek-v4-pro
- Bridge attempts: 4
- Bridge error: opencode/zai-coding-plan/glm-5.2: Bridge returned 503: bridge at capacity (queue_timeout, lane=reserved): active=20/20 queued=12/48 — no model was started
No concerns are listed because nothing examined the change — absence of findings here is absence of evidence, not a pass.
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.
tangletools
left a comment
There was a problem hiding this comment.
⚠️ Value Audit did not run — no verdict
This is not an approval and not a clean bill of health. Neither interrogation lens returned a judgment, so this PR has no value-audit evidence for or against it.
| Status | audit-incomplete (could not run) |
| Why | value: cli-bridge admission rejected (queue saturated); usefulness: cli-bridge admission rejected (queue saturated) |
| Lenses answered | 0 of 2 |
| What to do | re-run once the CLI bridge has capacity: pr-reviewerctl trigger <repo>#<pr> --force |
💰 Value — error
value agent never ran: the CLI bridge refused admission (no model was started).
- Model: opencode/deepseek/deepseek-v4-pro
- Bridge attempts: 4
- Bridge error: opencode/kimi-for-coding/k2p7: Bridge returned 503: bridge at capacity (queue_timeout, lane=reserved): active=20/20 queued=19/48 — no model was started
🎯 Usefulness — error
usefulness agent never ran: the CLI bridge refused admission (no model was started).
- Model: opencode/deepseek/deepseek-v4-pro
- Bridge attempts: 4
- Bridge error: opencode/zai-coding-plan/glm-5.2: Bridge returned 503: bridge at capacity (queue_timeout, lane=reserved): active=20/20 queued=18/48 — no model was started
No concerns are listed because nothing examined the change — absence of findings here is absence of evidence, not a pass.
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.
✅ No Blockers —
|
Superseded by re-review — no blocking findings on latest commit.
tangletools
left a comment
There was a problem hiding this comment.
✅ Approved — 3 non-blocking findings — 3d5dd9b5
Full multi-shot audit completed 3/3 planned shots over 3 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-17T06:27:01Z · immutable trace
tangletools
left a comment
There was a problem hiding this comment.
🟢 Value Audit — sound
| Verdict | sound |
| Coverage | 1 of 2 lenses (usefulness) |
| Concerns | 1 (1 weak-concern) |
| Heuristic | 0.0s |
| Duplication | 0.0s |
| Interrogation | 665.0s (2 bridge agents) |
| Total | 665.0s |
⚠️ Partial audit — the verdict covers only usefulness. value: cli-bridge admission rejected (queue saturated). Treat the missing lens as unexamined, not as clear.
💰 Value — error
value agent never ran: the CLI bridge refused admission (no model was started).
- Model: opencode/deepseek/deepseek-v4-pro
- Bridge attempts: 4
- Bridge error: opencode/kimi-for-coding/k2p7: Bridge returned 503: bridge at capacity (queue_timeout, lane=reserved): active=20/20 queued=15/48 — no model was started
🎯 Usefulness — sound
A well-fitted library capability that fills a real gap — sub-selecting a measured candidate's changes has no safe route today because promotion's plan-hash binding structurally rejects any filtered plan — and it closes that gap by routing the subset through the same single measurement-and-promotion
- Integration: Exported at the package root (src/kb-improvement.ts:43-49 -> src/index.ts:27 'export * from ./kb-improvement') of the published library @tangle-network/agent-knowledge@8.0.7, so it is reachable by every downstream consumer on publish. In-repo callers are tests only (tests/kb-improvement/selected-candidate.test.ts), which is the standing pattern for this surface: promoteKnowledgeCandidate (transiti
- Fit with existing patterns: Follows the codebase's established composition pattern exactly: optimizeKnowledgeBasePolicy (optimization.ts:143-177) already wraps improveKnowledgeBase with a derived deterministic implementationRef, a stableId runId, and a custom updateKnowledge that materializes a derived candidate and binds metadata into lifecycle evidence — this helper is the same grain applied to path selection. It reuses th
- Real-world viability: Built past the happy path: crash recovery via resumeTransaction with plan-hash validation (selected-candidate.ts:251-258); tamper detection at three points — source plan re-verified against the frozen promotionPlanHash (167-169), each file re-hashed before materialization (417-421), and post-materialization exact-change assertion (261-265); evidence-to-candidate binding re-checked after the run (2
- Model: opencode/zai-coding-plan/glm-5.2
- Bridge attempts: 4
🎯 Usefulness Audit
🟡 Empty-selection null-candidate path is deliberately admitted but unproven [robustness] ``
selected-candidate.ts:280-284 explicitly supports selectedPaths: [] ('measuring the exact baseline as a null candidate'), yielding a candidate-ready run whose promotion would be a content no-op; the suite (tests/kb-improvement/selected-candidate.test.ts) never exercises it, so its evaluator/receipt/promotion behavior is asserted only by construction. Either add one test pinning the null-candidate outcome or reject empty selections at the boundary — worth a reviewer's eye, but it flows the ordina
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.
|
Disposition of the 3 non-blocking LOW findings on Local verification (worktree
Finding 1 — untested selection edge paths (LOW). Accepted as a coverage gap, not a defect. The named branches (empty Finding 2 — Finding 3 — bare catch in the dependency-check evaluator (LOW). Accepted, test-only. The No finding blocks merge. Merging on the multi-shot |
The unsafe shortcut this avoids
A whole knowledge candidate can pass while an arbitrary subset of its file changes fails: a selected page may lose its supporting source, break a wikilink, regress readiness, or become a duplicate. Filtering the already-approved activation plan would silently inherit the whole candidate's evidence for a hybrid that was never measured.
Change
Add
improveSelectedKnowledgeCandidate, which derives a caller-selected subset from one previously measured candidate and then routes that subset through the ordinary knowledge-improvement evidence and promotion path as a new candidate.The helper:
promotionPlanHash;knowledge/index.mdpaths;selection.jsonreceipt beside the derived candidate;promoteKnowledgeCandidatepath.There is no second activation path and no evidence reuse from the broad source candidate.
Tests
The suite proves:
This closes the partial-promotion gap from Discovery's adoption audit with a stronger invariant than filtering an activation plan: every promoted subset has its own evidence.