mirror of
https://github.com/garrytan/gbrain.git
synced 2026-07-28 06:23:01 +00:00
Garry TanandClaude Opus 4.7
f311f1368a
Two related v0.42.0.0 bugs that conspired to make `runSkillOpt` structurally
unable to accept any candidate edit. Either alone would have killed self-evolution;
together they made the loop a no-op for every input.
**Bug 1 (orchestrator gap):** `runOptimizationLoop` in orchestrator.ts called
`runReflect({successes: [], failures: []})` with hardcoded empty arrays. The
forward gate's `scoredRollouts` were computed then voided. `runReflect`
short-circuits both modes when their batches are empty, so the optimizer was
never asked to propose an edit. Every step hit the no_edits_applied branch.
Fix: add `scoredRollouts: ScoredRollout[]` to `GateResult` and
`runsPerTask?: number` to `ValidateGateOpts`. Forward pass uses
`runsPerTask: 1`; orchestrator partitions returned rollouts by `score >= 0.5`
and threads real successes + failures into `runReflect`.
**Bug 2 (parser misuse):** `parseEditsResponse` in reflect.ts routed every
optimizer response through `parseJudgeJson` first. `parseJudgeJson` looks for
a `score` key (it's a judge-output parser, not an edits parser) and returns
null for any JSON without one — including the well-formed `{"edits": [...]}`
the optimizer is contractually required to emit. The function then early-
returned `[]` and the actual `tryExtractEdits` path on the next line was
unreachable dead code.
Fix: drop the wrong-typed guard. `parseEditsResponse` now calls
`tryExtractEdits` directly. Export it so `reflect.test.ts` can pin the
contract independently of the chat transport.
**Why this slipped through 152 prior skillopt tests:** zero unit coverage
of `parseEditsResponse` or `runReflect`. The existing E2E `all-reject` case
asserted no_improvement (which was true for the wrong reason — empty edits,
not gate rejection). Both bugs were structurally invisible to the existing
test surface.
**New coverage:**
- `test/skillopt/reflect.test.ts` (15 cases):
- 8 `parseEditsResponse` cases including the IRON-RULE regression pin
for the v0.42.0.1 fix (`{"edits": [...]}` JSON must survive the parser).
- 7 `runReflect` D7 contract cases: both modes fire, empty-batch skips,
additive token usage, one-mode-throws-other-still-works, rejected-buffer
flows into anti-bias prompt.
- Documents the trailing-comma limitation as an explicit out-of-scope pin
(so a future tightening of `tryExtractEdits` lights this test up
intentionally).
- `test/e2e/skillopt-loop.serial.test.ts` (7 cases):
- HAPPY PATH: stubbed `gateway.chat` acts as both target agent (emits
sections based on skill content) and optimizer (proposes a real
add-Citations edit). Drives `runSkillOpt` end-to-end against PGLite.
Asserts outcome=accepted, SKILL.md mutated with new section,
frontmatter preserved (D5), history has one committed row,
best.md mirrors disk, delta > epsilon, receipt fields populated.
- 5 broken cases (each isolates a distinct orchestrator-visible failure):
1. Below-baseline regression: optimizer proposes a destructive edit;
gate rejects with reason=below_baseline; SKILL.md unchanged;
rejected-buffer captures the bad edit for anti-bias context.
2. Malformed reflect JSON: orchestrator degrades gracefully to
no_improvement without crashing.
3. Anchor-not-found: applyEditBatch rejects all; sel gate skipped;
rejected-buffer captures with reason=apply_failed.
4. Budget exhausted mid-step: outcome=aborted, no pending rows survive.
5. Converged-skill re-run: starting from already-perfect skill →
no_improvement (no thrash on a well-tuned starting point).
- IDEMPOTENT RE-RUN: drive runSkillOpt twice in sequence. Run 1 accepts.
Run 2 sees improved baseline, no failures, returns no_improvement.
SKILL.md byte-identical to post-run-1; history still has exactly 1
committed row. Proves stability at the fixed point.
All hermetic (no DATABASE_URL, no API keys). PGLite in-memory engine,
tempdir SKILL.md + benchmark, stubbed gateway.chat via
`__setChatTransportForTests`. `.serial.test.ts` because the stub installs
module state and the loop walks shared disk state across epochs.
Test counts after fix: 174 skillopt-surface tests pass (149 pre-existing
unit + 15 new reflect unit + 3 existing E2E + 7 new E2E). Typecheck clean.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>