Skip to content

feat(attention): an agent mention nobody answered for a week is missed, not "needs you" - #2364

Closed
lilyshen0722 wants to merge 2 commits into
mainfrom
feat/needs-you-missed-after-7d
Closed

lilyshen0722 wants to merge 2 commits into
mainfrom
feat/needs-you-missed-after-7d

Conversation

@lilyshen0722

Copy link
Copy Markdown
Contributor

Why

Only a thread reply counts as a reply (Sam, 09-06). So an agent's mention that nobody answers in its thread stays in the needs-you queue for good. Seats post status and gate notes that name a person, and nothing ever expired them.

A read-only dry run on production, 10-20, found 640 open agent-mention rows older than a week:

  • 535 on the lily-shen operator account;
  • 104 on Sam's;
  • 1 on connector-ops.

Sam's queue held 119 rows, the oldest from 09-11, and almost none of them still needed him. A separate check of every ask in it found none that did.

What

  • AttentionItem.resolvedBy gains missed. It means resolved, but never counted as acted on. The row stays in the store, and its message stays in the pod.

  • sweepMissedAgentMentions({ now, apply }) resolves an open mention as missed when both hold:

    • its source is AGENT_MENTION_MISSED_DAYS (7) days old: sourceCreatedAt, falling back to createdAt;
    • its author is an agent: actorUserId resolves to an isBot user.

    The write re-checks status: 'open', so a reply or acknowledgement that lands during a pass keeps its own resolution.

  • Never expired: a person's mention, a decision, an approval or a handoff. A row with no recorded actor is left alone too.

  • schedulerService runs the sweep hourly at :17 and logs every pass.

When this sweep goes live, Sam's queue drops from 119 to about 15, by rule, with nothing deleted.

Tests

  • attentionItemService.test.js:
    • an agent's week-old mention becomes missed and a person's doesn't;
    • the exact query: open, has an actor, older than the 7-day cutoff by source time with the createdAt fallback;
    • nothing old enough means no actor lookup and no write;
    • a dry run writes nothing;
    • the window is 7.
  • models/AttentionItem.resolvedBy.test.js: the real schema accepts replied, acknowledged and missed, and refuses anything else.

Mutations. Each fails 1 test:

mutation result
enum without missed 1 failed
no bot filter 1 failed
stamp acknowledged instead 1 failed
a 1-day window 1 failed
drop the status: 'open' guard at write 1 failed

34 related suites pass (445 tests). Lint shows 0 errors and tsc is clean on the changed files.

Also needed outside this PR

Growth's daily NEEDSYOU line counts "acted" as resolved && resolvedBy !== 'acknowledged', so it would count missed as acted on. That script also has to exclude missed. It's Growth's script, so the change has been flagged there.

Merge

Draft. It goes in Wednesday's second deploy if it's gated by then, otherwise the next one.

🤖 Generated with Claude Code

https://claude.ai/code/session_01Wkz5axAzCvoLEFkZ5KtkKG

lilyshen0722 and others added 2 commits October 20, 2026 12:29
…d, not "needs you"

A thread reply is the only reply (Sam, 2026-09-06), so an agent's mention that
nobody answered in its thread stayed in the needs-you queue for good. Seats
post status and gate notes that name a person; the 2026-10-20 audit found 640
such rows older than a week (535 on one operator account, 104 on Sam's, whose
queue held 119, the oldest from 09-11).

- AttentionItem.resolvedBy gains `missed`: resolved, never counted as acted
  on. The row stays in the store and its message stays in the pod.
- sweepMissedAgentMentions resolves an open mention as `missed` once its
  source is AGENT_MENTION_MISSED_DAYS (7) old and its author is an agent,
  re-checking `status: open` at write time so a reply or acknowledgement that
  lands during the pass keeps its own resolution. A person's mention, a
  decision, an approval and a handoff never expire this way, and a row with no
  recorded actor is left alone.
- schedulerService runs it hourly and logs every pass.

Reply semantics are unchanged.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Wkz5axAzCvoLEFkZ5KtkKG

@lilyshen0722 lilyshen0722 left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Code gate — #2364 at 3bc4f0a0a324ad41f1b5661200a1a19ca54c2a11

origin/main = cd37b32e0f9288ee24922362d3b86d17c5590368, which is this PR's merge-base. CI at the exact head: 14 pass / 2 skipping; Test & Coverage and Service Tests (Tier 1 — real DBs) both SUCCESS in run 39484258341, whose head_sha I read back as the head above. BASE: 3 suites / 52 tests.

No defects. Ten mutations, every one discriminates — including the two I expected to come back green.

mutation (attentionItemService.ts) site result
drop kind: 'mention' from the find :305 1 failed
drop status: 'open' from the write filter :322 1 failed
treat every actor as an agent :317 1 failed
invert the bot filter :315 1 failed
drop the createdAt fallback branch :310 1 failed
window 7 → 8 days :298 2 failed — including the window is seven days
apply default true → false :301 1 failed
drop the actorUserId existence clause :307 1 failed
resolvedBy: 'missed' → 'acknowledged' :323 1 failed

A person's items never expiring is structural, not incidental

I checked what the guarantee actually rests on rather than taking the test's word. Kind is 'mention' | 'approval' | 'decision' | 'handoff' (:23), and recordTaskAttention writes kind: 'handoff' as Kind (:414) — so decisions, approvals and handoffs are outside the sweep's find by type, not by timing, and a person's mention is excluded by the isBot: true actor filter. Three mutations each break it from a different side, which is the right shape for a guarantee with two independent terms.

The $or on sourceCreatedAt with the createdAt fallback is this file's own established idiom — resolveMentionAttentionForReply carries the identical pair at :195-200, with the comment explaining why ("sourceCreatedAt is written on all new rows"). Reused verbatim, so it is consistency rather than a new risk. Worth noting that the single top-level $or is also safe here: the pre-existing site needs $and: [{ $or: targets }] precisely because two top-level $ors would clobber, and this query has only one.

Two things to know, neither a change

The apply default is the inverse of both sibling sweeps, deliberately. sweepResolvedMentionAttention (:250) and sweepDoneTaskHandoffs (:445) both default apply = false; this one defaults true, and the scheduler calls it bare. That is pinned — flipping the default to false fails a test — so it is a decision, not a slip. The cost is that someone reading across the three, or calling this one from a script on the house assumption of dry-run, writes rows instead of counting them. One sentence in the docblock would close that; I would not change the default now that a test depends on it.

resolvedBy has no reader in this repo — only writers. grep finds four write sites and no branch that reads it; getOpenQueue filters on status. So of the stated property, the "resolved" half is verifiable and pinned here, and the "never counted as acted on" half lives entirely in Growth's NEEDSYOU allowlist, which is outside this repo and cannot be checked from it. The comment you added to the enum is the right mitigation and is the only thing carrying it — keep it in any rebase.

The query is unindexed (no AttentionItem index leads with kind or status; the two status indexes lead with recipientUserId and podId), so the hourly pass is a collection scan — exactly like both sibling sweeps, and the eligible set collapses after the first pass. Not worth an index on 640 rows.

The shared rebase: not one line, and I ran it

The note says whichever merges second takes "a one-line rebase that keeps both." Measured — git merge-tree reports content conflicts in three files, and performing the merge needs four resolutions:

  1. AttentionItem.ts — the TS union and the block comment above it. Both sides rewrote the same comment differently, so this is a prose merge, not a line union.
  2. AttentionItem.ts — the Mongoose enum array.
  3. attentionItemService.ts — the export default and module.exports lines. This one is a rename collision, not a union: #2363 renames TASK_HANDOFF_RE to TASK_HANDOFF_CANDIDATE_RE and redefines its pattern, while this PR's export line still names the old symbol. Keeping either side verbatim loses real names; both naive choices fail loudly (a missing identifier is a compile error, a dropped one reddens the other PR's tests), so nothing goes silent — but "keep both" is ambiguous here in a way it is not for the enum.
  4. backend/__tests__/unit/services/attentionItemService.test.js — and this one does not parse if you resolve it the obvious way. Both sides append a describe block, and the });\n}); after the conflict region closes only one of them. I concatenated the two sides and got TS1005: '}' expected at line 781, with that suite failing to load while the other four passed. The correct resolution inserts ours' two closers before theirs opens.

Good news, measured rather than assumed: the two PRs are compatible. With all four resolved properly, the production files typecheck clean (50 pre-existing tsc errors on the tree, all in cleanup-test-data.ts and friends, none in any merged file) and the combined suites are 5 suites / 86 tests, all passing — 78 from #2363's scope plus this PR's 8. So the only thing wrong with the note is the word "one-line", and the one place a careless resolution bites is the test file rather than the enum everyone is watching.

Written by sprint-review, a Commonly agent — https://commonly.me/v2/pods/6a692a1be833c668acdb84cf

GATE: PASS

@lilyshen0722

Copy link
Copy Markdown
Contributor Author

Folded into #2363, which now carries this change as commit b24fe55a (cherry-picked, with the four conflicts with #2363 resolved). The two PRs edited the same enum, comment, export lines and test file, so one PR and one gate avoid a four-resolution rebase in the middle of Wednesday's second deploy. sprint-review's PASS here at 3bc4f0a0 covers this change; the combined head is re-gated on #2363.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant