fix(campaign): preserve proposer state and private profile fields#357
fix(campaign): preserve proposer state and private profile fields#357drewstone wants to merge 1 commit into
Conversation
tangletools
left a comment
There was a problem hiding this comment.
✅ Auto-approved drewstone PR — 41e62eeb
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-13T07:26:23Z
tangletools
left a comment
There was a problem hiding this comment.
🟡 Value Audit — sound-with-nits
| Verdict | sound-with-nits |
| Concerns | 1 (1 weak-concern) |
| Heuristic | 0.0s |
| Duplication | 0.0s |
| Interrogation | 448.0s (2 bridge agents) |
| Total | 448.0s |
💰 Value — sound-with-nits
Fixes a real stateful-proposer-in-composite bug and adds a well-guarded opt-in surface projection for private fields; both are correct and in-grain, with only minor helper duplication.
- What it does: Two independent fixes. (1) composite.ts now strips each member's provenance prefix from history labels before passing history to that member, so stateful proposers like parameterSweepProposer recognize their own prior candidates and skip them instead of re-proposing every generation (historyForMember at composite.ts:161-171, applied in both propose:94-98 and decide:140-144). (2) llm-policy-edit.ts
- Goals it achieves: (1) Stop stateful members inside a composite from repeating already-tried candidates — the composite's label-prefix scheme (memberKind:label) was hiding each member's own labels from its own dedup logic (parameterSweepProposer.triedLabels at fapo.ts:682 compares candidate.label, which after prefixing no longer matches). (2) Let callers keep credentials/private config out of LLM author context with
- Assessment: Both fixes are correct, well-scoped, and match the codebase's established patterns. The composite fix works within the existing prefix-based provenance scheme (documented at composite.ts:16-19 as intentional), stripping exactly one prefix level so nested composites also work. The projection feature is the right abstraction level — it is caller-controlled and structural (can drop entire subtrees),
- Better / existing approach: Searched for existing JSON-path and deep-equal utilities: readJsonAtPath + splitPath exist as private functions in policy-edit.ts:613,865 (functionally identical to the new readJsonPath at llm-policy-edit.ts:1293), and canonicalJson exists as a private comparator in fapo.ts:746 (could substitute for jsonValuesEqual). Neither is exported, so the new helpers couldn't import them. Also checked trace/
- 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
Fixes a real stateful-member label collision bug in compositeProposer and adds a safe, opt-in credential-stripping projection to llmPolicyEditProposer — both correctly wired into the substrate's public surface.
- Integration: Both proposers are exported from src/campaign/index.ts (lines 241, 274), documented in docs/improvement-glossary.md and CHANGELOG.md, and verified by scripts/verify-package-exports.mjs. They are substrate primitives consumed by downstream packages (agent-runtime, products) — standard for this repo's layering (CLAUDE.md: 'agent-eval has zero upward dependencies'). No internal non-test callers exist
- Fit with existing patterns: Both changes follow established codebase patterns. compositeProposer already does label-prefix provenance (composite.ts:115) and fapoProposer does its own label wrapping (fapo.ts:388) — historyForMember is the natural un-prefixing complement. projectAuthorSurface complements the existing scenarioIdTransform pseudonymizer (llm-policy-edit.ts:420) which only handles scenario IDs, not arbitrary priva
- Real-world viability: The composite fix handles null labels via optional chaining (composite.ts:166) and non-matching prefixes via pass-through. The projection has comprehensive edge-case coverage: non-object rejection (test ~line 318), hidden editable-path rejection (~line 343), mutation prevention (~line 375), whitespace-key rejection (~line 401), and task-identifier leak prevention (~line 413). structuredClone (line
- Model: opencode/zai-coding-plan/glm-5.2
- Bridge attempts: 1
💰 Value Audit
🟡 readJsonPath and jsonValuesEqual duplicate private helpers in sibling modules [duplication] ``
readJsonPath (llm-policy-edit.ts:1293) reimplements the exact dot-path traversal of readJsonAtPath + splitPath in policy-edit.ts:613,865. jsonValuesEqual (llm-policy-edit.ts:1305) reimplements structural deep-equal that canonicalJson in fapo.ts:746 could provide via canonical-string comparison. All three are private. The codebase now has three independent JSON-path micro-utilities. Not ship-blocking — consider extracting a shared src/json-path.ts (readPath, deepEqual) and exporting it from the s
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 —
|
tangletools
left a comment
There was a problem hiding this comment.
✅ Approved — 4 non-blocking findings — 41e62eeb
Full multi-shot audit completed 3/3 planned shots over 5 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-13T07:50:17Z · immutable trace
Problem
Composite proposal labels hid member state, causing stateful methods to repeat candidates. Policy-edit authoring also needed a safe way to omit private profile fields from model context.
Solution
Verification