From 08ad24688bdd30f769a3949eba5c0166251d0540 Mon Sep 17 00:00:00 2001 From: Garry Tan Date: Wed, 27 May 2026 08:31:24 -0700 Subject: [PATCH] fix(cycle): align ALL_PHASES skillopt position with actual dispatch order MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit v0.42.0.0 added skillopt to ALL_PHASES right after `patterns` (line 127), but the dispatch block in runCycle (line ~1912) actually runs skillopt between `conversation_facts_backfill` and `embed`. The two were inconsistent, and the serial test `report.phases.map(p => p.phase)).toEqual(ALL_PHASES)` was failing on master because of it. A second pre-existing failure: the two phase-count assertions in `test/core/cycle.serial.test.ts` still said `toBe(20)` even though ALL_PHASES grew to 21 when skillopt was added. The author bumped the array but forgot the test. Two fixes, one commit: 1. Move `'skillopt'` in ALL_PHASES from after `patterns` to between `conversation_facts_backfill` and `embed`, matching where runCycle actually dispatches it. Runtime behavior is unchanged — only the declaration order moves. Updated the surrounding comment to call out the position invariant and reference the test that pins it. 2. Update both `toBe(20)` assertions in cycle.serial.test.ts to `toBe(21)` with a v0.42.0.0 history line in the running comments. Why declaration follows runtime (not the other way around): the comment intent ("Runs AFTER patterns — graph-fresh") is still satisfied because "after the entire main graph-mutating cluster" is strictly fresher than "right after patterns". No design intent is lost. Test result: cycle.serial.test.ts is now 28/28 (was 27/28 on master + my prior commit). Skillopt suite still 174/174. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/core/cycle.ts | 18 ++++++++++++------ test/core/cycle.serial.test.ts | 6 ++++-- 2 files changed, 16 insertions(+), 8 deletions(-) diff --git a/src/core/cycle.ts b/src/core/cycle.ts index 0d64c4eea..4b579a597 100644 --- a/src/core/cycle.ts +++ b/src/core/cycle.ts @@ -119,12 +119,6 @@ export const ALL_PHASES: CyclePhase[] = [ // BATCH_SIZE*10 chunks where edges_backfilled_at IS NULL or stale. 'resolve_symbol_edges', 'patterns', - // v0.41.20.0 SkillOpt — self-evolving skills phase. Runs AFTER patterns - // (graph-fresh) so any skill that depends on cross-session themes gets - // optimized against the freshest state. Default OFF; opt-in via - // `gbrain config set cycle.skillopt.enabled true`. Bundled-skill safety - // (D16): never auto-mutates bundled skills. - 'skillopt', // v0.41 T9 — concept synthesis (global, pack-gated). Runs AFTER patterns // so the cluster pass sees fresh cross-session themes. Same pack-gate // model as extract_atoms. @@ -158,6 +152,18 @@ export const ALL_PHASES: CyclePhase[] = [ // block placement, which runs between the calibration trio and embed), // and BEFORE embed so newly-inserted facts get embedded same-cycle. 'conversation_facts_backfill', + // v0.41.20.0 SkillOpt — self-evolving skills phase. Dispatch order + // places it AFTER the main graph-mutating cluster (extract, patterns, + // consolidate, calibration, conversation-facts) so any skill that + // depends on cross-session themes gets optimized against the freshest + // state — strictly fresher than "right after patterns" since downstream + // phases also mutate state the optimizer reads. Default OFF; opt-in via + // `gbrain config set cycle.skillopt.enabled true`. Bundled-skill safety + // (D16): never auto-mutates bundled skills. Position MUST match the + // dispatch block in runCycle (see line ~1912) — pinned by the + // `report.phases.map(p => p.phase)).toEqual(ALL_PHASES)` assertion in + // test/core/cycle.serial.test.ts. + 'skillopt', 'embed', 'orphans', // v0.39 T12: passive schema-suggest. Runs LATE so post-sync brain state diff --git a/test/core/cycle.serial.test.ts b/test/core/cycle.serial.test.ts index 960dc230f..b8c734c0c 100644 --- a/test/core/cycle.serial.test.ts +++ b/test/core/cycle.serial.test.ts @@ -392,7 +392,8 @@ describe('runCycle — yieldBetweenPhases hook', () => { // v0.39.0.0: 17 phases (added `schema-suggest` between orphans and purge — T12 schema cathedral). // v0.41.2.0: 19 phases (added `extract_atoms` after extract_facts + `synthesize_concepts` after patterns). // v0.41.11.0: 20 phases (added `conversation_facts_backfill` between consolidate and propose_takes). - expect(hookCalls).toBe(20); + // v0.42.0.0: 21 phases (added `skillopt` after patterns — self-evolving skills cycle phase). + expect(hookCalls).toBe(21); }); test('hook exceptions do not abort the cycle', async () => { @@ -406,7 +407,8 @@ describe('runCycle — yieldBetweenPhases hook', () => { // v0.36.1.0: 16 phases (Hindsight calibration wave adds propose_takes, grade_takes, calibration_profile). // v0.39.0.0: 17 phases (T12 schema-suggest phase between orphans and purge). // v0.41.11.0: 20 phases (+extract_atoms, +synthesize_concepts, +conversation_facts_backfill). - expect(report.phases.length).toBe(20); + // v0.42.0.0: 21 phases (+skillopt after patterns). + expect(report.phases.length).toBe(21); }); });