Skip to content

fix(mt#4961): Refresh stale conversation titles on growth, title handoffs from the work - #3724

Merged
edobry merged 4 commits into
mainfrom
task/mt-4961
Sep 11, 2026
Merged

edobry merged 4 commits into
mainfrom
task/mt-4961

Conversation

@minsky-ai

@minsky-ai minsky-ai Bot commented Sep 11, 2026 •

Copy link
Copy Markdown
Contributor

Summary

A conversation's generated title (mt#3321) was minted once from its opening turns and never revisited, so a long-running conversation drifted from its own title — the originating incident: titled "Handoff on mt#4956 task claim" at 2 turns, unchanged 163 turns and two implementation cycles later. This PR (mt#4961) adds two things: (1) a growth-crossing REFRESH so a titled conversation that has grown materially is re-titled, and (2) handoff-aware prompting so a conversation resumed via an ADR-046 succession claim (handoff mt#N) is titled after the work package, not the act of claiming it.

Key Changes

SC1 — refresh on growth (packages/domain/src/transcripts/title-pipeline.ts)

  • titleCandidateConditions() is restructured from a two-clause AND into ONE OR of two branches: the existing untitled branch (unchanged semantics), and a new titled-and-grown-enough branch. candidateConditionCount() now reports 1 in normal mode (was 2).
  • New exported TITLE_REFRESH_GROWTH_FACTOR = 5, grounded in the 2026-09-11 corpus measurement (live regime, 503 conversations titled within 30 min of their first turn): growth p50 5.75x, p75 11.8x, p90 21.7x, 266/503 grew ≥5x. Both turn counts (now vs. at-titling) are derived from agent_transcript_turns at query time — no new column.
  • Terminates by construction: every attempt (success, or a no-subject re-ask via recordSkip) unconditionally re-stamps title_attempted_at, which re-bases the comparison. A refreshed row cannot re-enter the candidate set until it grows another full 5x past the count at the refresh — the same invariant mt#4179 established for the untitled branch, applied to the titled one.
  • R1 (reviewer round 1): ingest-since-titling pre-filter. The refresh branch now also requires last_ingested_jsonl_timestamp > title_attempted_at, placed BEFORE the two correlated count subqueries. A conversation with no ingest since its title was stamped cannot have grown, and a refresh re-stamps title_attempted_at, so this is a cheap short-circuit, not a behavior change to which rows ultimately qualify. See Execution evidence below for the measured cost.

SC1 window (title-generator.ts) — new selectRefreshTitleTurns(headTurns, tailTurns): the first 4 substantive turns plus 12 substantive turns from the last TURN_SCAN_LIMIT (36) turns, de-duplicated by turn index and re-sorted into conversation order. TitlePipeline.titleRow() takes an isRefresh flag; a refresh loads both a head window (loadTurns, unchanged) and a new tail window (loadTailTurns, DESC-ordered then reversed). generateTitle() no longer re-applies the MAX_TURNS cap internally (only filters empty-content turns) — re-imposing it would truncate a refresh's wider window back to a first-time titling's size. run() distinguishes a first titling (titled) from a replaced title (refreshed) by whether the row's title was already non-null going in. R1: generateTitle now enforces a separate hard cap, MAX_PROMPT_TURNS = TITLE_REFRESH_HEAD_TURNS + MAX_TURNS = 16, so a caller that bypasses selection entirely and hands it a raw, unbounded turn array still cannot blow the prompt budget — a well-behaved caller (one that pre-selected via selectTitleTurns/selectRefreshTitleTurns) is never truncated by it, since neither selection ever produces more than 16.

SC2 — writes only Minsky's columns — the update set is exactly title/titleAttemptedAt/titleSkipReason (asserted directly against applied[0]'s keys). No Minsky-side custom-title concept exists to protect (custom-title is a Claude Code JSONL record type dropped at ingest, per the spec's planning-pass correction) — nothing in this PR touches that path.

SC3 — handoff-resumed conversations (title-generator.ts) — TitleGenerator takes an optional third constructor arg, WorkPackageLookup: (taskId) => Promise<{title, members} | null>. When the first human turn matches ^\s*\/?(?:handoff|action\s+handoff)\s+mt#(\d+) or a minsky://task/mt%23(\d+) link and the lookup resolves, its title and member list are prefixed onto the prompt with an instruction to title the WORK, not the claim. The generator stays pure (no DB import); title-pipeline.ts's new lookupWorkPackage() supplies it from tasksTable (checking kind === "work-package") and workPackageMembersTable.

SC4 — single definition holds — scripts/smoke-conversation-titles.ts gains --refresh-only, a CLIENT-SIDE filter on the already-fetched rows (title !== null); the query is still exactly and(...titleCandidateConditions()). The existing structural test (smoke script IMPORTS the filter rather than restating it) still passes.

SC5 — observable — TitlePipelineRunResult.refreshed flows into ConversationTitleSweepDeps.runTitling()'s return type and the sweeper's per-tick log.info payload (src/cockpit/sweepers.ts). /api/sweeps was checked directly (src/cockpit/routes/sweeps.ts) and does NOT render per-sweep domain counts at all — it exposes only generic scheduling liveness (name/cadence/lastAttemptAt/consecutiveFailures/etc.), the same as it already does for the pre-existing titled/skipped fields. No code change was needed or made there; this is a finding, not a gap I introduced (see also the R1 non-blocking note below — the reviewer flagged this same observation and it is acknowledged out of scope for this task). The conversation overview route (routes/conversations.ts) now selects and returns titleAttemptedAt, folded into the response cache's version token, and the conversation detail header (ConversationPage.tsx) renders "titled <relative time>" beside the title using the existing relativeTime helper (lib/format.ts). List-hover is out of scope per the spec.

Testing

Unit tests only locally (bun); no live DB/model access in this session. ConversationOverviewPanel.test.tsx picks up the new required titleAttemptedAt field on the shared payload type (ancillary, type-compatibility only — that panel never reads title fields).

Reviewer round 1 (0 blocking, 6 non-blocking) — dispositions

The reviewer APPROVED with 0 blocking findings and 6 non-blocking notes. Two were acted on (above, tagged R1); the other four are acknowledged and left out of this task's scope:

  1. Prompt-bloat bound on the work-package member list (SC3) — a package with a very large member count could inflate the prompt. Noted; not addressed here. No task filed yet for a member-count cap.
  2. Dry-run window in scripts/smoke-conversation-titles.ts — the --refresh-only preview doesn't wire the handoff WorkPackageLookup into its own directly-constructed TitleGenerator (only TitlePipeline's --execute path gets it). Noted; out of scope for SC4 as written (which asked only for the --refresh-only filter).
  3. /api/sweeps carries no per-sweep domain counts (titled/refreshed/skipped) — confirmed finding, pre-existing for every domain sweep, not introduced or fixed by this PR. Noted; a dedicated task would need to design what domain-count exposure on that route should look like generally, not just for titling.
  4. Inline handoff claims — extractHandoffTaskId's regex only matches a claim at the START of the first human turn; a claim mentioned mid-message is not detected. Noted; matches the ADR-046 succession pattern this task targets (the claim IS the opening message), and a broader inline-detection pass is out of scope.

Execution evidence:

AT1 (candidate selection) and SC1 — packages/domain/src/transcripts/title-pipeline.test.ts, describe "refresh on growth (mt#4961)":

$ bun test --preload ./tests/setup.ts --timeout=15000 packages/domain/src/transcripts/title-pipeline.test.ts packages/domain/src/transcripts/title-generator.test.ts
bun test v1.3.14 (0d9b296a)
🔇 Global test setup: Logger and console mocked to prevent output during tests

 61 pass
 0 fail
 149 expect() calls
Ran 61 tests across 2 files. [199.00ms]

Includes: titled-at-21/now-105 is a candidate; titled-at-21/now-104 is not (one short of the factor); a never-attempted titled row is not refresh-eligible; a growth-crossing refresh replaces the title and counts as refreshed not titled; AT2 — a refresh whose model finds no subject keeps the prior title and stamps the attempt; SC2 — the update set for a successful refresh is exactly title/titleAttemptedAt/titleSkipReason; TITLE_REFRESH_GROWTH_FACTOR is 5; a real-SQL rendering test (PgDialect().sqlToQuery) asserting the rendered WHERE text carries the growth comparison; and (R1, new) "the ingest-since-titling guard precedes the count subqueries", asserting the new last_ingested_jsonl_timestamp > title_attempted_at guard text appears in branch 2 BEFORE the turns_now subquery. R1 measured cost (coordinating agent, prod EXPLAIN (ANALYZE, BUFFERS), ~1,900 titled rows): without the guard, a LIMIT 25 pass took 4,928 ms (474 rows scanned to find 25, ~20 ms cold per row for the turns_now subplan, ~2.6 ms for turns_at_titling); with the guard, a full pass (no LIMIT) took 231 ms, evaluated the subplans for only 446 rows, and found 281 candidates, matching the ~280 corpus estimate.

R1, new — title-generator.test.ts, "caps the turns actually sent to the model at 16 (4 + 12) even when the caller bypasses selection": 30 raw turns passed directly to generateTitle produce exactly 16 Operator: lines in the prompt, keeping the opening (turn 0 present) and dropping everything past the cap (turn 16 absent). Included in the 61-pass run above.

AT3 (handoff-aware prompt) — title-generator.test.ts, describe "handoff-resumed conversations (mt#4961, SC3)" and "selectRefreshTitleTurns": first turn handoff mt#4956 prefixes the package title and both members; the slash-prefixed harness form and a minsky://task/ link also match; a non-package task id (lookup returns null), no lookup wired, and a non-matching first turn all produce no prefix. selectRefreshTitleTurns tests cover the head+tail combination, order, and de-duplication on a short conversation where the windows overlap. All included in the 61-pass run above.

SC5 wiring — src/cockpit/conversation-summary-sweep.test.ts:

$ bun test --preload ./tests/setup.ts --timeout=15000 src/cockpit/conversation-summary-sweep.test.ts
 8 pass
 0 fail
 9 expect() calls
Ran 8 tests across 1 file. [268.00ms]

Header render — src/cockpit/web/pages/ConversationPage.chrome.test.tsx (happy-dom):

$ bun test --preload ./tests/dom-setup.ts --preload ./tests/setup.ts --timeout=15000 --path-ignore-patterns='services/**' --path-ignore-patterns='src/cockpit/web/dist/**' ./src/cockpit/web/pages/ConversationPage.chrome.test.tsx
 6 pass
 0 fail
 20 expect() calls
Ran 6 tests across 1 file. [474.00ms]

Related tests (bun scripts/run-related-tests.ts) over every changed production file: 738 + 6 pass, 0 fail, across 41 related test files (pre-R1 run; R1's two changes are confined to the same two domain files and re-verified directly above), including server-conversation-overview.test.ts and the pre-existing ConversationPage.test.tsx, sweepers.test.ts, routes/sweeps.test.ts — no regressions.

AT4, AT5: live runs recorded under ## Live verification by the coordinating agent after creation.

Negative control — refresh candidate: reverted titleCandidateConditions()'s second branch entirely (full revert of the SC1 clause) and re-ran the SQL-rendering test (the one test in this suite that reads the REAL query rather than a fake DB's own reimplementation of the predicate — see note below). Observed FAILING:

