🌐 US-Proxy
class="logged-out env-production page-responsive" style="word-wrap: break-word;" >
Skip to content

feat: add chat summary tab in the right sidebar and per-chat cost endpoint - #26649

Merged
jaaydenh merged 80 commits into
mainfrom
chat-summary-4kfj
Jul 27, 2026
Merged

feat: add chat summary tab in the right sidebar and per-chat cost endpoint#26649
jaaydenh merged 80 commits into
mainfrom
chat-summary-4kfj

Conversation

@jaaydenh

@jaaydenh jaaydenh commented Jun 24, 2026

Copy link
Copy Markdown
Contributor

Stacked on #26657 (the persisted whole-chat summary backend). Base branch is chat-summary-62j9; review/merge that first.

Adds a reusable ChatSummary component.

The summary text is the persisted whole-chat summary (chat.summary) introduced by #26657. It is generated asynchronously and may be null until the first summary is produced, in which case the popover renders a muted empty state. Live updates arrive via that PR's chat_summary_change watch event, which is already merged into the chat caches.

Cost is served by a new per-chat endpoint, GET /api/experimental/chats/{chat}/cost, which rolls up assistant-message cost across a chat's root and child (subagent) chats and is authorized like the other {chat} routes (read on the chat, 404 otherwise).

Visual and interaction coverage lives in ChatSummary.stories.tsx and ChatSummaryPopover.stories.tsx (including populated-summary, empty-state, and cost-loading cases).

@github-actions

github-actions Bot commented Jun 24, 2026

Copy link
Copy Markdown

Docs preview

Check off each page once it's been reviewed. If a page changes in a later push, its checkbox clears automatically so it gets a fresh look. Pages not yet wired into the docs navigation aren't listed here.

Add a persisted chats.summary populated by a background generator after
successful root-chat turns, delivered live via a new chat_summary_change
watch event, with per-feature cost attribution and a deployment-wide
summary-generation model override.
The UnknownContextReturns400 subtest hardcodes the valid override
context list in its expected error. Adding the summary_generation
context changed the message, so update both assertions.
@jaaydenh
jaaydenh force-pushed the chat-summary-4kfj branch from 28cda13 to aa80827 Compare June 24, 2026 12:56
@jaaydenh
jaaydenh changed the base branch from main to chat-summary-62j9 June 24, 2026 12:56
jaaydenh added 2 commits June 25, 2026 07:14
Background summary and manual title generation write hidden,
soft-deleted chat_messages rows tagged with cost_source to attribute
their spend. The AFTER STATEMENT history triggers advanced
chats.history_version for those rows too, so the history_version
optimistic guard on UpdateChatSummary was invalidated by the very
usage row recorded for that same summary. The summary write (and the
last_turn_summary write racing behind it) was then rejected as stale
even when no new turn had occurred, so chats.summary was never
persisted.

Add migration 000531 recreating the history triggers so only rows with
cost_source IS NULL (ordinary turn history) advance history_version.
snapshot_version still advances for every change, so history_version
correctly trails after an accounting-only write and catches up on the
next real message.
Add a reusable ChatSummary component surfaced via an info button placed to
the left of the chat sharing button in the chat header. The button opens a
"Summary" popover (400px on desktop, full width on mobile) showing the
chat's summary plus its Created/Updated dates and cumulative cost.

The summary text is the persisted whole-chat summary (chat.summary) from the
backend summary feature this branch is stacked on. It is generated
asynchronously and may be null until the first summary is produced, in which
case the popover renders a muted empty state.

Cost is served by a new per-chat endpoint, GET
/api/experimental/chats/{chat}/cost, that rolls up assistant-message cost
across a chat's root and child (subagent) chats and is authorized like the
other {chat} routes.
@jaaydenh
jaaydenh force-pushed the chat-summary-4kfj branch from aa80827 to ca032fd Compare June 25, 2026 09:49
@jaaydenh
jaaydenh changed the base branch from chat-summary-62j9 to fix-chat-history-accounting June 25, 2026 09:50
@jaaydenh

Copy link
Copy Markdown
Contributor Author

/coder-agents-review
@codex review

@coder-agents-review

coder-agents-review Bot commented Jun 26, 2026

Copy link
Copy Markdown
Contributor

Chat: Review posted | View chat
Requested: 2026-06-29 14:49 UTC by @jaaydenh
Spend: $97.93 / $100.00

Review history
  • R1 (2026-06-26): 21 reviewers, 6 Nit, 5 P3, 2 P4, COMMENT. Review
  • R2 (2026-06-29): 9 reviewers, 7 Nit, 6 P3, 2 P4, COMMENT. Review
  • R3 (2026-06-29): 3 reviewers, 7 Nit, 6 P3, 2 P4, APPROVE. Review

deep-review v0.9.0 | Round 3 | 7a57ac0..d786b24

Last posted: Round 3, 15 findings (6 P3, 2 P4, 7 Nit), APPROVE. Review

Finding inventory

Findings

# Sev Status Location Summary Round Reviewer Posted
CRF-1 P3 Author fixed (bcbd65e) ChatTopBar.tsx:101 ChatSummaryTopBarButton duplicates popover-with-reset pattern; 3-4 copies across PR R1 Netero, Robin P3, Zoro P3 Yes
CRF-2 P3 Author fixed (bcbd65e) ChatSummaryPopover.tsx:44 ChatSummaryPopoverContent shows infinite spinner on query failure, no error state R1 Netero, Nami P3, Chopper Note Yes
CRF-3 P3 Author fixed (bcbd65e) ChatSummaryPopover.tsx:52 Frontend discards unpriced_message_count; shows $0.00 for unpriced chats instead of flagging partial total R1 Hisoka P3, Knov P3, Mafuuu P3, Pariston P3, Nami Note Yes
CRF-4 P3 Author fixed (bcbd65e) exp_chats.go:2227 Dead IsNotAuthorizedError check returns 403 instead of 404; inconsistent with sibling handlers behind same middleware R1 Chopper P3, Kite P3, Ryosuke P3, Razor P3 Yes
CRF-5 P3 Author fixed (bcbd65e) exp_chats_test.go:10426 No test for GetChatCost on a chat with zero assistant messages R1 Bisky Yes
CRF-6 Nit Author fixed (bcbd65e) exp_chats.go:2231 Generic InternalServerError instead of specific error message like sibling handlers R1 Chopper Yes
CRF-7 Nit Author fixed (bcbd65e) ChatSummaryPopover.tsx:31 Comment "no extra request" is factually wrong; background refetch fires every open R1 Leorio Yes
CRF-8 Nit Author fixed (bcbd65e) ChatTopBar.tsx:107 contentGeneration remount trick has no comment explaining the non-obvious pattern R1 Leorio Yes
CRF-9 Nit Author fixed (bcbd65e) ChatSummaryPopover.stories.tsx:1 No error-state stories for summary popover; sibling sharing popover has them R1 Nami Yes
CRF-10 P4 Author fixed (bcbd65e) chats.sql:2424 No test seeds a non-assistant role message to verify the role filter R1 Bisky Yes
CRF-11 P4 Author fixed (bcbd65e) ChatSummary.tsx:22 Verbose comments restate what code already shows; 10+ instances across new files R1 Gon Yes
CRF-12 Nit Author fixed (bcbd65e) chats.ts:1981 staleTime: 30_000 is an inline magic number; file already defines PROMPTS_STALE_MS = 30_000 R1 Gon Yes
CRF-13 Nit Author fixed (bcbd65e) chats.sql:2402 SQL comment "Always returns exactly one row" lacks qualification that chat must exist R1 Mafuuu, Razor, Knuckle Yes
CRF-14 P3 Author fixed (d786b24) ARCHITECTURE.md:262 New paragraph references InsertChatAccountingMessage and cost_source which do not exist at HEAD R2 Netero Yes
CRF-15 Nit Author fixed (d786b24) ChatSummary.tsx:30 Comment verbosity pattern persists in new panel code (6 instances across ChatSummary.tsx and ChatSummaryPanel.tsx) R2 Gon Yes

Contested and acknowledged

(none)

Round log

Round 1

Panel. Netero + 21 reviewers. 5 P3, 2 P4, 6 Nit, 0 dropped. Reviewed against 9ad21cb..b4e09f9.

Round 2

Panel. Netero + 9 reviewers. All 13 R1 findings addressed. 1 P3, 1 Nit new. Reviewed against 7a57ac0..bcbd65e.

Round 3

Panel. Netero + 3 reviewers. All 15 findings addressed. 0 new. Reviewed against 7a57ac0..d786b24.

About deep-review

CRF = Coder Review Finding (P0-P4, Nit, Note)

Reviewer Focus
Bisky tests
Chopper ops/errors
Churn-guard change verification
Ging language modernization
Gon naming
Hisoka edge cases
Killua perf
Kite change integrity
Knov contracts
Knuckle SQL
Komugi flake/determinism
Kurapika security
Law decomposition
Leorio docs
Luffy product
Mafu-san process
Mafuuu contracts
Melody dispatch/pairing
Meruem structural
Nami frontend
Netero mechanical checks
Pariston premise testing
Pen-botter product gaps
Razor verification
Robin duplication
Ryosuke Go arch
Takumi concurrency
Zoro shape

🤖 Managed by Coder Agents.

@chatgpt-codex-connector chatgpt-codex-connector 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b4e09f9cf0

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread coderd/database/queries/chats.sql Outdated

@coder-agents-review coder-agents-review Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Clean vertical slice: SQL CTE with root rollup, dbauthz wrapper, handler with swagger, Go SDK, TS API + query factory, presentational component, data-fetching wrapper, ChatTopBar integration, and 9 story variants. Test density at 51.2% with genuine assertions throughout. The decomposition into a pure ChatSummary and a data-fetching ChatSummaryPopoverContent is well done.

Severity breakdown: 5 P3, 2 P4, 6 Nit.

The strongest convergent finding (5 reviewers independently): the backend deliberately tracks priced vs unpriced messages to let callers flag partial totals, but the frontend drops both counts and passes only total_cost_micros. A chat with no pricing configured shows "$0.00" instead of indicating cost tracking is unavailable. The data to fix this is already in the API response.

As Pariston put it: "The backend author understood the problem deeply enough to track partial pricing. The frontend didn't carry that understanding into the display."

The other convergent finding (4 reviewers + 4 notes): the getChatCost handler has a dead IsNotAuthorizedError -> Forbidden branch. ExtractChatParam middleware already handles auth with 404; the handler's 403 would leak resource existence if the middleware were removed. The sibling getChatMessages behind the same middleware does not have this check, confirming it was incorrectly copied from getChatCostSummary (which legitimately needs it because it runs outside ExtractChatParam).

🤖 This review was automatically generated with Coder Agents.

Comment thread site/src/pages/AgentsPage/components/ChatTopBar.tsx Outdated
Comment thread site/src/pages/AgentsPage/components/ChatSummaryPopover.tsx Outdated
Comment thread site/src/pages/AgentsPage/components/ChatSummaryPopover.tsx Outdated
Comment thread coderd/exp_chats.go Outdated
Comment thread coderd/exp_chats_test.go
Comment thread site/src/pages/AgentsPage/components/ChatSummaryPopover.stories.tsx Outdated
Comment thread coderd/database/queries/chats.sql
Comment thread site/src/pages/AgentsPage/components/ChatSummary.tsx Outdated
Comment thread site/src/api/queries/chats.ts Outdated
Comment thread coderd/database/queries/chats.sql Outdated
jaaydenh and others added 8 commits June 29, 2026 06:59
# Conflicts:
#	docs/admin/security/audit-logs.md
Resolve the actionable findings from the deep review of #26657:

- P0/CRF-2: renumber the chat summary migration from 000530 to 000534 to
  avoid colliding with 000530_relay_host_nats_port on main.
- CRF-3: scope summary_change/chat_summary_change merges to their own
  field so an equal-timestamp event cannot clobber the other summary;
  add the equal-timestamp regression tests.
- CRF-4/CRF-13: bail out of background summary generation when shutdown
  has begun so Close() is not blocked, and fix the stale comment.
- CRF-5: re-read the chat before the cadence gate so rapid turns do not
  both pass against a stale snapshot.
- CRF-7: add a CHECK constraint on chat_messages.cost_source.
- CRF-8: use a dedicated chatSummaryWriteTimeout for the summary write.
- CRF-17: only count sentence terminators at a word boundary so dotted
  identifiers do not inflate the count.
- CRF-1/CRF-6: deduplicate the usage-recording and model-override
  resolution paths shared by title and summary generation.
- CRF-15/CRF-16: cover ChatMessageVisibilityUser and model-only user
  messages in the summary tests.
- CRF-9/10/11/12/19/22: comment cleanups, errors.AsType, and drop the
  popover reference from the settings description.
CRF-23: add TestCountSentenceTerminators so the CRF-17 fix is covered.
Without it, removing the word-boundary guard in countSentenceTerminators
would not fail any test. The test asserts periods inside dotted
identifiers are not counted and that a dotted-identifier-dense summary
stays under summaryMaxSentences.
Resolve conflicts in coderd/x/chatd/chatd.go by routing both recordManualTitleUsage and recordChatSummaryUsage through the base branch's recordHiddenUsageMessageTx helper, and reimplementing that helper with this PR's atomic InsertChatAccountingMessage insert. This drops the removed UpdateChatMessageCostSource two-step tag and the last_model_config_id restore, which the dedicated accounting insert makes unnecessary.

Renumber the chat_history_ignore_accounting migration from 000531 to 000535 so it runs after 000534_chat_summary, which adds the cost_source column the recreated history triggers depend on.
…exclusion

Address review feedback on the chat history accounting fix:

- InsertChatAccountingMessage stores cost_source verbatim instead of NULLIF(@cost_source, ''), so an empty value fails the chat_messages CHECK instead of silently becoming NULL and re-enabling the history_version advance the trigger guard prevents.

- Document in coderd/x/chatd/ARCHITECTURE.md that hidden accounting rows tagged with cost_source do not advance history_version.

- Add a mock test for the recordChatSummaryUsage summary path and a DB assertion that an empty cost_source is rejected.
Move summary generation model overrides out of the persisted summary PR so the base feature always uses the chat model.

Co-authored-by: Cursor <cursoragent@cursor.com>
jaaydenh added 3 commits June 29, 2026 10:15
…000530 collision

Merging main into this stack produced two migrations numbered 000530:
000530_relay_host_nats_port (from main) and 000530_chat_summary (this
stack). The duplicate broke coderd/database/dump.sql generation, which
cascaded into the gen, lint, sqlc-vet, offlinedocs, and test-go CI jobs.

Renumber the stack's migrations above main's latest so every number is
unique:
- 000530_chat_summary                   -> 000534_chat_summary
- 000531_chat_history_ignore_accounting -> 000535_chat_history_ignore_accounting

The schema is unchanged (dump.sql regenerates identically). This resolves
an inherited base-stack conflict and may need reconciliation if the base
PR is renumbered separately.
…feedback

Move the chat summary out of the top-bar info button + popover and into a
new left-most "Summary" tab in the right panel:
- Add ChatSummaryPanel, a data wrapper that renders the existing
  presentational ChatSummary inside the panel and fetches the chat + cost
  only while the tab is visible.
- Remove the info button, ChatSummaryPopover, and the ChatTopBar
  render-prop wiring. Summary is the default right-panel tab when no tab
  selection is persisted.

Address review feedback on the per-chat cost endpoint and UI:
- GetChatCostByChatID matches the chat family by indexed columns instead of
  COALESCE(root_chat_id, id) so Postgres can use the chat indexes.
- Drop the dead IsNotAuthorizedError branch (the middleware already returns
  404) and return a specific "Failed to get chat cost." 500, matching the
  sibling chat handlers.
- Surface chat and cost query errors and partial (unpriced) totals in the
  UI, with matching stories.
- Add ZeroMessages and ExcludesNonAssistantMessages query tests.
- Name the cost staleTime constant and clarify the SQL comments.
jaaydenh and others added 10 commits July 8, 2026 15:15
Resolve conflict in coderd/x/chatd/chatd.go: adopt the renamed summary threshold constants (summaryInitialTurnThreshold, summaryStaleTurnThreshold) from the base branch while keeping the subagentReportSummary* constants added on this branch.

Generated by Coder Agents.
@github-actions github-actions Bot added the stale This issue is like stale bread. label Jul 16, 2026
@github-actions github-actions Bot closed this Jul 19, 2026
@jaaydenh jaaydenh reopened this Jul 20, 2026
jaaydenh and others added 3 commits July 20, 2026 06:26
Resolve merge conflicts in three files:

- coderd/x/chatd/quickgen.go: keep both "net/http" and "regexp"
  imports (both are used).
- coderd/exp_chats_internal_test.go: union the imports and keep all
  tests from both sides (TestGetChatCostSurfacesReadAuthzRace and
  TestGetChatCostQueriesRequestedChat from this branch, plus
  TestEnrichMissingChatAgentIDs from the base branch).
- coderd/x/chatd/turn_summary_internal_test.go: keep this branch's
  subagent-report summary persistence test
  (TestSuccessfulChildChatOutcomeStoresReportSummaryWithoutPush). Drop
  TestPendingChatPersistsSummaryButSkipsWebPush because the base branch
  removed the unused "pending"/"paused"/"completed" chat statuses
  (migration 000543, PR #27064); that status no longer exists.
@jaaydenh jaaydenh removed the stale This issue is like stale bread. label Jul 20, 2026
@linear-code

linear-code Bot commented Jul 21, 2026

Copy link
Copy Markdown

CODAGT-568

jaaydenh added a commit that referenced this pull request Jul 23, 2026
…6657)

Adds a persisted whole-chat summary that backs the chat summary popover.
A new nullable `chats.summary` column is populated in the background
after a successful root-chat turn and pushed to clients via a new
`chat_summary_change` watch event (distinct from `summary_change`, which
is bound to `last_turn_summary`), so the popover reads `chat.summary`
straight off the loaded `Chat` with no extra query.

This is the data source for the popover and per-chat cost UI built in
#26649; the popover can consume `chat.summary` once this lands (the
field is nullable, so merge order does not matter).

## How it works

- **Generation** runs in the existing successful-turn finalize hook,
detached from the request so the user's turn is never blocked. A cadence
gate generates the first summary after one completed turn, then
regenerates every three turns, using the `chats.summary_generated_at`
freshness marker. Generation reads compaction-aware history, renders it
to a bounded plain-text transcript (short transcripts are skipped), and
asks for a 1-3 sentence summary via structured output. Failures never
clear an existing summary.
- **Staleness** is guarded by `history_version` (mirroring
`last_turn_summary`), so a background write racing a newer turn loses
while worker lifecycle transitions cannot reject a fresh write.
- **Model selection** uses the chat's configured model.

## Deferred to follow-ups

- **Cost accounting**: the `chat_messages.cost_source` discriminator and
summary/title usage recording were removed from this PR so summary
persistence is not blocked by hidden accounting rows advancing
`history_version`. Title usage recording stays on main's
`InsertChatMessages` path.
- **Model override**: deployment-wide summary generation model selection
is split into #26803; the base feature always uses the chat model.

## Notes

- Migration `000540` adds `chats.summary` and
`chats.summary_generated_at`, and recreates `chats_expanded` to expose
the new columns.
- Root chats only; shared viewers pick up the summary on their next
refetch (live watch events are owner-only).

Refs #26649

---------

Co-authored-by: Cursor <cursoragent@cursor.com>
Base automatically changed from chat-summary-62j9 to main July 23, 2026 15:36
jaaydenh and others added 2 commits July 23, 2026 16:11
Resolve conflicts from #26657 (whole-chat summary landed on main) and the AgentsPage -> AgentsPageLayout refactor.

- Drop the branch's duplicate 000549_chat_summary migration; main's identical 000551_chat_summary supersedes it.
- Adopt main's reordered summary helper signatures (logger before chat) while keeping the net-new subagent report summary feature.
- Re-integrate chatCostIdsToInvalidate and the per-chat cost endpoint into the renamed AgentsPageLayout.
- Regenerate database code (adds GetChatModelUsageCostByChatID) and API docs.
@jaaydenh
jaaydenh requested a review from a team as a code owner July 27, 2026 08:03
TestSuccessfulChildChatOutcomeStoresReportSummaryWithoutPush now inserts
an assistant message, so deriveFinalTurnRunResult runs past the empty
final-text guard and reaches ensureSyntheticAPIKeyID. That path calls
p.clock.Now(), which segfaulted because the hand-built Server literal
left clock nil. The panic killed the whole coderd/x/chatd test binary,
reporting every test in the package as failed.

Generated by Coder Agents on behalf of @jaaydenh.
@jaaydenh
jaaydenh merged commit 6f2011a into main Jul 27, 2026
32 checks passed
@jaaydenh
jaaydenh deleted the chat-summary-4kfj branch July 27, 2026 09:05
@github-actions github-actions Bot locked and limited conversation to collaborators Jul 27, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants