feat: chat auto-archive owner digest notifications#24643
Conversation
Docs preview |
johnstcn
force-pushed
the
cj/chat-archive-core
branch
2 times, most recently
from
4583b50 to
641ee3a
Compare
johnstcn
force-pushed
the
cj/chat-archive-notifications
branch
from
5ef1213 to
d18e271
Compare
johnstcn
changed the title
johnstcn
force-pushed
the
cj/chat-archive-core
branch
7 times, most recently
from
81ffa45 to
e1e03ba
Compare
johnstcn
force-pushed
the
cj/chat-archive-notifications
branch
4 times, most recently
from
2110a30 to
d18e271
Compare
johnstcn
force-pushed
the
cj/chat-archive-core
branch
from
d0cc93d to
ddbfe37
Compare
johnstcn
force-pushed
the
cj/chat-archive-notifications
branch
3 times, most recently
from
52454f0 to
967880f
Compare
Adds a background job in `dbpurge` that periodically archives chats inactive beyond a configurable threshold. Each archived root chat gets a background audit entry tagged `chat_auto_archive`. Disabled by default. * New `AutoArchiveInactiveChats` SQL query with LATERAL last-activity subquery and partial index on archive candidates * `site_configs`-backed `auto_archive_days` setting with admin-only PUT, any-authenticated-user GET * Cascade archive via `root_chat_id`; pinned chats and active threads exempt * Root-only audit dispatch on detached context, matching manual archive (`patchChat`) behavior * 11 subtests covering disabled no-op, boundary, deleted messages, child activity, pinned exemption, multi-owner, idempotency, and batch pagination PR #24643 adds per-owner digest notifications. PR #24704 adds the requisite UI controls. > 🤖
johnstcn
force-pushed
the
cj/chat-archive-notifications
branch
2 times, most recently
from
d6e9113 to
65bac27
Compare
Layers per-owner digest notifications onto the chat auto-archive subsystem introduced in #24642. Each tick's archived rows are grouped by owner, the top 25 titles per owner are rendered into a new 'Chats Auto-Archived' notification template, and any remainder surfaces as 'and N more'. Duplicate same-day digests collapse via the notifier's existing dedupe hash. The template body branches on retention_days: when retention is disabled (retention_days=0), users are told archived chats are kept indefinitely rather than falsely claiming imminent deletion. Adds one test subtest for the digest overflow cap, plus digest assertions to the existing archive subtests from #24642. Two notification test fixtures (standard and retention-zero) lock the two template branches into goldens. Depends on #24642. Migration 000476 adds the notification template row; 000475 (in the core PR) already created the candidate index.
johnstcn
force-pushed
the
cj/chat-archive-notifications
branch
from
65bac27 to
05d96ca
Compare
|
/coder-agents-review |
![coder-agents-review[bot]](https://avatars.githubusercontent.com/in/3454270?s=60&v=4){kind=small}
There was a problem hiding this comment.
Well-engineered feature. The dual-context shutdown design, the deterministic blockingEnqueuer test, the self-review commit that filled three categories of gaps before requesting review, and the thorough golden-file coverage across all template branches are all quality work.
Severity count: 1 P2 (body), 8 P3, 4 Nit, 1 Note.
The P2 is a stale claim in the PR description: it says dedupe collapses same-day digests, but the code comments and BatchSizePagination test confirm the opposite. Four reviewers independently flagged the template grammar bug (singular "chat was" when overflow reveals multiple chats). The most actionable P3s are the missing config-field test assertions and the CTA linking to the active-chats view instead of archived.
P2 [DEREM-4]: PR description stale dedupe claim. The description says "Duplicate same-day digests collapse via the notifier's existing dedupe hash." The code comments at dbpurge.go:384 say the opposite: "notification_messages dedupe will not collapse these, because each tick's payload carries a different title list." The BatchSizePagination test explicitly asserts two separate digests for the same owner across ticks. The initial commit carried this claim; the self-review commit corrected the code comments but left the description untouched. Please update the PR description to match the corrected understanding. (Mafu-san)
Ging-Go found no Go idiom issues. Pariston's differential diagnosis confirmed the problem-solution alignment: framing #1 (missing notification) is the right one, and the stateless per-tick model is a reasonable first implementation.
"A user who clicks 'View chats' expecting to see their archived conversations sees their active ones instead and has to find the archived filter." (Luffy)
🤖 This review was automatically generated with Coder Agents.
PR description updated. |
|
/coder-agents-review |
There was a problem hiding this comment.
All 15 R1 findings addressed. The DEREM-7 fix (both chat config reads skip together on error) is stronger than what was suggested. The DEREM-2 fix chose a pragmatically different approach: since singular-with-overflow is unreachable under the current cap, the golden was updated to a realistic shape rather than patching a dead template branch. DEREM-11's sorted owner iteration now uses slice.Ascending matching the repo idiom.
R2 findings: 2 P3, 3 Nit. The two P3s are a garbled comment and a missing golden test for the singular template branch. The three Nits are cleanup from fix commits.
Meruem verified the structural correctness of the two-context split, concurrency safety of buildDigestData, and the sorted owner iteration. No interleaving issues found.
"This test suite is a little treasure chest." (Bisky)
🤖 This review was automatically generated with Coder Agents.
2 participants
Depends on #24642
Adds per-owner digest notifications onto the chat auto-archive subsystem.
Each tick's archived rows are grouped by owner, the top 25 titles per owner are rendered into a new
Chats Auto-Archivednotification template, and any remainder surfaces asand N more. Each digest is per-tick, so users with large amounts of purgeable data may get multiple notifications in sequence (one per user per tick).The template body branches on
retention_days: when retention is disabled (retention_days=0), users are told archived chats are kept indefinitely rather than falsely claiming imminent deletion.Changes
000XXX_chat_auto_archive_notification_templateadds new notification templatedbpurge: threadsnotifications.EnqueuerthroughNew; and enqueues notification message.cli/server.go: passesoptions.NotificationsEnqueuerintodbpurge.New.coderd/notifications/events.go: newTemplateChatAutoArchiveDigestUUID.coderd/inboxnotifications.go: inbox registration.Notificationssection tochat-auto-archive.md.