error: expect(received).toContain(expected)
Expected to contain: "\"agent_transcripts\".\"title\" IS NOT NULL"
Received: "(\n      (\n        \"agent_transcripts\".\"title\" IS NULL\n        AND (\"agent_transcripts\".\"title_attempted_at\" IS NULL\n             OR \"agent_transcripts\".\"last_ingested_jsonl_timestamp\" > \"agent_transcripts\".\"title_attempted_at\"\n             OR (\"agent_transcripts\".\"title_skip_reason\" = 'no-turns' AND EXISTS (\n                   SELECT 1 FROM \"agent_transcript_turns\" turns_probe\n                    WHERE turns_probe.agent_session_id = \"agent_transcripts\".\"agent_session_id\"\n                      AND (turns_probe.user_text IS NOT NULL\n                           OR turns_probe.assistant_text IS NOT NULL))))\n      )\n    )"
(fail) TitlePipeline > refresh on growth (mt#4961) > the rendered SQL carries the refresh branch's growth comparison [1.59ms]
 0 pass
 26 filtered out
 1 fail

Change fully reverted immediately after and full suite re-confirmed green. Note on test design: the fake-DB-backed candidate tests (isCandidate() in makeDb) model the predicate's intended BEHAVIOR in JS and do not read the real WHERE clause at all — reverting the production SQL does not fail them, which is why the SQL-rendering tests (via PgDialect().sqlToQuery) are the actual falsifiable controls for this clause, not the AT1 fake-DB tests. Both kinds of test are kept: the fake-DB tests pin the pipeline's counting/branching behavior; the SQL-rendering tests pin the query text and (as of R1) its clause ORDER.

Rendered refresh SQL

Full titleCandidateConditions() output via new PgDialect().sqlToQuery(condition), updated for R1 to include the ingest-since-titling guard:

(
      (
        "agent_transcripts"."title" IS NULL
        AND ("agent_transcripts"."title_attempted_at" IS NULL
             OR "agent_transcripts"."last_ingested_jsonl_timestamp" > "agent_transcripts"."title_attempted_at"
             OR ("agent_transcripts"."title_skip_reason" = 'no-turns' AND EXISTS (
                   SELECT 1 FROM "agent_transcript_turns" turns_probe
                    WHERE turns_probe.agent_session_id = "agent_transcripts"."agent_session_id"
                      AND (turns_probe.user_text IS NOT NULL
                           OR turns_probe.assistant_text IS NOT NULL))))
      )
      OR
      (
        "agent_transcripts"."title" IS NOT NULL
        AND "agent_transcripts"."title_attempted_at" IS NOT NULL
        AND "agent_transcripts"."last_ingested_jsonl_timestamp" > "agent_transcripts"."title_attempted_at"
        AND (SELECT count(*) FROM "agent_transcript_turns" turns_now
              WHERE turns_now.agent_session_id = "agent_transcripts"."agent_session_id")
            >= $1 * GREATEST(
              (SELECT count(*) FROM "agent_transcript_turns" turns_at_titling
                WHERE turns_at_titling.agent_session_id = "agent_transcripts"."agent_session_id"
                  AND turns_at_titling.started_at <= "agent_transcripts"."title_attempted_at"),
              1)
      )
    )

$1 = 5 (TITLE_REFRESH_GROWTH_FACTOR, unchanged by R1). The added line is AND "agent_transcripts"."last_ingested_jsonl_timestamp" > "agent_transcripts"."title_attempted_at", placed directly after the two IS NOT NULL checks and before the turns_now count subquery, so Postgres's left-to-right AND short-circuiting skips both correlated subqueries for a row that fails it.

Measured against prod by the coordinating agent (EXPLAIN (ANALYZE, BUFFERS)) — this is what motivated R1's fix, not a claim made without evidence: pre-guard, a LIMIT 25 pass took 4,928 ms (474 rows scanned, each paying ~20 ms cold for turns_now and ~2.6 ms for turns_at_titling); post-guard, a full pass (no LIMIT) took 231 ms, evaluated the subplans for only 446 of ~1,900 titled rows, and found 281 candidates — matching the ~280 corpus estimate in TITLE_REFRESH_GROWTH_FACTOR's docblock.

Terminating-candidate invariant (mt#4179) preserved: a refreshed row cannot re-enter the candidate set until it grows another full TITLE_REFRESH_GROWTH_FACTORx (5x) past the turn count AT THE REFRESH, because every attempt — success or a no-subject re-ask — unconditionally re-stamps title_attempted_at, which is the only state the comparison reads. The R1 ingest guard does not weaken this: it is an additional necessary condition (a row failing it was never a candidate to begin with, since no ingest means no growth), not a replacement for the growth arithmetic.

Deploy verification:

packages/domain and src/cockpit are deploy surface. Confirmed via the sanctioned predicate over every changed file:

true packages/domain/src/transcripts/title-pipeline.ts
true packages/domain/src/transcripts/title-pipeline.test.ts
true packages/domain/src/transcripts/title-generator.ts
true packages/domain/src/transcripts/title-generator.test.ts
true src/cockpit/sweepers.ts
true src/cockpit/conversation-summary-sweep.test.ts
false scripts/smoke-conversation-titles.ts
true src/cockpit/routes/conversations.ts
true src/cockpit/web/widgets/RunDetail.tsx
true src/cockpit/web/pages/ConversationPage.tsx
true src/cockpit/web/pages/ConversationPage.chrome.test.tsx
true src/cockpit/web/widgets/ConversationOverviewPanel.test.tsx

No [no-deploy-impact] tag is used. R1's changes are confined to title-pipeline.ts/title-pipeline.test.ts and title-generator.ts/title-generator.test.ts, already covered above. The coordinating agent runs deployment_wait-for-latest with notBefore and expectCommitSha after merge and asserts the /health body's service identity; local cockpit restarted for AT4.

Live verification

(empty — AT4/AT5 are live runs performed by the coordinating agent after merge with prod credentials, per the dispatch instructions. The R1 EXPLAIN ANALYZE measurements above were run by the coordinating agent directly against prod during review, ahead of merge.)

What I could not do

  • AT4/AT5 (live re-title of f290bb69-… on the running cockpit, and the smoke script's live model call) — no prod DB or Anthropic credentials in this session; deferred to the coordinating agent per the dispatch instructions.
  • The --refresh-only smoke-script preview does not wire the handoff WorkPackageLookup into its own directly-constructed TitleGenerator (only TitlePipeline's --execute path gets it, since TitlePipeline's constructor wires the lookup) — the dry-run preview branch was out of scope for that wiring per the task's SC4 description, which only asked for the --refresh-only filter. (Reviewer round 1 non-blocking note build(deps-dev): bump @typescript-eslint/eslint-plugin from 7.18.0 to 8.32.0 #2, acknowledged above.)

🤖 Generated with Claude Code

https://claude.ai/code/session_01UtrzXnxTKigMWeQzigNfUZ

edobry and others added 3 commits September 11, 2026 00:02
…doff-resumed conversations from the work

titleCandidateConditions() (title-pipeline.ts) gains a second branch, OR-ed
against the existing untitled branch: a row that already has a title
re-enters the candidate set once its total turn count has grown at least
TITLE_REFRESH_GROWTH_FACTOR (5x) since title_attempted_at, both counts
derived from agent_transcript_turns with no new column. The comparison
terminates because every attempt (success or a no-subject re-ask via
recordSkip) re-stamps title_attempted_at, re-basing the growth measure.

TitlePipeline.run() now distinguishes a first titling (titled) from a
replaced title (refreshed) via TitlePipelineRunResult, and titleRow() takes
an isRefresh flag: a refresh loads both the conversation's opening (head)
and its last TURN_SCAN_LIMIT turns (tail) and calls the new
selectRefreshTitleTurns() (title-generator.ts) — 4 substantive head turns
plus 12 substantive tail turns, de-duplicated and re-sorted into
conversation order.

TitleGenerator also gains handoff-aware prompting (SC3): when the first
human turn matches a succession-claim shape (`handoff mt#N`, `action
handoff ...`, or a minsky://task/ link) and an injected WorkPackageLookup
resolves the claimed task to a work-package-kind task, its title and
member list are prefixed onto the prompt so the model titles the WORK, not
the claim. The generator stays pure (no DB import); TitlePipeline supplies
the lookup from the tasks/work-package-members tables.

generateTitle() no longer re-applies the MAX_TURNS cap internally (it now
only filters empty-content turns) — the cap belongs to whichever selection
the caller already made, and re-imposing it would truncate a refresh's
wider head+tail window back down to a first-time titling's size.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UtrzXnxTKigMWeQzigNfUZ
…andidates in the smoke script

TitlePipelineRunResult's new `refreshed` field is threaded through the
cockpit title sweeper (src/cockpit/sweepers.ts) into its per-tick log line,
and a wiring test confirms a tick accepts and completes on a result
carrying it (conversation-summary-sweep.test.ts). `/api/sweeps`
(routes/sweeps.ts) was checked directly: it exposes only generic
scheduling liveness (name/cadence/lastAttemptAt/etc.), never per-sweep
domain counts, so no change is needed there for `refreshed` to become
visible — it already isn't for `titled`/`skipped` either.

The conversation overview route (routes/conversations.ts) now selects and
returns `titleAttemptedAt`, folded into the response cache's version token
so a refresh invalidates a cached overview immediately. The conversation
detail header (ConversationPage.tsx) renders "titled <relative time>"
beside the title using the existing `relativeTime` helper, sourced through
RunDetail.tsx's shared `ConversationOverviewPayload` type.

scripts/smoke-conversation-titles.ts gains a `--refresh-only` flag that
filters the dry-run preview's already-fetched rows to `title IS NOT NULL`
client-side; the query itself is still exactly
`and(...titleCandidateConditions())`, so there is no restated WHERE to
drift.

ConversationOverviewPanel.test.tsx picks up the new required
`titleAttemptedAt` field on the shared payload type (set to null in its
fixture; the panel itself never reads title fields).

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UtrzXnxTKigMWeQzigNfUZ
@minsky-ai minsky-ai Bot added the authorship/co-authored Co-authored by human and AI agent label Sep 11, 2026
@minsky-reviewer

minsky-reviewer Bot commented Sep 11, 2026 •

Copy link
Copy Markdown

Minsky Reviewer Status

Verdict: APPROVED — no blocking findings
Review: View review
Model: openai/gpt-5 | Tokens: 762K prompt, 11K completion | Duration: 176s
Mode: normal

Commands

  • /review — request a fresh review

@minsky-reviewer minsky-reviewer Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Independent adversarial review (Chinese-wall)
Reviewer: minsky-reviewer[bot] via openai:gpt-5
Tier: 2


Solid, well-scoped change: adds a growth-based refresh path with terminating semantics, wires a handoff-aware prompt via an injected lookup, preserves the single-definition filter, and surfaces observability (refreshed count in logs and titled-at in the header). Tests are thorough, including a real-SQL rendering assertion to pin the refresh branch.

Non-blocking notes to consider: (1) the refresh subquery likely needs an index on (agent_session_id, started_at) to avoid per-candidate scans; (2) the work-package prefix can bloat prompts with large member lists — consider bounds; (3) the smoke script’s --refresh-only mode still previews the opening-only window, which under-represents refresh behavior; (4) the handoff regex misses inline claims — clarify intent or relax; (5) generateTitle no longer enforces MAX_TURNS, so unseen callers could pass large windows; and (6) /api/sweeps lacks per-sweep domain counts (not required, but it limits external observability). One spec divergence: SC2 in the original spec mentions excluding custom titles; the PR amends it to "writes only Minsky’s columns" — update the spec accordingly to avoid confusion.

With these addressed or tracked, this is ready to merge.

Findings

  • [NON-BLOCKING] packages/domain/src/transcripts/title-pipeline.ts:145 — Refresh growth-count subquery likely needs an index on (agent_session_id, started_at) to avoid full scans
    titleCandidateConditions()'s refresh branch counts turns at/after titling using two correlated count(*) subqueries over agent_transcript_turns, one of which filters on turns_at_titling.started_at <= title_attempted_at:
(SELECT count(*) FROM agent_transcript_turns turns_at_titling
 WHERE turns_at_titling.agent_session_id = agent_transcripts.agent_session_id
   AND turns_at_titling.started_at <= agent_transcripts.title_attempted_at)

agent_transcript_turns has a PK on (agent_session_id, turn_index) (see packages/domain/src/storage/schemas/agent-transcript-turns-schema.ts), but no composite index that begins with agent_session_id and includes started_at. That means the started_at <= … predicate cannot be satisfied by the existing PK index ordering and is likely to devolve into an index-only scan over all turns for the session with a filter on started_at, or worse a heap scan depending on visibility. At scale (long sessions), this can become a per-candidate O(N) operation.

Suggestion: add a supporting index for this query shape, e.g. CREATE INDEX CONCURRENTLY idx_turns_session_started_at ON agent_transcript_turns(agent_session_id, started_at) (or adjust to your migration conventions), or rephrase the comparison to use turn_index (which is indexed) if it can be proven monotonic with started_at. Absent a new index, please attach EXPLAIN (ANALYZE, BUFFERS) evidence that the current plan stays bounded on representative production conversations.

  • [NON-BLOCKING] packages/domain/src/transcripts/title-generator.ts:333 — Work-package prefix can grow unbounded with large member lists, risking prompt bloat
    buildWorkPackagePrefix() renders every member with - <id>: <title> and the prefix is prepended verbatim to the model prompt. There is no cap or truncation on members.length or on title length. Large work packages could substantially bloat the prompt (and cost), and the title model is explicitly the "cheap" tier.

Consider bounding the number of members (e.g., first 10 with a "+N more" note) and/or truncating long member titles to a fixed length to keep the prompt size predictable. Add a test to lock the bound.

  • [NON-BLOCKING] scripts/smoke-conversation-titles.ts:139 — Dry-run preview does not exercise the refresh window (head+tail) used in production refreshes
    For --refresh-only, the smoke script previews refresh candidates correctly (client-side filter), but it still generates titles using selectTitleTurns (opening-only) rather than selectRefreshTitleTurns (opening + recent tail) that the production refresh path uses. This can under-represent the refresh's actual behavior in the dry-run when a conversation drifted from its opening.

If the intent is to preview the prompt the refresh would use, consider switching to selectRefreshTitleTurns for --refresh-only mode, or printing both variants so operators can see the delta. The PR body notes this was out-of-scope; surfacing here as a gap in observability rather than a functional defect.

  • [NON-BLOCKING] src/cockpit/sweepers.ts:2609 — /api/sweeps does not surface per-sweep domain counts; refreshed is only visible in logs
    SC5 adds refreshed to the title sweep result and it is logged (log.info), but /api/sweeps continues to expose only scheduling liveness. This matches the route's documented scope (checked in src/cockpit/routes/sweeps.ts), but makes it harder to verify refresh activity remotely without log access.

Consider adding optional per-tick domain counters to a separate diagnostics route or to the conversation routes that now carry titleAttemptedAt, so operators can observe refresh activity without tailing logs. Non-blocking because the spec's amended SC5 doesn’t require /api/sweeps to carry counts.

  • [NON-BLOCKING] packages/domain/src/transcripts/title-generator.ts:286 — Handoff task-id extractor only matches start-of-string claims; legitimate inline claims will be missed
    extractHandoffTaskId() requires the first turn to match ^\s*\/?(?:handoff|action\s+handoff)\s+mt#(\d+) or a minsky://task/mt%23(\d+) link. If a user writes a natural-language sentence that includes handoff mt#N mid-string (e.g., "Can we handoff mt#4956 now?"), the current regex won’t match and the work-package prefix will not be added.

If inline claims are in scope for ADR-046 handoffs, consider relaxing the regex to find a claim anywhere on the line (with appropriate false-positive guards). If start-of-line is intentional, add a brief comment to state that only canonical slash-command style is supported.

  • [NON-BLOCKING] packages/domain/src/transcripts/title-generator.ts:362 — generateTitle now ignores MAX_TURNS entirely; callsites that bypass selection can send very large windows
    generateTitle() used to apply selectTitleTurns (which enforces MAX_TURNS). It now trusts the caller’s selection and only filters out non-substantive turns. Tests pass visible windows in, and TitlePipeline uses selectTitleTurns/selectRefreshTitleTurns, but external callsites (e.g., future scripts/tests) can accidentally pass unbounded arrays, inflating cost. The doc comment calls this out but there’s no runtime guard.

Consider an optional safety cap (e.g., MAX_TURNS * 2) or asserting in dev/test builds that inputs are pre-selected, to prevent regressions where unseen callers inadvertently bypass the cap.

Spec verification

Criterion Status Evidence
SC1 — refresh on growth. Met packages/domain/src/transcripts/title-pipeline.ts:129-170 defines TITLE_REFRESH_GROWTH_FACTOR = 5 and adds an OR-branch in titleCandidateConditions() comparing total turn count vs. GREATEST(count_at_titling,1). title-generator.ts:211-260 adds selectRefreshTitleTurns (head+tail), and TitlePipeline.titleRow() uses it when isRefresh is true (title-pipeline.ts:354-373). Tests at title-pipeline.test.ts:494-559 cover growth-crossing, exact factor, and no-subject behavior; SQL rendering test ensures branch presence.
SC2 — custom titles are never overwritten. Not Met The amended planning states there is no Minsky-side custom-title to protect and reframes SC2 to "writes only Minsky's columns." The implemented code adheres to the amended SC2 (update set is exactly title, titleAttemptedAt, titleSkipReason — asserted at title-pipeline.test.ts:531-545). However, the original Success Criteria requires excluding rows with a custom-title path from refresh candidate selection. No such exclusion exists in titleCandidateConditions() and no custom-title column exists. This is a spec/implementation divergence. Either update the task spec to reflect the amended SC2 (as in the PR body) or add an exclusion once a custom-title path exists.
SC3 — handoff-resumed conversations are titled from the work. Met packages/domain/src/transcripts/title-generator.ts:286-331 adds extractHandoffTaskId and work-package prefixing via injected WorkPackageLookup; title-pipeline.ts:49-89, 301-309 wires lookupWorkPackage() from DB and injects it into TitleGenerator. Tests at title-generator.test.ts:308-395 cover slash form, bare form, and minsky:// links; null lookup and non-matching cases produce no prefix.
SC4 — single definition holds. Met titleCandidateConditions() remains the single exported builder (title-pipeline.ts:146-170, 214-270) and scripts/smoke-conversation-titles.ts:114-141 imports it rather than restating; --refresh-only is a client-side filter after fetching title. TitlePipeline.candidateConditionCount() updated to 1 in normal mode and has tests at title-pipeline.test.ts:346-368.
SC5 — observable. Met TitlePipelineRunResult gains refreshed (title-pipeline.ts:105-123), sweepers.ts:2609-2618 logs refreshed, and routes/conversations.ts:412-433,693-711 returns titleAttemptedAt in overview, which ConversationPage.tsx:275-294 renders as "titled ". Tests at conversation-summary-sweep.test.ts:133-174 exercise refreshed in a sweep tick, and ConversationPage.chrome.test.tsx:233-253 asserts the header render.

Documentation impact

  • no-update-needed — This PR changes internal selection logic and prompt construction, adds a new refreshed counter to sweep logs, and surfaces titleAttemptedAt in the conversation detail UI. There are no repo docs describing title-refresh behavior or /api/sweeps payload details to invalidate; src/cockpit/routes/sweeps.ts intentionally omits domain counts. No user-facing CLI or API contract changed beyond adding a nullable field to conversation overview, which is self-describing in the UI and covered by tests. I did not find docs referencing conversation title refresh or the conversation detail payload.

@minsky-reviewer minsky-reviewer Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Independent adversarial review (Chinese-wall)
Reviewer: minsky-reviewer[bot] via openai:gpt-5
Tier: 2


Verification round: prior iteration had no BLOCKING findings. I re-checked the amended spec against the current diff. SC1 (growth-based refresh) is implemented via an added OR-branch in titleCandidateConditions(), with supporting head+tail window selection and tests including a real SQL rendering assertion. SC2 (writes only Minsky’s columns) is honored; the update set is exactly the three title fields. SC3 (handoff-aware prompt) is wired with an injected WorkPackageLookup and covered by tests. SC4 (single definition) holds — the smoke script imports the shared condition and adds only a client-side refresh-only filter. SC5 observability is met: refreshed is plumbed through the sweeper and titleAttemptedAt renders in the header.

I found no new critical defects introduced by this commit. The non-blocking observations from the prior round (indexing, prompt size bounds, smoke preview window, regex scope, optional guard on generateTitle, sweeps route liveness-only) remain advisory and do not block merge. Ready to proceed.

Spec verification

Criterion Status Evidence
SC1 — refresh on growth. Met packages/domain/src/transcripts/title-pipeline.ts:169-216, 251-270, 111-168 — adds TITLE_REFRESH_GROWTH_FACTOR=5 and an OR-branch in titleCandidateConditions() comparing total turn count vs. GREATEST(count_at_titling,1); packages/domain/src/transcripts/title-generator.ts:195-211, 238-260 — adds selectRefreshTitleTurns (head+tail) and supporting types; packages/domain/src/transcripts/title-pipeline.ts:374-423 — TitlePipeline.titleRow() uses selectRefreshTitleTurns when isRefresh is true. Tests in packages/domain/src/transcripts/title-pipeline.test.ts:494-559 and SQL rendering at :551-559 assert the growth branch is present.
SC2 — custom titles are never overwritten. Met Per the amended Success Criteria (Planning pass), SC2 reframed to 'the refresh writes only Minsky's columns.' The update set for a successful refresh is asserted to be exactly title/titleAttemptedAt/titleSkipReason at packages/domain/src/transcripts/title-pipeline.test.ts:531-545, and recordSkip never writes title (title-pipeline.ts:486-516). No harness custom-title path is touched anywhere.
SC3 — handoff-resumed conversations are titled from the work. Met packages/domain/src/transcripts/title-generator.ts:286-391 — introduces extractHandoffTaskId, buildWorkPackagePrefix, and WorkPackageLookup injection; packages/domain/src/transcripts/title-pipeline.ts:44-90, 308-316 — wires lookupWorkPackage(db, taskId) and injects into TitleGenerator; tests at packages/domain/src/transcripts/title-generator.test.ts:308-401 cover slash/bare/link forms and null lookup fallback.
SC4 — single definition holds. Met titleCandidateConditions() remains the single exported selector (packages/domain/src/transcripts/title-pipeline.ts:200-216); TitlePipeline.candidateConditionCount() updated to reflect a single composed condition (packages/domain/src/transcripts/title-pipeline.ts:318-327) with tests at packages/domain/src/transcripts/title-pipeline.test.ts:346-368. The smoke script imports titleCandidateConditions and applies only a client-side --refresh-only filter (scripts/smoke-conversation-titles.ts:144-167).
SC5 — observable. Met TitlePipelineRunResult adds refreshed and sweeper logs it (packages/domain/src/transcripts/title-pipeline.ts:133-147, src/cockpit/sweepers.ts:2609-2618); route adds titleAttemptedAt to the conversation detail payload (src/cockpit/routes/conversations.ts:412-433, 693-711) and the header renders 'titled ' (src/cockpit/web/pages/ConversationPage.tsx:275-294) with tests at src/cockpit/web/pages/ConversationPage.chrome.test.tsx:233-253.

Adoption sweep

Symbol Kind Consumers found Classification Notes
packages/domain/src/transcripts/title-generator.selectRefreshTitleTurns function packages/domain/src/transcripts/title-pipeline.ts:401 — selects head+tail turns for refresh, packages/domain/src/transcripts/title-generator.test.ts:261 — unit tests for window selection Adopted New selection function for refresh windows is used by TitlePipeline and covered by tests; no further wiring required.

Documentation impact

  • no-update-needed — Behavioral changes are internal to the title pipeline and sweeper wiring, with UI-only addition of a 'titled ' line in the conversation header. No repo docs describe refresh behavior or the sweeper counters; routes/sweeps intentionally omits domain counts. I checked src/cockpit/routes/conversations.ts and related UI; no external API contract documented elsewhere is invalidated.

…titling; cap generateTitle's window

Reviewer measured the refresh branch's rendered SQL against prod: a full
pass evaluated the two correlated count subqueries for every titled row
(~1,900), costing 4,928ms for a LIMIT-25 pass. Add
`last_ingested_jsonl_timestamp > title_attempted_at` to the refresh branch,
placed before both subqueries so Postgres's left-to-right AND
short-circuiting skips them for a row that cannot have grown (no ingest
since titling means no new turns). Post-guard: 231ms for a full pass,
subplans evaluated for 446 of ~1,900 rows, 281 candidates found (matches
the ~280 corpus estimate). TITLE_REFRESH_GROWTH_FACTOR and the semantics of
which rows ultimately qualify are unchanged.

generateTitle() now enforces MAX_PROMPT_TURNS (TITLE_REFRESH_HEAD_TURNS +
MAX_TURNS = 16) as a hard backstop on the turns actually sent to the model,
so a caller that bypasses selectTitleTurns/selectRefreshTitleTurns entirely
cannot send an unbounded window; a well-behaved caller is never truncated
by it since neither selection function ever produces more than 16.

Added a SQL-rendering test asserting the guard precedes the count
subqueries, and a generateTitle test asserting the 16-turn cap.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UtrzXnxTKigMWeQzigNfUZ
@github-actions

Copy link
Copy Markdown

Cockpit Preview

Preview URL: https://cockpit-preview-production.up.railway.app

Deployed from commit 59a89e884.
Preview runs in read-only mode — mutations are disabled.

Updated by Cockpit Preview.

@minsky-reviewer minsky-reviewer Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Independent adversarial review (Chinese-wall)
Reviewer: minsky-reviewer[bot] via openai:gpt-5
Tier: 2


The R1 feedback was addressed: the refresh-branch ingest guard now precedes the correlated subqueries and generateTitle enforces a hard 16-turn cap, both covered by targeted tests. SC1–SC5 in the amended spec are met: the candidate set adds a titled-and-grown branch with factor 5 and short-circuit; refresh windows combine head+tail correctly; only title/titleAttemptedAt/titleSkipReason are written; handoff-aware prompting is injected and wired; single-definition discipline holds and the smoke script imports the predicate; observability adds refreshed and the overview includes titleAttemptedAt. I found no blocking issues introduced by the fix. One minor, non-blocking note: handoff detection reads only the very first selected turn’s user text; scanning for the first human turn would align even more tightly with SC3. Overall, the change is sound and ready to merge.

Findings

  • [NON-BLOCKING] packages/domain/src/transcripts/title-generator.ts:318 — Handoff detection checks only the very first turn’s user text; consider scanning for the first human turn
    extractHandoffTaskId is consulted via buildHandoffPrefix with firstHumanText taken from turns[0]?.userText in generateTitle() (packages/domain/src/transcripts/title-generator.ts:318-336). If the first selected entry has assistant-only content (or a system/meta turn) and the handoff claim is on the next human turn, the prefix will not be applied even though the conversation is a valid ADR‑046 handoff. The spec targets "the first human turn"; a more robust (still cheap) approach is to scan the provided turns for the earliest element where userText has visible content and test that string. This is advisory and can be deferred — the common case (first human turn is the first substantive turn) still works.

Documentation impact

  • no-update-needed — Behavioral changes are internal (candidate selection logic, result counters, and prompt-shaping) and surfaces already wired in code: /api/sweeps intentionally does not expose per-sweep domain counts; conversation detail header renders titleAttemptedAt and is covered by tests. No standalone docs in repo reference the titling refresh semantics, and route contracts remain unchanged (only added a field in the conversations overview payload). I checked src/cockpit/routes/conversations.ts and src/cockpit/routes/sweeps.ts to ensure public API shapes and descriptions remain consistent.

@edobry
edobry merged commit 1c4eba4 into main Sep 11, 2026
23 checks passed
@edobry
edobry deleted the task/mt-4961 branch September 11, 2026 04:39

@minsky-reviewer minsky-reviewer Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Independent adversarial review (Chinese-wall)
Reviewer: minsky-reviewer[bot] via openai:gpt-5
Tier: 2


Verification R2: The R1 non-blocking requests were addressed — the ingest-since-titling guard now precedes the count subqueries in the refresh branch, and generateTitle() enforces a hard 16‑turn cap to bound callers that bypass selection. SC1–SC5 of the amended spec are met with comprehensive unit tests and correct wiring through the sweeper and conversation detail header. I found no new blocking defects introduced by these fixes. One minor, non-blocking note remains: handoff detection reads only the first provided turn’s userText; scanning for the earliest human turn in the window would align even more closely with SC3. Overall, this is solid and ready to merge.

Findings

  • [NON-BLOCKING] packages/domain/src/transcripts/title-generator.ts:330 — Handoff detection examines only the very first provided turn’s user text
    In TitleGenerator.generateTitle() the succession-claim check uses turns[0]?.userText (mt#4961 SC3). If the first selected element is an assistant/system/meta turn and the first human claim appears on the next turn, the work‑package prefix will not be applied even though the conversation is a valid ADR‑046 handoff. This matches the R3 note and is intentionally out of scope here, but consider scanning the provided window for the earliest turn with userText content to align more tightly with “first human turn.”

Spec verification

Criterion Status Evidence
SC1 — refresh on growth. A titled conversation re-enters the candidate set when it has grown materially since titling: turn count at least 5× what it was at title_attempted_at OR at least 4 hours of ingested activity after it, whichever comes first (thresholds grounded in this incident: titled at ~2 turns, drifted by ~80 and by ~7 hours; tune against the corpus before READY, do not keep round numbers by default). The re-generation reads a bounded, recent-weighted window rather than only the opening turns, and replaces the title only when the model returns a subject; a no-subject re-ask keeps the existing title. Cost bound: at most one regeneration per growth crossing, so a conversation is re-titled a handful of times over its life, never per tick. Met packages/domain/src/transcripts/title-pipeline.ts:150-199 defines TITLE_REFRESH_GROWTH_FACTOR=5 and implements a titled-and-grown refresh branch in titleCandidateConditions() with derived turn counts; packages/domain/src/transcripts/title-pipeline.ts:413-460 loads head+tail windows and uses selectRefreshTitleTurns for recent-weighted window; packages/domain/src/transcripts/title-generator.ts:207-259 adds selectRefreshTitleTurns() and TITLE_REFRESH_HEAD_TURNS=4; packages/domain/src/transcripts/title-pipeline.ts:471-492 writes new title only when model returns a subject, otherwise recordSkip keeps prior title. Tests asserting factor=5 and SQL shape at packages/domain/src/transcripts/title-pipeline.test.ts:632-706.
SC2 — custom titles are never overwritten. A row carrying a custom title (mt#4621's path, custom-title) is excluded from refresh; the exclusion is part of titleCandidateConditions() so the smoke script previews it too. N/A The amended spec’s Premise corrections state there is no Minsky-side custom title to protect; SC2 was rewritten to “writes only Minsky’s columns.” The implementation aligns with the amended SC2 below and does not reference external custom-title records.
SC3 — handoff-resumed conversations are titled from the work. When the first human turn matches the succession claim shape (handoff mt#N, action handoff …, or a minsky://task/ link to a work-package task), the generator's prompt window includes that package's title and member list, so the first title names the work rather than the claim. Met packages/domain/src/transcripts/title-generator.ts:239-306 adds WorkPackageLookup, extractHandoffTaskId(), buildWorkPackagePrefix(), and prefixes the prompt in generateTitle() (lines ~325-353) when the first selected turn matches; packages/domain/src/transcripts/title-pipeline.ts:38-83 implements lookupWorkPackage() and injects it into TitleGenerator (lines 234-241). Tests in packages/domain/src/transcripts/title-generator.test.ts:314-413 cover all three detection forms and fallbacks.
SC4 — single definition holds. titleCandidateConditions() stays the one place the candidate set is defined; scripts/smoke-conversation-titles.ts previews the refresh candidates with no restated filter; TitlePipeline.candidateConditionCount is updated with the new clauses. Met packages/domain/src/transcripts/title-pipeline.ts:202-238 exports titleCandidateConditions() as the single query; scripts/smoke-conversation-titles.ts:146-168 imports that definition and applies only a client-side --refresh-only filter without restating WHERE; packages/domain/src/transcripts/title-pipeline.ts:246-257 updates candidateConditionCount() semantics (now 1 in normal mode). Tests in packages/domain/src/transcripts/title-pipeline.test.ts:382-397 assert candidateConditionCount() reports 1.
SC5 — observable. The title sweep's result counts gain refreshed (regenerated titles) beside candidates/titled/skipped, and the cockpit's conversation list shows the title's titleAttemptedAt on hover or in the overview so a stale-looking title can be dated. Met src/cockpit/sweepers.ts:2517-2524 and 2609-2618 accept and log refreshed; src/cockpit/conversation-summary-sweep.test.ts:131-172 adds a sweeper tick test with refreshed; src/cockpit/routes/conversations.ts:74-85 and 412-433, 693-710 include titleAttemptedAt in payload and cache token; src/cockpit/web/pages/ConversationPage.tsx:283-296 renders “titled ” when present, with tests in src/cockpit/web/pages/ConversationPage.chrome.test.tsx:239-258.

Documentation impact

  • no-update-needed — This PR changes internal selection logic and adds a refreshed count plus a titleAttemptedAt field surfaced in existing cockpit routes/components. I checked src/cockpit/routes/conversations.ts (payload comment documents the new field) and src/cockpit/routes/sweeps.ts (no per-domain counts are exposed by design). There are no standalone docs describing title refresh semantics to update, and existing route contracts remain backward compatible (only additive).

edobry added a commit that referenced this pull request Sep 13, 2026
…versation date

## Summary

`pre-narration-detector` (log-only) suppresses a claimed PR outcome only on evidence it can read from the parent transcript's tool NAMES and tool INPUTS. That leaves the orchestrator pattern unsuppressible: an implementer subagent creates the PR in ITS transcript, its report reaches the parent, and the parent's "PR #3723 is up for mt#4959" fires. Measured over the detector's whole post-mt#4810 corpus (2026-08-30 → 09-13, 265 records): **12 unsuppressed, 12 false** — 7 relays of a subagent-created PR, 3 claims dated before their conversation began, one relay of a subagent's research report, one undated catch-up-table entry.

**One correction to the spec's mechanism, found at planning and folded into the criteria:** the subagent's report is NOT the `Agent` tool's `tool_result`. For a background agent that result is the launch stub (`Async agent launched successfully…` — all 10 in `f290bb69`); the report arrives as a **string-content user line** carrying a `<task-notification>` envelope (`<summary>Agent "…" finished</summary>` … `<result>PR created: **#3723** …</result>`). Implementing SC1 as originally written would have suppressed 0 of 7. Replaying the 7 relays through the detector's own `windowSlice`: 7 of 7 have such a line naming the PR in window, 0 have it in any `tool_result`.

## Key changes

- `.minsky/hooks/pre-narration-detector.ts`
  - **`relayed-subagent-report`** (fourth source): the claim's PR number appears PR-shaped (`PR #N`, `PR N`, `#N`, `pull/N`, `changeset/N`; a lone `#N` may not follow a word character — `\w`, underscore included after R1 — so `mt#4959` / `mem#1386` / `foo_#123` never read as PRs) in a subagent report within the window — (a) a `<task-notification>` user line whose `<summary>` opens `Agent`, or (b) the `tool_result` of an `Agent` / `SendMessage` / `TaskOutput` call. `Background command …` / `MCP task …` envelopes are excluded (the parent's own command coming back). Identity-scoped: a report naming the WRONG number is the pre-narration this detector exists for. Applies to any category whose claim names a PR.
  - **`dated-historical`** (fifth source, `merged` family only): the claim's own sentence carries an ISO date strictly before the day of the conversation's first timestamped line. Anchored to the conversation, not to the presence of a clock time — the coarser signal mt#4256 measured and rejected as "would suppress genuine current claims that happen to cite when they were observed"; a claim dated inside the conversation's span keeps firing. `(?!\d)` rather than a trailing `\b` after the day, because `2026-09-02T21:19:45Z` (1 of the 3 measured) is followed by `T`. "The conversation" is the PARENT transcript on both call paths (`ctx.transcriptLines` via `resolveParentTranscriptLines`; `main()` via `resolveParentTranscriptLinesForPath`).
  - Both sources come LAST in the chain, so the three older reasons report exactly what they did.
  - `buildSuppressionEvidence(lines, windowTurns)` assembles every input once — `run()`, `main()` and the replay script all call it, which is what makes "the replay evaluates on the detector's own terms" hold by construction. `detectPreNarrationWithSuppression` gains an optional 4th parameter (`RelayEvidence`); omitting it disables both new sources (the safe direction). `#3412`'s two `console.error` lines in `main()` are untouched.
- `scripts/diagnose-pre-narration-window.ts` — `replayCurrentDetector` routes through the shared builder; new `--reasons` mode replays EVERY record in the window (suppressed ones included) and tallies reasons by record (comparable to the log's `suppressionReasons`) and by match (the invariant).
- `.minsky/hooks/pre-narration-detector.test.ts` — 18 tests; `.claude/hooks/pre-narration-detector.ts` regenerated by pre-commit.

## Testing

Typecheck: 8 projects clean (`validatedWorkspace` = the session dir). Lint: 0 errors / 0 warnings over 4490 files (re-run after R1).

Execution evidence:

- **AT1** (`bun test --preload ./tests/setup.ts --timeout=15000 ./.minsky/hooks/pre-narration-detector.test.ts ./.minsky/hooks/suppression-contract.test.ts ./scripts/diagnose-pre-narration-window.test.ts`, at head `75abc4be3`):
  ```
  115 pass
   0 fail
  Ran 115 tests across 3 files. [161.00ms]
  ```
  The fixture is the `f290bb69` envelope verbatim in shape (ids shortened), no `session_pr_create` call, final turn "PR #3723 is up for mt#4959 …" → suppressed under `relayed-subagent-report`. Negative controls: (i) the envelope naming #3724 → fires; (ii) the same number inside a `Background command "Wait for CI build on PR 3723" completed` notification → fires. Second positive: the report as a foreground `Agent` `tool_result` → suppressed under the same reason. Also: a relayed `merged` claim (the `acd0c445` steelman shape) suppresses; a same-turn tool call still outranks a report; a claim naming no PR is never report-backed; omitting the relay evidence disables both sources; `extractPrShapedNumbers` reads every PR spelling and no task/memory short id, and (R1) `foo_#123` / `bar9#456` are not PRs while `(#123)` / `foo #456` are; report evidence is window-scoped.
- **AT2**: "PR #34861 merged **2026-07-21** — one second before the issue closed." in a conversation whose first timestamped line is 2026-09-10 → suppressed under `dated-historical`; the same sentence with the date removed → fires; the same sentence dated on the conversation's first day → fires; "PR #3581 merged 2026-09-02T21:19:45Z." in a conversation begun 2026-07-01 → fires (later than start); a dated `pr-created` claim → fires (merged family only); no timestamped line → exclusion off; the datetime form is asserted directly in the helper test.
- **AT3 / SC3 / SC4 — paired replay** (`bun scripts/diagnose-pre-narration-window.ts --reasons --until 2026-09-13T12:45:00Z --log <main checkout's log>`; the BEFORE side ran the identical harness with the detector restored to `origin/main` and the two old inputs; the AFTER side re-run at `75abc4be3`, unchanged by R1):
  ```
  === BEFORE (detector at origin/main):
  today's detector says: 265 records, 12 unsuppressed, 0 unreplayable
    by MATCH:   270 same-turn-tool-call   53 window-tool-call   3 identity-scoped-tool-call
  === AFTER (this branch):
  today's detector says: 265 records, 1 unsuppressed, 0 unreplayable
    by RECORD:  203 same-turn-tool-call   53 window-tool-call   8 relayed-subagent-report   3 identity-scoped-tool-call   3 dated-historical
    by MATCH:   270 same-turn-tool-call   53 window-tool-call   8 relayed-subagent-report   3 identity-scoped-tool-call   3 dated-historical
  ```
  Both sides reproduce the log's own record-level tally exactly (`log says: 265 / 12 unsuppressed / 202 / 53 / 3`), so the replay is faithful. **SC3: 12 → 1** — the survivor is the `d9c7c2b0` catch-up-table entry ("PR #3715 merged" inside a time-stamped table, undated, backed only by Bash reads), as the spec predicts. **SC4: by MATCH the three older reasons are equal, 270 / 53 / 3.** The record-level `same-turn-tool-call` 202 → 203 is the `#251` record: its turn also carried a same-turn-suppressed `review-approved` match, which becomes visible in the record's reason set once its live `merged` match is suppressed — the spec's SC4 was amended to state the invariant at match level and name this instance.
- **SC1 / SC2**: the two reason strings are distinct constants with their own docblocks; `relayed-subagent-report` reaches 8 (7 relays + the steelman), `dated-historical` 3 (#3581, #251, #34861).

Negative control: the new test file run against the un-fixed detector (`git restore --source=origin/main -- .minsky/hooks/pre-narration-detector.ts`, whole file):
```
SyntaxError: Export named 'SUPPRESSION_DATED_HISTORICAL' not found in module '…/.minsky/hooks/pre-narration-detector.ts'.
 0 pass
 1 fail
```
The discriminating control is the BEFORE half of the paired replay above: the same `--reasons` harness, fed the old detector's two inputs, leaves all 12 unsuppressed.

Negative control — R1 underscore: the pinning test against the pre-R1 detector (`git restore --source=96a3c2bbe -- .minsky/hooks/pre-narration-detector.ts`, whole file, then restored):
```
(fail) mt#5109 — report evidence helpers > PR #3750 R1: an underscore is a word character too — `foo_#123` is not a PR
 95 pass
 1 fail
```

## Live verification

The paired replay IS the live exercise: 265 real calibration records replayed against their real session transcripts through the real-wired `buildSuppressionEvidence` → `detectPreNarrationWithSuppression` path that `run()` calls. No mock, no fixture, 0 unreplayable.

Deploy verification: `[no-deploy-impact]` — `isDeploySurfaceFile` is `false` for all four changed files (`.minsky/hooks/pre-narration-detector.ts`, its test, the `.claude/hooks` mirror, `scripts/diagnose-pre-narration-window.ts`); `findAffectedServices` → `[]`. The detector is a Claude Code hook; it takes effect at the next hook run on a checkout that has the merge.

Documentation impact: the detector's mechanism is documented in its own docblocks (each new reason constant, the two evidence builders, the chain ordering). `hook-observers.mdc`'s Pre-narration entry is an index line (trigger + status + override, "no detail page yet — owned by mt#4992") and enumerates no suppression sources, so it is unchanged.

## Review rounds

**R1** (review 5190779986, on `96a3c2bbe`): BLOCKING — the lone-`#N` lookbehind was `[A-Za-z0-9]`, omitting `_`. Verified true against the docblock's own intent; fixed to `\w` with a pinning test observed failing pre-fix. Non-blocking 1 (lookbehind support): kept; hooks run under Bun/JSC, which supports it, and three sibling hooks already use lookbehind — noted in the docblock. Non-blocking 2 (`conversationStartDay` on "flattened parent+subagent lines"): the premise does not hold — `ctx.transcriptLines` is resolved by `resolveParentTranscriptLines`, which re-parses the PARENT alone whenever subagent candidates exist, and `main()` uses `resolveParentTranscriptLinesForPath`; documented in the function's docblock rather than narrowed.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

https://claude.ai/code/session_01VuuiE8iQYtmaajWTbJmhzK

Co-Authored-By: minsky-ai[bot] <minsky-ai[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

authorship/co-authored Co-authored by human and AI agent

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant