diff --git a/CHANGELOG.md b/CHANGELOG.md index bf8c1ab67..0d5ea93df 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,20 +2,20 @@ ## [Unreleased] -**/plan-tune's inferred profile no longer pins at 1.0 after a few reviews, and it now shows how many answers moved each dimension.** This closes the last two parts of #2623. One /plan-eng-review produces about five answers, and every answer added its full delta, so eight sessions of "fix now" took `architecture_care` to 0.998, as uninformative as the 0.5 it replaced. Now each logged session moves a dimension at most as far as one strong answer, and the same eight sessions read 0.808. The profile also records `contributing_events` per dimension, so a dimension with no evidence reads 0 instead of a bare 0.5. +**/plan-tune's inferred profile no longer pins at 1.0 after a few reviews, and it now shows how many answers moved each dimension.** This closes the last two parts of #2623. One /plan-eng-review produces about five answers, and every answer added its full delta, so eight sessions of "fix now" took `architecture_care` to 0.998, as uninformative as the 0.5 it replaced. Now each UTC day moves a dimension at most as far as one strong answer, and eight days of the same answers read 0.808. The profile also records `contributing_events` per dimension, so a dimension with no evidence reads 0 instead of a bare 0.5. ### Fixed -- **`bin/gstack-developer-profile`**: `--derive` sums each session's deltas per dimension and clamps the sum to the largest single delta in the signal map (±0.06 today). Answers within a session net out before the clamp, and a row whose `session_id` is missing, empty or an unsubstituted placeholder such as `SESSION_ID` is its own session. `--trace` uses the same accumulation, so it lists every contributing answer, each clamped session ("session review-1: +0.250 capped to +0.060") and a `TOTAL` line whose value matches `--derive`. The bound keys on the logged `session_id`, so an answer that both the PostToolUse hook and the agent log (under different ids) still counts once per writer. -- **`bin/gstack-developer-profile`**: a logged choice named after an `Object.prototype` member, such as `constructor`, or a log line that parses to a non-object such as `null`, no longer makes every later `--derive` and `--trace` fail; signal lookups use own properties only. `--trace` replaces control characters in the logged fields it prints, so a logged `session_id` can't forge a `TOTAL` line. +- **`bin/gstack-developer-profile`**: `--derive` sums each UTC day's deltas per dimension (by the row's `ts`) and clamps the sum to the largest single delta in the signal map (±0.06 today). Answers within a day net out before the clamp, and a row without a parseable `ts` is its own bucket. The bound ignores `session_id` on purpose: the PostToolUse hook and the agent log different ids for one review, and an unsubstituted `SESSION_ID` placeholder would merge every review into one bound or leave each answer unbounded. Several reviews on one day count as one. `--trace` uses the same accumulation, so it lists every contributing answer, each clamped day ("day 2026-04-01: +0.250 capped to +0.060") and a `TOTAL` line whose value matches `--derive`. +- **`bin/gstack-developer-profile`**: a logged choice named after an `Object.prototype` member, such as `constructor`, or a log line that parses to a non-object such as `null`, no longer makes every later `--derive` and `--trace` fail; signal lookups use own properties only. `--trace` replaces control characters in the logged fields it prints, so a logged `ts` can't forge a `TOTAL` line. ### Added - **`inferred.contributing_events`**: per dimension, the number of logged answers that moved it (an answer that writes two deltas to one dimension counts once). Ported from #2625 by @v639dragoon, with co-author credit. #2625's autonomy signal from `followed_recommendation` stays parked, as decided in its Oct 8 review. ### Docs -- `docs/designs/PLAN_TUNING_V0.md` documents `signal_events`, `contributing_events` and the per-session bound in the profile schema. +- `docs/designs/PLAN_TUNING_V0.md` documents `signal_events`, `contributing_events` and the per-day bound in the profile schema, including why the bound ignores `session_id`. ### Tests -- `test/gstack-developer-profile.test.ts`: `contributing_events` counts unique rows when one answer writes two dimensions, and is all zero for an answer that matches no signal. The bound equals the signal map's largest delta. Five answers in one session move a dimension by exactly the bound in both directions. `--trace` prints the clamped sessions and a total that matches `--derive`, and replaces control characters. A `constructor` choice and a `null` line are ignored. Negative controls: the same five answers in five sessions, rows with a missing, empty or placeholder `session_id`, and an answer pair that nets out are not clamped. The four original positive tests fail against the old bin, the `constructor` and control-character tests fail against the previous head, and a one-sided clamp fails the bound tests. +- `test/gstack-developer-profile.test.ts`: `contributing_events` counts unique rows when one answer writes two dimensions, and is all zero for an answer that matches no signal. The bound equals the signal map's largest delta. Five answers on one day move a dimension by exactly the bound in both directions, whether they carry one `session_id`, five different ones, the literal `SESSION_ID` placeholder, or one review logged by both the hook and the agent. The day is the UTC day of `ts`. `--trace` prints the clamped days and a total that matches `--derive`, and replaces control characters. A `constructor` choice and a `null` line are ignored. Negative controls: the same answers on separate days, a second day after a capped first day, rows without a parseable `ts`, and an answer pair that nets out are not clamped. The original positive tests fail against the old bin; the day-bound, `constructor` and control-character tests fail against the per-session version; a one-sided clamp fails the bound tests. - The test helper `logQuestion` sets `GSTACK_QUESTION_LOG_NO_DERIVE=1`, so the background derive can't race a test's own `--derive` (from #2625). ### Gate integrity diff --git a/bin/gstack-developer-profile b/bin/gstack-developer-profile index ec4378451..642db13f8 100755 --- a/bin/gstack-developer-profile +++ b/bin/gstack-developer-profile @@ -12,7 +12,7 @@ # --profile emit the full profile as JSON (all fields). # --gap emit declared-vs-inferred gap as JSON. # --trace show events that contributed to a dimension, each -# session clamped by the per-session bound, and the total. +# UTC day clamped by the per-day bound, and the total. # --narrative (v2 stub) output a coach bio paragraph. # --vibe (v2 stub) output the one-word archetype. # --check-mismatch detect meaningful gaps between declared and observed. @@ -407,14 +407,15 @@ do_gap() { # ----------------------------------------------------------------------- # Signal accumulation shared by --derive and --trace, so the trace always # explains exactly the value derive wrote. Each row's SIGNAL_MAP deltas are -# summed per (session, dimension) and each session's sum is clamped to -# ±cap, where cap is the largest single delta in SIGNAL_MAP: one session -# moves a dimension at most as far as one strong answer. Without the bound -# a single /plan-eng-review (about five answers) moved a dimension by 0.25 -# and about eight sessions pinned it at 1.0 (#2623). A row without a -# session_id, or with an unsubstituted placeholder such as SESSION_ID, is -# its own session. The bound keys on the writer's session_id, -# which differs between the PostToolUse hook and the agent's own log call. +# summed per (UTC day, dimension) and each day's sum is clamped to ±cap, +# where cap is the largest single delta in SIGNAL_MAP: one day moves a +# dimension at most as far as one strong answer. Without the bound a single +# /plan-eng-review (about five answers) moved a dimension by 0.25 and about +# eight reviews pinned it at 1.0 (#2623). The bound keys on the row's ts, +# not its session_id: writers log different ids for one review (the +# PostToolUse hook logs the host conversation, the agent the preamble run) +# and an unsubstituted placeholder such as SESSION_ID would merge or split +# reviews. A row without a parseable ts is its own bucket. # The body is a single-quoted bash string: its JS must not contain a single # quote (use double quotes or backticks), or the assignment ends early and # every subcommand fails to parse. @@ -428,7 +429,7 @@ function accumulateSignals(lines, QUESTIONS, SIGNAL_MAP, dims) { } } const contributing = Object.fromEntries(dims.map((d) => [d, 0])); - const sessions = new Map(); + const buckets = new Map(); const rows = []; const skills = new Set(); const qids = new Set(); @@ -448,22 +449,23 @@ function accumulateSignals(lines, QUESTIONS, SIGNAL_MAP, dims) { const deltas = Object.hasOwn(byChoice, e.user_choice) ? byChoice[e.user_choice] : []; if (!Array.isArray(deltas) || deltas.length === 0) continue; signalEvents++; - const real = typeof e.session_id === "string" && e.session_id && !/^[A-Z_]+$/.test(e.session_id); - const session = real ? e.session_id : "row-" + count; - const sums = sessions.get(session) || {}; - sessions.set(session, sums); + const when = e.ts ? new Date(e.ts) : null; + const day = when && !Number.isNaN(when.getTime()) ? when.toISOString().slice(0, 10) : null; + const key = day ? "day " + day : "row " + count; + const bucket = buckets.get(key) || { day: day || "undated row " + count, sums: {} }; + buckets.set(key, bucket); for (const d of deltas) { - sums[d.dim] = (sums[d.dim] || 0) + d.delta; - rows.push({ ts: e.ts, question_id: e.question_id, choice: e.user_choice, session, dim: d.dim, delta: d.delta }); + bucket.sums[d.dim] = (bucket.sums[d.dim] || 0) + d.delta; + rows.push({ ts: e.ts, question_id: e.question_id, choice: e.user_choice, day: bucket.day, dim: d.dim, delta: d.delta }); } for (const dim of new Set(deltas.map((d) => d.dim))) contributing[dim] = (contributing[dim] ?? 0) + 1; } const totals = Object.fromEntries(dims.map((d) => [d, 0])); const capped = []; - for (const [session, sums] of sessions) { + for (const { day, sums } of buckets.values()) { for (const [dim, sum] of Object.entries(sums)) { const bounded = Math.max(-cap, Math.min(cap, sum)); - if (Math.abs(sum - bounded) > 1e-9) capped.push({ session, dim, sum, bounded }); + if (Math.abs(sum - bounded) > 1e-9) capped.push({ day, dim, sum, bounded }); totals[dim] = (totals[dim] ?? 0) + bounded; } } @@ -581,10 +583,10 @@ do_trace() { console.log(' ' + clean(r.ts).slice(0,19) + ' ' + clean(r.question_id) + ' → ' + clean(r.choice) + ' (' + signed(r.delta) + ')'); } for (const c of acc.capped.filter((c) => c.dim === target)) { - console.log(' session ' + clean(c.session) + ': ' + signed(c.sum, 3) + ' capped to ' + signed(c.bounded, 3)); + console.log(' day ' + clean(c.day) + ': ' + signed(c.sum, 3) + ' capped to ' + signed(c.bounded, 3)); } const total = acc.totals[target] ?? 0; - console.log('TOTAL: ' + signed(total, 3) + ' (each session capped at ±' + acc.cap + ') → ' + normalizeToDimensionValue(total).toFixed(3)); + console.log('TOTAL: ' + signed(total, 3) + ' (each UTC day capped at ±' + acc.cap + ') → ' + normalizeToDimensionValue(total).toFixed(3)); } }); " diff --git a/docs/designs/PLAN_TUNING_V0.md b/docs/designs/PLAN_TUNING_V0.md index 25870721f..005948e0e 100644 --- a/docs/designs/PLAN_TUNING_V0.md +++ b/docs/designs/PLAN_TUNING_V0.md @@ -105,7 +105,7 @@ After weighing Codex's argument, we chose to roll back CEO EXPANSION and ship an } ``` -`signal_events` counts the logged answers that moved any dimension, and `contributing_events` counts, per dimension, the logged answers that moved it (an answer that writes two deltas to one dimension counts once), so a dimension with no evidence (0) is visible beside one whose evidence balances out; both print 0.5 (#2623). Each value is the sigmoid of its accumulated deltas, and each session's sum per dimension is clamped to the largest single delta in the signal map (currently ±0.06), so one long review can't pin a dimension; `--trace ` lists every contributing answer, each clamped session and the total. The bound keys on the logged `session_id`, and the two writers use different ids: the PostToolUse hook logs the host's conversation id, and the agent's own log call logs the preamble's run id. An answer logged by both therefore counts once per writer, and a hook-only conversation that runs several reviews shares one bound. A row whose `session_id` is missing, empty or an unsubstituted placeholder such as `SESSION_ID` is its own session. +`signal_events` counts the logged answers that moved any dimension, and `contributing_events` counts, per dimension, the logged answers that moved it (an answer that writes two deltas to one dimension counts once), so a dimension with no evidence (0) is visible beside one whose evidence balances out; both print 0.5 (#2623). Each value is the sigmoid of its accumulated deltas, and each UTC day's sum per dimension (by the row's `ts`) is clamped to the largest single delta in the signal map (currently ±0.06), so one burst of answers can't pin a dimension; `--trace ` lists every contributing answer, each clamped day and the total. The bound keys on the day, not on `session_id`: the PostToolUse hook and the agent log different ids for the same review (the host's conversation id and the preamble's run id), and an agent can leave the `SESSION_ID` placeholder unsubstituted, so no logged id reliably marks one review. The cost is that several reviews on one day count as one. `contributing_events` still counts an answer logged by both writers twice. A row without a parseable `ts` is its own bucket. **Diversity check** (Codex #13): `inferred` is considered "enough data" only when `sample_size >= 20 AND skills_covered >= 3 AND question_ids_covered >= 8 AND days_span >= 7`. Below this, `/plan-tune profile` shows "not enough observed data yet" instead of a potentially-misleading inferred value. diff --git a/test/gstack-developer-profile.test.ts b/test/gstack-developer-profile.test.ts index 338fc6717..3e0d5bb25 100644 --- a/test/gstack-developer-profile.test.ts +++ b/test/gstack-developer-profile.test.ts @@ -363,79 +363,107 @@ describe('gstack-developer-profile --derive', () => { }); // ----------------------------------------------------------------------- -// Per-session bound (#2623): one session moves a dimension at most as far -// as the largest single delta in SIGNAL_MAP. +// Per-day bound (#2623): one UTC day moves a dimension at most as far as the +// largest single delta in SIGNAL_MAP, whatever session_id the writers log. // ----------------------------------------------------------------------- -describe('gstack-developer-profile per-session bound', () => { +describe('gstack-developer-profile per-day bound', () => { const CAP = Math.max(...Object.values(SIGNAL_MAP).flatMap((byChoice) => Object.values(byChoice).flat().map((d) => Math.abs(d.delta)))); - const arch = (session_id: string | undefined, user_choice = 'fix-now', i = 0) => - logQuestion({ skill: 'plan-eng-review', question_id: 'plan-eng-review-arch-finding', question_summary: 'arch?', user_choice, ts: `2026-04-0${i + 1}T10:00:00Z`, ...(session_id ? { session_id } : {}) }); + const arch = (day: number, user_choice = 'fix-now', extra: Record = {}) => + logQuestion({ skill: 'plan-eng-review', question_id: 'plan-eng-review-arch-finding', question_summary: 'arch?', user_choice, ts: `2026-04-${String(day).padStart(2, '0')}T10:00:00Z`, ...extra }); const inferred = () => (readProfile() as { inferred: { values: Record; contributing_events: Record } }).inferred; + const value = (total: number) => +normalizeToDimensionValue(total).toFixed(3); test('the bound is the largest single delta in the signal map', () => { expect(CAP).toBe(0.06); }); - test('five answers in one session move a dimension by the bound, not by their sum', () => { - for (let i = 0; i < 5; i++) arch('one-review', 'fix-now', i); + test('five answers on one day move a dimension by the bound, not by their sum', () => { + for (let i = 0; i < 5; i++) arch(1, 'fix-now', { session_id: 'one-review' }); runDev('--derive'); - expect(inferred().values.architecture_care).toBe(+normalizeToDimensionValue(CAP).toFixed(3)); - expect(inferred().values.risk_tolerance).toBe(+normalizeToDimensionValue(-CAP).toFixed(3)); + expect(inferred().values.architecture_care).toBe(value(CAP)); + expect(inferred().values.risk_tolerance).toBe(value(-CAP)); expect(inferred().contributing_events.architecture_care).toBe(5); }); - test('the same five answers across five sessions are not bounded (negative control)', () => { - for (let i = 0; i < 5; i++) arch(`s${i}`, 'fix-now', i); + test('the same five answers on five days are not bounded (negative control)', () => { + for (let i = 0; i < 5; i++) arch(i + 1, 'fix-now', { session_id: 'one-review' }); runDev('--derive'); - expect(inferred().values.architecture_care).toBe(+normalizeToDimensionValue(5 * 0.05).toFixed(3)); + expect(inferred().values.architecture_care).toBe(value(5 * 0.05)); }); - test('rows without a session_id each count as their own session', () => { - for (let i = 0; i < 3; i++) arch(undefined, 'fix-now', i); + test('a second day still moves the dimension after the first day is capped (negative control)', () => { + for (let i = 0; i < 5; i++) arch(1); + for (let i = 0; i < 5; i++) arch(2); runDev('--derive'); - expect(inferred().values.architecture_care).toBe(+normalizeToDimensionValue(3 * 0.05).toFixed(3)); + expect(inferred().values.architecture_care).toBe(value(2 * CAP)); }); - test('rows with an unsubstituted SESSION_ID placeholder each count as their own session', () => { - for (let i = 0; i < 3; i++) arch('SESSION_ID', 'fix-now', i); + test('distinct session_ids on one day share the day bound', () => { + for (let i = 0; i < 5; i++) arch(1, 'fix-now', { session_id: `s${i}` }); runDev('--derive'); - expect(inferred().values.architecture_care).toBe(+normalizeToDimensionValue(3 * 0.05).toFixed(3)); + expect(inferred().values.architecture_care).toBe(value(CAP)); }); - test('rows with an empty session_id each count as their own session', () => { - for (let i = 0; i < 3; i++) { - logQuestion({ skill: 'plan-eng-review', question_id: 'plan-eng-review-arch-finding', question_summary: 'arch?', user_choice: 'fix-now', session_id: '' }); + test('an unsubstituted SESSION_ID placeholder is bounded per day like any other id', () => { + for (let i = 0; i < 5; i++) arch(1, 'fix-now', { session_id: 'SESSION_ID' }); + for (let i = 0; i < 3; i++) arch(i + 2, 'fix-now', { session_id: 'SESSION_ID' }); + runDev('--derive'); + expect(inferred().values.architecture_care).toBe(value(CAP + 3 * 0.05)); + }); + + test('one review logged by both the hook and the agent is bounded once per day', () => { + for (let i = 0; i < 5; i++) { + arch(1, 'fix-now', { source: 'hook', tool_use_id: `t${i}`, session_id: 'claude-conversation-uuid' }); + arch(1, 'fix-now', { session_id: '1234-1700000000-ab' }); } runDev('--derive'); - expect(inferred().values.architecture_care).toBe(+normalizeToDimensionValue(3 * 0.05).toFixed(3)); + expect(inferred().values.architecture_care).toBe(value(CAP)); + expect(inferred().contributing_events.architecture_care).toBe(10); }); - test('answers in one session net out before the bound applies', () => { - arch('mixed', 'fix-now', 0); - arch('mixed', 'accept-risk', 1); + test('the day is the UTC day of ts', () => { + arch(1, 'fix-now'); + logQuestion({ skill: 'plan-eng-review', question_id: 'plan-eng-review-arch-finding', question_summary: 'arch?', user_choice: 'fix-now', ts: '2026-04-01T20:00:00-08:00' }); runDev('--derive'); - expect(inferred().values.architecture_care).toBe(+normalizeToDimensionValue(0.05 - 0.04).toFixed(3)); + expect(inferred().values.architecture_care).toBe(value(2 * 0.05)); }); - test('--trace shows the capped session and a total that matches --derive', () => { - for (let i = 0; i < 5; i++) arch('one-review', 'fix-now', i); - arch('later', 'defer', 6); + test('rows without a parseable ts are each their own bucket', () => { + for (let i = 0; i < 3; i++) { + logQuestion({ skill: 'plan-eng-review', question_id: 'plan-eng-review-arch-finding', question_summary: 'arch?', user_choice: 'fix-now', ts: 'not-a-date' }); + } + runDev('--derive'); + expect(inferred().values.architecture_care).toBe(value(3 * 0.05)); + }); + + test('answers on one day net out before the bound applies', () => { + arch(1, 'fix-now'); + arch(1, 'accept-risk'); + runDev('--derive'); + expect(inferred().values.architecture_care).toBe(value(0.05 - 0.04)); + }); + + test('--trace shows the capped day and a total that matches --derive', () => { + for (let i = 0; i < 5; i++) arch(1); + arch(7, 'defer'); runDev('--derive'); const r = runDev('--trace', 'architecture_care'); expect(r.stdout).toContain('6 events for architecture_care'); - expect(r.stdout).toContain('session one-review: +0.250 capped to +0.060'); - expect(r.stdout).not.toContain('session later'); - expect(r.stdout).toContain(`TOTAL: +0.040 (each session capped at ±${CAP}) → ${inferred().values.architecture_care.toFixed(3)}`); + expect(r.stdout).toContain('day 2026-04-01: +0.250 capped to +0.060'); + expect(r.stdout).not.toContain('day 2026-04-07'); + expect(r.stdout).toContain(`TOTAL: +0.040 (each UTC day capped at ±${CAP}) → ${inferred().values.architecture_care.toFixed(3)}`); const risk = runDev('--trace', 'risk_tolerance'); - expect(risk.stdout).toContain('session one-review: -0.100 capped to -0.060'); + expect(risk.stdout).toContain('day 2026-04-01: -0.100 capped to -0.060'); }); test('--trace replaces control characters in logged fields', () => { - for (let i = 0; i < 5; i++) arch('a\nTOTAL: +9.999 \u001b[31mforged\u007f\u009b', 'fix-now', i); + for (let i = 0; i < 2; i++) { + logQuestion({ skill: 'plan-eng-review', question_id: 'plan-eng-review-arch-finding', question_summary: 'arch?', user_choice: 'fix-now', ts: '\u001b\u007f\u009b\nTOTAL: +9.999' }); + } const r = runDev('--trace', 'architecture_care'); expect(r.stdout.split('\n').filter((l) => l.startsWith('TOTAL:'))).toHaveLength(1); - expect(r.stdout).toContain('session a?TOTAL: +9.999 ?[31mforged??: +0.250 capped to +0.060'); + expect(r.stdout).toContain(' ????TOTAL: +9.999 plan-eng-review-arch-finding'); expect(/[\x00-\x08\x0b-\x1f\x7f-\x9f]/.test(r.stdout)).toBe(false); }); });