diff --git a/CHANGELOG.md b/CHANGELOG.md index 7997df744..86bdf2a81 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,7 +5,7 @@ **/plan-tune's inferred profile no longer pins at 1.0 after a few reviews, and it now records 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 JSON (`--profile`) also records `contributing_events` per dimension, so a dimension with no evidence reads 0 there instead of a bare 0.5; /plan-tune's display and `--check-mismatch` don't read it yet (#3436). ### Fixed -- **`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. A `ts` without a zone is read as UTC, and rows without a parseable string `ts` share one undated bucket, so a junk timestamp can't escape the bound. `diversity.days_span` counts the same UTC days. 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`**: `--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. Only an ISO 8601 date or date-time `ts` counts as dated, and a time without a zone is read as UTC; every other row shares one undated bucket, so a junk timestamp can't escape the bound. `diversity.days_span` counts the same UTC days. 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 @@ -15,7 +15,7 @@ - `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 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` even when the machine's zone is west of UTC, and a `ts` without a zone is read as UTC. `--trace` prints the clamped days and a total that matches `--derive`, and replaces control characters. A `constructor` choice and a `null` line are ignored. Rows with an unparseable or non-string `ts` share one undated bucket. Negative controls: the same answers on separate days, a second day after a capped first day, 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, and local-day bucketing fails the UTC 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` even when the machine's zone is west of UTC, and a `ts` without a zone, `T`- or space-separated, is read as UTC. `--trace` prints the clamped days and a total that matches `--derive`, and replaces control characters. A `constructor` choice, a `null` line and array-valued `question_id` or `user_choice` rows are ignored. Rows whose `ts` is not ISO 8601 (`TS`, `junk 1`, a number) share one undated bucket and add no days to `days_span`. Negative controls: the same answers on separate days, a second day after a capped first day, 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, and local-day bucketing fails the UTC 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 c44c2a086..aaf791813 100755 --- a/bin/gstack-developer-profile +++ b/bin/gstack-developer-profile @@ -415,9 +415,10 @@ do_gap() { # 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 ts without a zone is read as UTC, and every row without a -# parseable string ts shares one undated bucket, so junk timestamps cannot -# escape the bound. diversity.days_span counts the same days. +# reviews. Only an ISO 8601 date or date-time counts as dated; a time +# without a zone is read as UTC. Every other row shares one undated bucket, +# so junk timestamps cannot escape the bound (Date alone parses strings +# like "junk 1" as real days). diversity.days_span counts the same days. # 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. @@ -445,8 +446,8 @@ function accumulateSignals(lines, QUESTIONS, SIGNAL_MAP, dims) { count++; skills.add(e.skill); qids.add(e.question_id); - const iso = typeof e.ts === "string" && /^\d{4}-\d\d-\d\dT\d\d:\d\d(:\d\d(\.\d+)?)?$/.test(e.ts) ? e.ts + "Z" : e.ts; - const when = typeof iso === "string" ? new Date(iso) : null; + const m = typeof e.ts === "string" ? /^(\d{4}-\d\d-\d\d)(?:[Tt ](\d\d:\d\d(?::\d\d(?:\.\d+)?)?)(Z|z|[+-]\d\d:\d\d)?)?$/.exec(e.ts) : null; + const when = m ? new Date(m[2] ? m[1] + "T" + m[2] + (m[3] || "Z").toUpperCase() : m[1]) : null; const day = when && !Number.isNaN(when.getTime()) ? when.toISOString().slice(0, 10) : null; if (day) days.add(day); const def = Object.hasOwn(QUESTIONS, e.question_id) ? QUESTIONS[e.question_id] : undefined; diff --git a/docs/designs/PLAN_TUNING_V0.md b/docs/designs/PLAN_TUNING_V0.md index ca69c9433..e9bf2b440 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 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 `ts` without a zone is read as UTC, rows without a parseable string `ts` share one undated bucket, and `diversity.days_span` counts the same UTC days. Nothing displays `contributing_events` yet (#3436). +`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. Only an ISO 8601 date or date-time `ts` counts as dated (a time without a zone is read as UTC), every other row shares one undated bucket, and `diversity.days_span` counts the same UTC days. Nothing displays `contributing_events` yet (#3436). **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 9ce89c35c..11685bb64 100644 --- a/test/gstack-developer-profile.test.ts +++ b/test/gstack-developer-profile.test.ts @@ -351,11 +351,15 @@ describe('gstack-developer-profile --derive', () => { const projects = path.join(tmpHome, 'projects'); for (const slug of fs.readdirSync(projects)) { const log = path.join(projects, slug, 'question-log.jsonl'); - if (fs.existsSync(log)) fs.appendFileSync(log, 'null\n'); + if (!fs.existsSync(log)) continue; + fs.appendFileSync(log, 'null\n'); + fs.appendFileSync(log, JSON.stringify({ skill: 'plan-eng-review', question_id: ['plan-eng-review-arch-finding'], user_choice: 'fix-now' }) + '\n'); + fs.appendFileSync(log, JSON.stringify({ skill: 'plan-eng-review', question_id: 'plan-eng-review-arch-finding', user_choice: ['fix-now'] }) + '\n'); } const r = runDev('--derive'); expect(r.status).toBe(0); - const p = readProfile() as { inferred: { contributing_events: Record; signal_events: number } }; + const p = readProfile() as { inferred: { contributing_events: Record; signal_events: number; sample_size: number } }; + expect(p.inferred.sample_size).toBe(2); expect(p.inferred.signal_events).toBe(1); expect(p.inferred.contributing_events.architecture_care).toBe(1); expect(runDev('--trace', 'architecture_care').stdout).toContain('1 events for architecture_care'); @@ -439,6 +443,7 @@ describe('gstack-developer-profile per-day bound', () => { atTs('2026-04-01T20:00:00-08:00'); deriveWestOfUtc(); expect(inferred().values.architecture_care).toBe(value(2 * 0.05)); + expect((readProfile() as { inferred: { diversity: { days_span: number } } }).inferred.diversity.days_span).toBe(2); }); test('a ts without a zone is read as UTC', () => { @@ -449,11 +454,23 @@ describe('gstack-developer-profile per-day bound', () => { expect((readProfile() as { inferred: { diversity: { days_span: number } } }).inferred.diversity.days_span).toBe(2); }); - test('rows without a parseable string ts share one undated bucket', () => { - for (const ts of ['not-a-date', 'TS', 1775000000]) atTs(ts); + test('a ts with a space separator and no zone is read as UTC', () => { + atTs('2026-04-01 23:30:00'); + atTs('2026-04-02T00:30:00Z'); + deriveWestOfUtc(); + expect(inferred().values.architecture_care).toBe(value(2 * 0.05)); + expect((readProfile() as { inferred: { diversity: { days_span: number } } }).inferred.diversity.days_span).toBe(2); + }); + + test('rows without an ISO 8601 string ts share one undated bucket', () => { + const junk: unknown[] = ['not-a-date', 'TS', 'junk 1', 'foo 12', 1775000000]; + for (const ts of junk) expect(atTs(ts)).toBe(0); runDev('--derive'); + const p = readProfile() as { inferred: { sample_size: number; contributing_events: Record; diversity: { days_span: number } } }; + expect(p.inferred.sample_size).toBe(junk.length); + expect(p.inferred.contributing_events.architecture_care).toBe(junk.length); expect(inferred().values.architecture_care).toBe(value(CAP)); - expect((readProfile() as { inferred: { diversity: { days_span: number } } }).inferred.diversity.days_span).toBe(0); + expect(p.inferred.diversity.days_span).toBe(0); }); test('answers on one day net out before the bound applies', () => {