From 30d91caa33beeb31333d49f05b91d4f6c1fa008e Mon Sep 17 00:00:00 2001 From: Phil Merrell Date: Tue, 15 Sep 2026 20:20:34 -0600 Subject: [PATCH] fix(admin-costs): list a user's deleted conversations in the cost drill-down A soft delete flips `status` and drops the sidebar GSI keys, but the session's `C#` cost rows and its share of the user's period total survive it. The per-user conversation list dropped those tombstones, so the audit could not account for the spend it was built to explain: one prod user showed a single $3.77 conversation against $20.32 of September cost. Fleet-wide, 183 of 5,440 September-active sessions (3.4%) are deleted and carry $58 of $817. The storage reader gains `include_deleted` (default off, so callers that list what the user can still open are unchanged); the admin service asks for tombstones, normalises legacy `deleted` rows to `status="deleted"`, and reports `deletedSessionCount` / `deletedSessionCost` on the response. The SPA badges deleted rows and appends "N deleted ($X still counted in the total)" to the summary line. Profile and anatomy still resolve for a deleted session because `SessionLookupIndex` keys are kept on the tombstone. Co-Authored-By: Claude Fable 5.1 --- .../src/apis/app_api/admin/costs/models.py | 6 +++ .../src/apis/app_api/admin/costs/service.py | 16 ++++++- .../apis/shared/storage/dynamodb_storage.py | 18 ++++++-- .../admin/costs/test_user_sessions_service.py | 44 ++++++++++++++++++- .../costs/test_content_free_projections.py | 28 ++++++++++-- .../user-conversations.component.spec.ts | 20 +++++++++ .../user-conversations.component.ts | 14 ++++++ .../admin/costs/models/admin-cost.models.ts | 7 +++ 8 files changed, 142 insertions(+), 11 deletions(-) diff --git a/backend/src/apis/app_api/admin/costs/models.py b/backend/src/apis/app_api/admin/costs/models.py index ec5fc5ab..be72e451 100644 --- a/backend/src/apis/app_api/admin/costs/models.py +++ b/backend/src/apis/app_api/admin/costs/models.py @@ -327,6 +327,12 @@ class UserSessionsResponse(BaseModel): # Sessions excluded because their cost is unrecorded (they are still listed, # with costKnown=False, when sort != "cost"; under cost-sort they trail). unknown_cost_count: int = Field(0, alias="unknownCostCount") + # Soft-deleted conversations in the list (status="deleted"). Listed, not + # hidden: a delete removes the row from the user's sidebar, not its cost + # rows or its share of `userPeriodCost`, so an audit that dropped them + # could not account for the period total. + deleted_session_count: int = Field(0, alias="deletedSessionCount") + deleted_session_cost: float = Field(0.0, alias="deletedSessionCost") class AttachmentProfile(BaseModel): diff --git a/backend/src/apis/app_api/admin/costs/service.py b/backend/src/apis/app_api/admin/costs/service.py index be484cad..4e266b5d 100644 --- a/backend/src/apis/app_api/admin/costs/service.py +++ b/backend/src/apis/app_api/admin/costs/service.py @@ -831,15 +831,23 @@ async def get_user_sessions( period = None if all_time else (period or self._get_current_period()) active_since = self._get_period_date_range(period)[0] if period else None + # Deleted conversations stay in the list: their cost rows and their + # share of the period total outlive the delete, so hiding them left + # a user's spend unaccounted for (one $3.77 row against $20.32). rows = await self.storage.get_user_session_diagnostics( user_id=user_id, active_since=active_since, + include_deleted=True, ) user_period_cost = await self._user_period_cost(user_id, period) threshold = compaction_token_threshold() summaries: List[UserSessionSummary] = [] for row in rows: + if row.get("deleted") or row.get("status") == "deleted": + # Legacy tombstones carry `deleted` without the status flip; + # normalise so the page has one signal to render. + row["status"] = "deleted" share = self._share(_as_float(row.get("totalCost")), user_period_cost) findings = run_diagnoses(self._row_facts(row, threshold, share)) summaries.append(self._session_summary(row, findings, share)) @@ -860,9 +868,11 @@ def recent_key(s: UserSessionSummary) -> str: ) unknown = sum(1 for s in summaries if not s.cost_known) + deleted = [s for s in summaries if s.status == "deleted"] + deleted_cost = sum(s.total_cost for s in deleted if s.total_cost is not None) logger.info( - f"User sessions: {len(summaries)} rows ({unknown} unknown-cost), " - f"returning {min(limit, len(summaries))}" + f"User sessions: {len(summaries)} rows ({unknown} unknown-cost, " + f"{len(deleted)} deleted), returning {min(limit, len(summaries))}" ) return UserSessionsResponse( user_id=user_id, @@ -871,6 +881,8 @@ def recent_key(s: UserSessionSummary) -> str: sessions=summaries[:limit], total=len(summaries), unknown_cost_count=unknown, + deleted_session_count=len(deleted), + deleted_session_cost=round(deleted_cost, 6), ) async def _attachment_profile(self, session_id: str) -> AttachmentProfile: diff --git a/backend/src/apis/shared/storage/dynamodb_storage.py b/backend/src/apis/shared/storage/dynamodb_storage.py index 8faaf08a..b010640a 100644 --- a/backend/src/apis/shared/storage/dynamodb_storage.py +++ b/backend/src/apis/shared/storage/dynamodb_storage.py @@ -825,11 +825,20 @@ async def get_user_session_diagnostics( self, user_id: str, active_since: Optional[str] = None, + include_deleted: bool = False, ) -> List[Dict[str, Any]]: """One user's session rows, content-free, with everything a diagnostic list needs: cost/cache rollups, context, model, enabled tool ids, agent binding, compaction coordinates and the behavioral counters. + ``include_deleted=True`` keeps soft-deleted rows (``deleted`` / + ``status="deleted"``). A delete is a tombstone, not a refund: the + session's ``C#`` rows and its share of the user's period total survive + it, so an audit that hides these rows cannot account for the user's + spend — one prod user showed a single $3.77 conversation against + $20.32 of period cost. The default stays exclusive for callers that + list what the user can still open. + Same bounded base-table query as :meth:`get_user_session_costs`; the difference is the projection (``SESSION_ROW_PROJECTION``) and the post-processing: ``compaction.summary`` is measured into @@ -848,6 +857,7 @@ async def get_user_session_diagnostics( active_since=active_since, projection=projection, names=names, + include_deleted=include_deleted, ) return [self._content_free_session_row(item) for item in items] @@ -857,13 +867,15 @@ async def _query_user_session_rows( active_since: Optional[str], projection: str, names: Optional[Dict[str, str]], + include_deleted: bool = False, ) -> List[Dict[str, Any]]: """Shared body of the two per-user session readers. ``PK = USER#``, ``SK begins_with S#`` — matches both the static (``S#``) and legacy (``S#ACTIVE#…``) schemes and no other row - family. Paginates, converts Decimals, drops soft-deleted rows, and - applies ``active_since`` client-side (``lastMessageAt`` is not a key). + family. Paginates, converts Decimals, drops soft-deleted rows unless + ``include_deleted``, and applies ``active_since`` client-side + (``lastMessageAt`` is not a key). """ from boto3.dynamodb.conditions import Key @@ -890,7 +902,7 @@ async def _query_user_session_rows( results = [] for item in items: item_float = self._convert_decimal_to_float(item) - if item_float.get("deleted"): + if item_float.get("deleted") and not include_deleted: continue if active_since: last_message_at = item_float.get("lastMessageAt") or "" diff --git a/backend/tests/apis/app_api/admin/costs/test_user_sessions_service.py b/backend/tests/apis/app_api/admin/costs/test_user_sessions_service.py index 811926d4..dfb2c53c 100644 --- a/backend/tests/apis/app_api/admin/costs/test_user_sessions_service.py +++ b/backend/tests/apis/app_api/admin/costs/test_user_sessions_service.py @@ -114,7 +114,7 @@ async def test_all_time_drops_the_period_and_the_share(): assert resp.period is None and resp.user_period_cost is None assert resp.sessions[0].share_of_user_period is None # No period → no active_since filter reaches storage. - service.storage.get_user_session_diagnostics.assert_awaited_once_with(user_id="u1", active_since=None) + service.storage.get_user_session_diagnostics.assert_awaited_once_with(user_id="u1", active_since=None, include_deleted=True) service.storage.get_user_cost_summary.assert_not_awaited() @@ -123,7 +123,7 @@ async def test_period_scoping_passes_the_month_start_to_storage(): service = _service([]) await service.get_user_sessions("u1", period="2026-09") service.storage.get_user_session_diagnostics.assert_awaited_once_with( - user_id="u1", active_since="2026-09-01" + user_id="u1", active_since="2026-09-01", include_deleted=True ) @@ -160,3 +160,43 @@ async def test_a_broken_period_cost_lookup_degrades_to_no_share(): resp = await service.get_user_sessions("u1", period="2026-09") assert resp.user_period_cost is None assert resp.sessions[0].share_of_user_period is None + + +@pytest.mark.asyncio +async def test_deleted_conversations_are_listed_flagged_and_rolled_up(): + service = _service( + [ + _row("live", totalCost=0.5), + _row("gone", totalCost=2.25, deleted=True, status="deleted"), + _row("legacy-tombstone", totalCost=1.0, deleted=True, status="active"), + _row("gone-unpriced", totalCost=None, deleted=True, status="deleted"), + ], + period_cost=4.0, + ) + + response = await service.get_user_sessions(user_id="u1", period="2026-09") + + # The storage reader is asked for tombstones explicitly — the default + # reader hides them, which is what left a $20 month showing one $3 row. + service.storage.get_user_session_diagnostics.assert_awaited_once() + assert service.storage.get_user_session_diagnostics.await_args.kwargs["include_deleted"] is True + + by_id = {s.session_id: s for s in response.sessions} + assert by_id["live"].status == "active" + assert by_id["gone"].status == "deleted" + # A legacy tombstone carries `deleted` without the status flip; the page + # gets one normalised signal. + assert by_id["legacy-tombstone"].status == "deleted" + assert response.total == 4 + assert response.deleted_session_count == 3 + assert response.deleted_session_cost == pytest.approx(3.25) + # Deleted rows still take their share of the period total. + assert by_id["gone"].share_of_user_period == pytest.approx(56.25) + + +@pytest.mark.asyncio +async def test_no_deleted_conversations_reports_zero(): + service = _service([_row("live")], period_cost=1.0) + response = await service.get_user_sessions(user_id="u1", period="2026-09") + assert response.deleted_session_count == 0 + assert response.deleted_session_cost == 0.0 diff --git a/backend/tests/costs/test_content_free_projections.py b/backend/tests/costs/test_content_free_projections.py index ffaf7085..454a4583 100644 --- a/backend/tests/costs/test_content_free_projections.py +++ b/backend/tests/costs/test_content_free_projections.py @@ -148,12 +148,32 @@ async def test_top_sessions_reader_keeps_title_by_decision(storage): assert "compaction" not in rows[0] # and it never widened into the diagnostic fields -@pytest.mark.asyncio -async def test_deleted_sessions_are_excluded_from_the_diagnostic_list(storage): - _seed(storage) +def _seed_deleted(storage): storage.sessions_metadata_table.put_item(Item={ "PK": f"USER#{USER_ID}", "SK": "S#gone", "GSI_PK": "SESSION#gone", "GSI_SK": "META", - "sessionId": "gone", "userId": USER_ID, "deleted": True, "totalCost": Decimal("9"), + "sessionId": "gone", "userId": USER_ID, "deleted": True, "status": "deleted", + "totalCost": Decimal("9"), "title": "DELETED TITLE", }) + + +@pytest.mark.asyncio +async def test_deleted_sessions_are_excluded_from_the_diagnostic_list_by_default(storage): + _seed(storage) + _seed_deleted(storage) rows = await storage.get_user_session_diagnostics(USER_ID) assert [r["sessionId"] for r in rows] == [SESSION_ID] + + +@pytest.mark.asyncio +async def test_deleted_sessions_are_listed_on_request_and_stay_content_free(storage): + # A delete is a tombstone, not a refund: the row's cost survives it, so + # the audit must be able to see it to account for the period total. + _seed(storage) + _seed_deleted(storage) + rows = await storage.get_user_session_diagnostics(USER_ID, include_deleted=True) + by_id = {r["sessionId"]: r for r in rows} + assert set(by_id) == {SESSION_ID, "gone"} + assert by_id["gone"]["deleted"] is True + assert by_id["gone"]["status"] == "deleted" + assert by_id["gone"]["totalCost"] == 9 + assert "title" not in by_id["gone"] diff --git a/frontend/ai.client/src/app/admin/costs/components/user-conversations.component.spec.ts b/frontend/ai.client/src/app/admin/costs/components/user-conversations.component.spec.ts index 1977435f..3cced21b 100644 --- a/frontend/ai.client/src/app/admin/costs/components/user-conversations.component.spec.ts +++ b/frontend/ai.client/src/app/admin/costs/components/user-conversations.component.spec.ts @@ -112,4 +112,24 @@ describe('UserConversationsComponent', () => { await vi.waitFor(() => expect(fixture.componentInstance.sessionsResource.error()).toBeTruthy()); expect(fixture.componentInstance.summaryLine()).toBe(''); }); + + it('lists deleted conversations and says their cost still counts', async () => { + const fixture = setup( + vi.fn().mockReturnValue( + of({ + ...RESPONSE, + total: 2, + deletedSessionCount: 1, + deletedSessionCost: 2.25, + sessions: [RESPONSE.sessions[0], { ...RESPONSE.sessions[0], sessionId: 'gone-1234-5678', status: 'deleted', totalCost: 2.25 }], + }), + ), + ); + const c = fixture.componentInstance; + await vi.waitFor(() => expect(c.sessionsResource.hasValue()).toBe(true)); + expect(c.summaryLine()).toBe('2 conversations · $5.12 recorded this month · 2 with unrecorded cost · 1 deleted ($2.25 still counted in the total)'); + // Deleted rows stay in the list, flagged, rather than being dropped. + const deleted = (c.sessionsResource.value()?.sessions ?? []).filter((s) => s.status === 'deleted'); + expect(deleted.map((s) => s.sessionId)).toEqual(['gone-1234-5678']); + }); }); diff --git a/frontend/ai.client/src/app/admin/costs/components/user-conversations.component.ts b/frontend/ai.client/src/app/admin/costs/components/user-conversations.component.ts index 7c9055bf..5a0ab267 100644 --- a/frontend/ai.client/src/app/admin/costs/components/user-conversations.component.ts +++ b/frontend/ai.client/src/app/admin/costs/components/user-conversations.component.ts @@ -133,6 +133,13 @@ import { @if (s.agentBound) { agent } + @if (s.status === 'deleted') { + deleted + } {{ s.lastMessageAt ? (s.lastMessageAt | date: 'MMM d, HH:mm') : '—' }} @@ -217,6 +224,13 @@ export class UserConversationsComponent { ]; if (v.userPeriodCost != null) parts.push(`${this.currency(v.userPeriodCost)} recorded this month`); if (v.unknownCostCount > 0) parts.push(`${v.unknownCostCount} with unrecorded cost`); + if ((v.deletedSessionCount ?? 0) > 0) { + // A delete is a tombstone, not a refund — say so, or the period total + // will not add up to the rows on screen. + parts.push( + `${v.deletedSessionCount} deleted (${this.currency(v.deletedSessionCost ?? 0)} still counted in the total)`, + ); + } return parts.join(' · '); }); diff --git a/frontend/ai.client/src/app/admin/costs/models/admin-cost.models.ts b/frontend/ai.client/src/app/admin/costs/models/admin-cost.models.ts index af0158d4..b5c17906 100644 --- a/frontend/ai.client/src/app/admin/costs/models/admin-cost.models.ts +++ b/frontend/ai.client/src/app/admin/costs/models/admin-cost.models.ts @@ -283,6 +283,13 @@ export interface UserSessionsResponse { total: number; /** Rows whose cost is unrecorded (listed, flagged, trailing under cost-sort). */ unknownCostCount: number; + /** + * Soft-deleted conversations in the list (`status === 'deleted'`). Listed, + * not hidden: a delete removes the row from the user's sidebar, not its cost + * rows or its share of `userPeriodCost`. + */ + deletedSessionCount?: number; + deletedSessionCost?: number; } export type UserSessionsSort = 'cost' | 'recent' | 'context' | 'messages';