fix: pre-landing review fixes (pass 2)

A session_id that is an unsubstituted placeholder such as SESSION_ID
counts as its own session instead of collapsing every agent-logged
answer into one bound; a log line that parses to null is skipped; the
accumulation tolerates a dimension it did not seed; the trace test covers
DEL and C1 controls; docs and CHANGELOG wording match the code.

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

CHANGELOG.md

@@ -2,11 +2,11 @@
 
 ## [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 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 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.
 
 ### 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 without a `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`, 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 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.
 
 ### 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.
@@ -15,7 +15,7 @@
 - `docs/designs/PLAN_TUNING_V0.md` documents `signal_events`, `contributing_events` and the per-session bound in the profile schema.
 
 ### 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 is ignored. Negative controls: the same five answers in five sessions, rows with a missing or empty `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 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.
 - 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

bin/gstack-developer-profile

@@ -412,7 +412,8 @@ do_gap() {
 # 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 is its own session. The bound keys on the writer's session_id,
+# 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.
 # 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
@@ -437,7 +438,7 @@ function accumulateSignals(lines, QUESTIONS, SIGNAL_MAP, dims) {
   for (const line of lines) {
     let e;
     try { e = JSON.parse(line); } catch { continue; }
-    if (!e.question_id || !e.user_choice) continue;
+    if (!e || typeof e !== "object" || !e.question_id || !e.user_choice) continue;
     count++;
     skills.add(e.skill);
     qids.add(e.question_id);
@@ -447,14 +448,15 @@ 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 session = typeof e.session_id === "string" && e.session_id ? e.session_id : "row-" + count;
+    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);
     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 });
     }
-    for (const dim of new Set(deltas.map((d) => d.dim))) contributing[dim]++;
+    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 = [];
@@ -462,7 +464,7 @@ function accumulateSignals(lines, QUESTIONS, SIGNAL_MAP, dims) {
     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 });
-      totals[dim] += bounded;
+      totals[dim] = (totals[dim] ?? 0) + bounded;
     }
   }
   return { cap, totals, contributing, rows, capped, count, signalEvents, skills, qids, days };

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 unique answers that moved it, 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 <dim>` 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.
+`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 <dim>` 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.
 
 **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.
 

test/gstack-developer-profile.test.ts

@@ -345,9 +345,14 @@ describe('gstack-developer-profile --derive', () => {
     });
   });
 
-  test('a choice named after an Object.prototype member is ignored, not fatal', () => {
+  test('a choice named after an Object.prototype member or a non-object line is ignored, not fatal', () => {
     logQuestion({ skill: 'plan-eng-review', question_id: 'plan-eng-review-arch-finding', question_summary: 'arch?', user_choice: 'constructor', session_id: 's1' });
     logQuestion({ skill: 'plan-eng-review', question_id: 'plan-eng-review-arch-finding', question_summary: 'arch?', user_choice: 'fix-now', session_id: 's2' });
+    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');
+    }
     const r = runDev('--derive');
     expect(r.status).toBe(0);
     const p = readProfile() as { inferred: { contributing_events: Record<string, number>; signal_events: number } };
@@ -392,6 +397,12 @@ describe('gstack-developer-profile per-session bound', () => {
     expect(inferred().values.architecture_care).toBe(+normalizeToDimensionValue(3 * 0.05).toFixed(3));
   });
 
+  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);
+    runDev('--derive');
+    expect(inferred().values.architecture_care).toBe(+normalizeToDimensionValue(3 * 0.05).toFixed(3));
+  });
+
   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: '' });
@@ -421,11 +432,11 @@ describe('gstack-developer-profile per-session bound', () => {
   });
 
   test('--trace replaces control characters in logged fields', () => {
-    for (let i = 0; i < 5; i++) arch('a\nTOTAL: +9.999 \u001b[31mforged', 'fix-now', i);
+    for (let i = 0; i < 5; i++) arch('a\nTOTAL: +9.999 \u001b[31mforged\u007f\u009b', 'fix-now', i);
     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(/[\x00-\x08\x0b-\x1f\x7f]/.test(r.stdout)).toBe(false);
+    expect(r.stdout).toContain('session a?TOTAL: +9.999 ?[31mforged??: +0.250 capped to +0.060');
+    expect(/[\x00-\x08\x0b-\x1f\x7f-\x9f]/.test(r.stdout)).toBe(false);
   });
 });