diff --git a/CHANGELOG.md b/CHANGELOG.md index f70fda2..eb19101 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,13 @@ Short, dated summary of notable fixes and changes. For the full "why," see `PHASE2-SETUP.md` (Crisp triage design) or the linked PRs. +## 2026-10-14 — Crisp triage: no re-investigation on support replies; dedupe skips quiet threads + +- **Stale check re-arms only on a new customer message.** A support reply re-investigated `session_3931e910` three times, and the two runs that finished found `no_defect`. The reopen path is unchanged. +- **Dedupe skips threads with no customer message in 14 days.** `session_f9e20b9b` was resolved Aug 14, was `unresolved` again by Sep 2 for a reason the API doesn't show, and was matched to an open issue in October. +- **Staleness now comes from real message timestamps.** `active.last` is missing on email threads, so the old `active.last ?? created_at` counted age since creation. +- Not tested against live Crisp. After merge, check the `stale ...h` log lines and the dedupe step summary's new "Skipped" count. + ## 2026-10-09 — PR build zip: optional "Try this PR in a live test site" link For QA and for support reproducing issues: the PR build comment can now include a link that opens a fresh WordPress (Playground, in the browser) with that build installed, logged in as admin. No install, nothing to clean up. diff --git a/scripts/crisp-activity.mjs b/scripts/crisp-activity.mjs new file mode 100644 index 0000000..a34c53d --- /dev/null +++ b/scripts/crisp-activity.mjs @@ -0,0 +1,45 @@ +// Pure helpers for judging real conversation activity from fetched Crisp messages. +// Customer messages drive re-investigation; `active.last` is often absent and +// `updated_at` is bumped by our own notes, so neither proves real activity. + +// crisp-dedupe-active.mjs skips threads whose newest customer message is older than this. +export const DEDUPE_MAX_QUIET_DAYS = 14; + +const DAY_MS = 24 * 60 * 60 * 1000; + +export function isCustomerText(message) { + return message.type === "text" && message.from === "user"; +} + +// Newest customer-or-agent text. Notes (including ours) and events are excluded. +export function newestTextTimestamp(messages, floor = 0) { + return messages.reduce((max, m) => (m.type === "text" ? Math.max(max, m.timestamp ?? 0) : max), floor); +} + +// Newest customer-written message; agent replies, notes and events never count. +export function newestCustomerTextTimestamp(messages, floor = 0) { + return messages.reduce((max, m) => (isCustomerText(m) ? Math.max(max, m.timestamp ?? 0) : max), floor); +} + +// Newest metadata timestamp, as a hint for whether to fetch messages at all. +// Callers must re-check against real messages before acting. +export function conversationActivityHint(conversation) { + return Math.max(conversation.active?.last ?? 0, conversation.updated_at ?? 0, conversation.created_at ?? 0); +} + +// True if the customer wrote nothing within `maxQuietDays` (no customer text counts as quiet). +export function isCustomerQuiet(messages, now = Date.now(), maxQuietDays = DEDUPE_MAX_QUIET_DAYS) { + const newest = newestCustomerTextTimestamp(messages); + return newest === 0 || now - newest > maxQuietDays * DAY_MS; +} + +// Reclassify only if never checked, or the customer wrote after the last check. +export function shouldReclassifyStale(messages, checkedThroughAt) { + if (checkedThroughAt === undefined) return true; + return newestCustomerTextTimestamp(messages) > checkedThroughAt; +} + +// Marker to store after a stale check; never moves backwards past existing entries. +export function nextCheckedThroughAt(messages, checkedThroughAt) { + return newestCustomerTextTimestamp(messages, checkedThroughAt ?? 0); +} diff --git a/scripts/crisp-activity.test.mjs b/scripts/crisp-activity.test.mjs new file mode 100644 index 0000000..fe34a4e --- /dev/null +++ b/scripts/crisp-activity.test.mjs @@ -0,0 +1,108 @@ +import { test } from "node:test"; +import assert from "node:assert/strict"; +import { + DEDUPE_MAX_QUIET_DAYS, + isCustomerText, + newestTextTimestamp, + newestCustomerTextTimestamp, + conversationActivityHint, + isCustomerQuiet, + shouldReclassifyStale, + nextCheckedThroughAt, +} from "./crisp-activity.mjs"; + +const DAY = 24 * 60 * 60 * 1000; +const NOW = Date.parse("2026-10-14T00:00:00Z"); +const at = (iso) => Date.parse(iso); + +// Mirrors the real shape of session_3931e910 (types/roles/timestamps only). +const msgs = [ + { type: "text", from: "user", timestamp: at("2026-10-09T09:53:17Z") }, + { type: "note", from: "operator", timestamp: at("2026-10-09T15:38:02Z") }, + { type: "text", from: "operator", timestamp: at("2026-10-13T03:32:13Z") }, + { type: "note", from: "operator", timestamp: at("2026-10-13T12:08:29Z") }, +]; + +test("isCustomerText: only text from the user", () => { + assert.equal(isCustomerText({ type: "text", from: "user" }), true); + assert.equal(isCustomerText({ type: "text", from: "operator" }), false); + assert.equal(isCustomerText({ type: "note", from: "user" }), false); + assert.equal(isCustomerText({ type: "file", from: "user" }), false); +}); + +test("newestTextTimestamp counts agent text but not notes/events", () => { + assert.equal(newestTextTimestamp(msgs), at("2026-10-13T03:32:13Z")); + assert.equal(newestTextTimestamp([{ type: "event", from: "operator", timestamp: 999 }]), 0); +}); + +test("newestCustomerTextTimestamp ignores agent replies and notes", () => { + assert.equal(newestCustomerTextTimestamp(msgs), at("2026-10-09T09:53:17Z")); + assert.equal(newestCustomerTextTimestamp([]), 0); + assert.equal(newestCustomerTextTimestamp([], 5), 5); +}); + +test("shouldReclassifyStale: an agent reply alone does NOT re-arm (3931e910 regression)", () => { + // Already checked through the customer's Oct 9 message; agent replied Oct 13. + assert.equal(shouldReclassifyStale(msgs, at("2026-10-09T09:53:17Z")), false); +}); + +test("shouldReclassifyStale: a newer customer message re-arms", () => { + const withNewCustomer = [...msgs, { type: "text", from: "user", timestamp: at("2026-10-13T20:00:00Z") }]; + assert.equal(shouldReclassifyStale(withNewCustomer, at("2026-10-13T03:32:13Z")), true); +}); + +test("shouldReclassifyStale: never-checked conversation is classified once", () => { + assert.equal(shouldReclassifyStale(msgs, undefined), true); + assert.equal(shouldReclassifyStale([], undefined), true); +}); + +test("shouldReclassifyStale: existing marker stamped from agent text still holds", () => { + // Real state: checkedThroughAt = Oct 13 03:32 (an agent reply). Customer's + // newest message is older, so nothing re-arms. + assert.equal(shouldReclassifyStale(msgs, at("2026-10-13T03:32:13Z")), false); +}); + +test("nextCheckedThroughAt never moves backwards", () => { + const existing = at("2026-10-13T03:32:13Z"); + assert.equal(nextCheckedThroughAt(msgs, existing), existing); + assert.equal(nextCheckedThroughAt(msgs, undefined), at("2026-10-09T09:53:17Z")); + assert.equal(nextCheckedThroughAt([], undefined), 0); +}); + +test("conversationActivityHint: takes the newest metadata, tolerates missing active.last", () => { + const convo = { created_at: 100, updated_at: 300 }; // active.last absent, like email threads + assert.equal(conversationActivityHint(convo), 300); + assert.equal(conversationActivityHint({ created_at: 100, updated_at: 300, active: { last: 500 } }), 500); + assert.equal(conversationActivityHint({ created_at: 100 }), 100); +}); + +test("isCustomerQuiet: f9e20b9b shape (one customer msg in Aug, bot note in Oct) is quiet", () => { + const f9e2 = [ + { type: "text", from: "user", timestamp: at("2026-08-06T16:11:57Z") }, + { type: "text", from: "operator", timestamp: at("2026-08-12T08:07:30Z") }, + { type: "event", from: "operator", timestamp: at("2026-08-14T04:11:34Z") }, + { type: "note", from: "operator", timestamp: at("2026-10-13T23:52:03Z") }, + ]; + assert.equal(isCustomerQuiet(f9e2, NOW), true); +}); + +test("isCustomerQuiet: boundary around the window", () => { + const fresh = [{ type: "text", from: "user", timestamp: NOW - (DEDUPE_MAX_QUIET_DAYS - 1) * DAY }]; + const old = [{ type: "text", from: "user", timestamp: NOW - (DEDUPE_MAX_QUIET_DAYS + 1) * DAY }]; + assert.equal(isCustomerQuiet(fresh, NOW), false); + assert.equal(isCustomerQuiet(old, NOW), true); +}); + +test("isCustomerQuiet: a recent agent reply or note does not make it fresh", () => { + const m = [ + { type: "text", from: "user", timestamp: NOW - 30 * DAY }, + { type: "text", from: "operator", timestamp: NOW - 1 * DAY }, + { type: "note", from: "operator", timestamp: NOW - 1000 }, + ]; + assert.equal(isCustomerQuiet(m, NOW), true); +}); + +test("isCustomerQuiet: no customer text at all counts as quiet", () => { + assert.equal(isCustomerQuiet([], NOW), true); + assert.equal(isCustomerQuiet([{ type: "text", from: "operator", timestamp: NOW }], NOW), true); +}); diff --git a/scripts/crisp-classify.mjs b/scripts/crisp-classify.mjs index b0214b8..8f97efe 100644 --- a/scripts/crisp-classify.mjs +++ b/scripts/crisp-classify.mjs @@ -18,6 +18,12 @@ import { credsForAccount, } from "./crisp-client.mjs"; import { classifyAndRoute } from "./crisp-classifier.mjs"; +import { + newestTextTimestamp, + conversationActivityHint, + shouldReclassifyStale, + nextCheckedThroughAt, +} from "./crisp-activity.mjs"; import { emitEvent } from "./events.mjs"; const { GITHUB_STEP_SUMMARY } = process.env; @@ -61,16 +67,9 @@ function transcriptFrom(messages) { .join("\n"); } -// Newest timestamp among real conversation messages only (customer/agent -// text). Private notes -- including this pipeline's own "Investigation -// report" notes -- and event messages are excluded: Stage 2 posts its note -// AFTER Stage 1 stamps the marker, so counting notes made every investigation -// look like "new activity" on the next run and re-investigated the same -// conversation every few hours (confirmed on session_bd0acc7b). Only the -// automatic stale/reopen paths use this; manual-note branches are unchanged. -function newestTextTimestamp(messages, floor = 0) { - return messages.reduce((max, m) => (m.type === "text" ? Math.max(max, m.timestamp ?? 0) : max), floor); -} +// newestTextTimestamp excludes notes, including our own: counting them made every +// investigation look like new activity (session_bd0acc7b). The stale path also +// re-arms only on customer messages (see crisp-activity.mjs). // A rate-limit failure (crispFetch already retried and gave up) on ONE session // shouldn't kill the whole run -- that skips "Commit advanced state" and @@ -180,8 +179,8 @@ async function main() { const lastCheckedMs = new Date(cursor.last_checked).getTime(); const activeConversations = await fetchActiveConversations(creds); activeConversations.forEach((conversation) => { - const lastActiveAt = conversation.active?.last ?? conversation.created_at; - conversation._checkManualNote = lastActiveAt > lastCheckedMs; + // Metadata hint only: `active.last` is absent on email-origin threads. + conversation._checkManualNote = conversationActivityHint(conversation) > lastCheckedMs; }); // Search directly for the trigger phrase too, in case the page cap buries it. @@ -198,22 +197,16 @@ async function main() { const record = escalated[conversation.session_id] ?? { manualNoteCount: 0 }; const previousResolveAt = resolvedSeen[conversation.session_id]; - // active.last, not created_at, so a reopened thread isn't read as ancient. - // NOTE: active.last (and even updated_at) can lag behind real message - // activity, especially for email-origin conversations -- confirmed for - // real (session_bd0acc7b showed live replies well after both fields - // had stopped moving). So this is only used to decide whether it's - // worth fetching messages at all, never as the final word on - // freshness -- see checkedThroughAt below, which is timestamped from - // the actual fetched messages instead. - const lastActiveAt = conversation.active?.last ?? conversation.created_at; - const staleHours = (Date.now() - lastActiveAt) / (1000 * 60 * 60); - const eligibleForAutoEscalate = staleHours >= AUTO_ESCALATE_HOURS && staleHours <= AUTO_ESCALATE_MAX_HOURS; + // Pre-fetch hint only. `active.last` is absent on email threads, and metadata + // can lag real activity (session_bd0acc7b), so staleness is re-checked + // against the fetched messages below. + const hintStaleHours = (Date.now() - conversationActivityHint(conversation)) / (1000 * 60 * 60); + const hintEligibleForAutoEscalate = hintStaleHours >= AUTO_ESCALATE_HOURS && hintStaleHours <= AUTO_ESCALATE_MAX_HOURS; // Seen resolved before, active again now -- a reopen. const isReopen = previousResolveAt !== undefined; - if (!conversation._checkManualNote && !eligibleForAutoEscalate && !isReopen) continue; + if (!conversation._checkManualNote && !hintEligibleForAutoEscalate && !isReopen) continue; // On skip, nothing is recorded for this session, so the next run // re-evaluates it from scratch. @@ -222,6 +215,14 @@ async function main() { const manualNoteCount = conversation._checkManualNote ? countManualTriggerNotes(messages) : record.manualNoteCount; const hasNewManualNote = manualNoteCount > record.manualNoteCount; + // Authoritative staleness: time since the newest real text (notes excluded, + // since ours bump `updated_at`). Falls back to the hint if there is no text. + const newestRealTextAt = newestTextTimestamp(messages); + const staleHours = newestRealTextAt > 0 + ? (Date.now() - newestRealTextAt) / (1000 * 60 * 60) + : hintStaleHours; + const eligibleForAutoEscalate = staleHours >= AUTO_ESCALATE_HOURS && staleHours <= AUTO_ESCALATE_MAX_HOURS; + if (!hasNewManualNote && !eligibleForAutoEscalate && !isReopen) continue; // A reopen only sees what's new since the previous resolve; a manual note gets everything. @@ -290,11 +291,12 @@ async function main() { // up, but can never be blocked forever the way a permanent // already-investigated flag was (confirmed for real: session_2fc63232 // stayed stuck across repeated resolve/reopen cycles). - const newestMessageAt = newestTextTimestamp(messages); - if (record.checkedThroughAt === undefined || newestMessageAt > record.checkedThroughAt) { + // Re-arms only on a new customer message: support replies re-investigated + // an already-triaged thread twice with no_defect (session_3931e910). + if (shouldReclassifyStale(messages, record.checkedThroughAt)) { const result = await classifyAndRoute(accountConfig, conversation, transcript); recordScan("stale", accountKey, conversation.session_id, result); - record.checkedThroughAt = newestMessageAt; + record.checkedThroughAt = nextCheckedThroughAt(messages, record.checkedThroughAt); if (result.repo && result.actionable && result.kind !== "none") { matrix.push({ session_id: conversation.session_id, repo: result.repo, kind: result.kind, account: accountKey }); investigated.add(conversation.session_id); diff --git a/scripts/crisp-dedupe-active.mjs b/scripts/crisp-dedupe-active.mjs index 2c662b4..89a82ba 100644 --- a/scripts/crisp-dedupe-active.mjs +++ b/scripts/crisp-dedupe-active.mjs @@ -8,11 +8,12 @@ import { readFile, writeFile } from "node:fs/promises"; import { fetchActiveConversations, getInboxKey, - fetchTranscript, + fetchRawMessages, postNote, credsForAccount, conversationUrl, } from "./crisp-client.mjs"; +import { isCustomerQuiet, DEDUPE_MAX_QUIET_DAYS } from "./crisp-activity.mjs"; import { chatJSON } from "./openai-client.mjs"; import { listOpenIssues, listIssueComments, commentOnIssue } from "./github-client.mjs"; @@ -97,6 +98,7 @@ async function main() { let checked = 0; let matched = 0; + let skippedQuiet = 0; for (const [accountKey, accountConfig] of Object.entries(accounts)) { let creds; @@ -115,9 +117,21 @@ async function main() { if (escalatedThisRun.has(conversation.session_id)) continue; checked++; - const transcript = await fetchTranscript(creds, conversation.session_id); + const messages = await fetchRawMessages(creds, conversation.session_id); + const transcript = messages + .filter((m) => m.type === "text") + .map((m) => `${m.from === "user" ? "Customer" : "Agent"}: ${m.content}`) + .join("\n"); if (!transcript.trim()) continue; + // Skip threads the customer went quiet on (session_f9e20b9b was matched two + // months after it was resolved). Not marked notified, so a new customer + // message re-evaluates it. + if (isCustomerQuiet(messages)) { + skippedQuiet++; + continue; + } + const repo = await resolveRepo(accountConfig, transcript, conversation); if (!repo) continue; @@ -175,7 +189,7 @@ async function main() { if (GITHUB_STEP_SUMMARY) { await writeFile( GITHUB_STEP_SUMMARY, - `### Crisp triage — active-conversation dedupe check\n\nChecked: ${checked} · Matched: ${matched}\n`, + `### Crisp triage — active-conversation dedupe check\n\nChecked: ${checked} · Matched: ${matched} · Skipped (no customer message in ${DEDUPE_MAX_QUIET_DAYS}d): ${skippedQuiet}\n`, { flag: "a" } ); } diff --git a/skills/debug-crisp-triage-not-investigated/SKILL.md b/skills/debug-crisp-triage-not-investigated/SKILL.md index ada1192..7068b19 100644 --- a/skills/debug-crisp-triage-not-investigated/SKILL.md +++ b/skills/debug-crisp-triage-not-investigated/SKILL.md @@ -23,7 +23,11 @@ And get its *current live state* from Crisp directly (a temporary read-only `wor 2. **Was it permanently blocked by a stale flag?** Pre-2026-09-29, `investigated.has(session_id)` and `escalated[session_id].autoEscalated` were permanent once set, with no reset. If you're reading this after that fix, this specific cause shouldn't recur — but if `checkedThroughAt` looks like it should have re-armed and didn't, that's a regression worth treating seriously, not shrugging off. 3. **Did a resolve/reopen cycle mask an independent staleness check?** If `state/resolved-seen.json` has an entry for this session (`isReopen` would be true) and the reopen delta was empty (nothing new since that resolve), confirm the fallback to a full-history stale check actually fired — check for a `stale ... classifier says` log line, not just a `reopened after resolve` one. If neither log line exists at all for recent runs, it may have fallen outside the active-conversation page cap (200/account) — check its rank directly rather than assuming (`session_bd0acc7b` was rank 12; a different real case that hit the cap was confirmed on `THEMEGRILL` specifically). 4. **Does the screenshot you're comparing against actually match this session_id?** Cross-check the exact `session_id` in the URL before trusting a customer/team screenshot's content against your state-file findings — a mismatched screenshot sent an entire investigation down the wrong path once (confused a different, unrelated conversation's real content for this one's, which had a much shorter, quieter real history). -5. **Was it a genuine metadata lag, not a bug?** `active.last`/`updated_at` can lag real message timestamps — confirmed for real on at least one email-origin conversation. If the state file's timestamp looks "too old" compared to what a human says actually happened, this is the most likely explanation, and no code fix resolves it except computing freshness from real message timestamps directly (which `checkedThroughAt` now does, post-2026-09-29). +5. **Was it a genuine metadata lag, not a bug?** `active.last`/`updated_at` can lag real message timestamps — confirmed for real on at least one email-origin conversation. If the state file's timestamp looks "too old" compared to what a human says actually happened, this is the most likely explanation, and no code fix resolves it except computing freshness from real message timestamps directly (which `checkedThroughAt` now does, post-2026-09-29). Also: on email-origin conversations `active.last` is often **absent entirely** (not frozen) — a log line like "stale 344h" that counts back to `created_at` is the tell. `updated_at` is also bumped by this pipeline's own notes, so it's never proof a human did anything. + +## The opposite symptom: "why was this investigated *again*" / "why was a resolved-looking thread matched" + +Diagnose from message **roles and timestamps**, not content (see step 0's PII rule). For each re-run, line up each investigation's start time (the `investigate / Investigate (session_…)` job in `crisp-triage.yml` runs, and `tg-autopilot-data` events) with the preceding `type=text` message and its `from` (`user` vs `operator`). Since 2026-10-14 the stale path re-arms only on a new **customer** message, and `crisp-dedupe-active.mjs` skips conversations with no customer message in 14 days — if either still fires on an agent-only or long-quiet thread, that's a regression. If Crisp shows `state=unresolved` for a thread support resolved, check for a `state:resolved` event and whether the session was in the Sep-style seed (`seed-escalated.mjs` seeds from `fetchActiveConversations`, so presence in `escalated.json` from a seed commit proves Crisp listed it as unresolved *before* the bot ever acted). ## What forces an answer right now, regardless of root cause @@ -31,4 +35,4 @@ And get its *current live state* from Crisp directly (a temporary read-only `wor ## After finding the cause -If it points at a real code gap (not "working as designed" or "genuine metadata limitation"), fix the root cause in `crisp-classify.mjs`, not just this one session — and check whether `crisp-dedupe-active.mjs` or any other script shares the same class of bug before calling it done (as of 2026-09-29, it does: no freshness gate at all, a known but unfixed gap — see `CHANGELOG.md`). +If it points at a real code gap (not "working as designed" or "genuine metadata limitation"), fix the root cause in `crisp-classify.mjs`, not just this one session — and check whether `crisp-dedupe-active.mjs` or any other script shares the same class of bug before calling it done (as of 2026-10-14 `crisp-dedupe-active.mjs` has a 14-day customer-message freshness gate — see `CHANGELOG.md`).