Rifqi Hafizuddin Claude Fable 5 commited on
Commit
b8d1576
·
1 Parent(s): 54070ac

[KM-715] charts: GET /charts by message_id only + tri-state status marker

Browse files

Lead review round: lookup drops analysis_id (message_id is server-minted UUID,
globally unique); every response is HTTP 200 with status success|empty|
not_found + a human message. empty vs not_found is decided against the turn's
message_traceability row (PK lookup), so not_found reliably means "no completed
turn". Needs the additive idx_message_charts_message index (contract DDL note).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Files changed (2) hide show
  1. src/api/v1/charts.py +37 -11
  2. src/charts/store.py +33 -15
src/api/v1/charts.py CHANGED
@@ -1,18 +1,24 @@
1
  """Charts endpoint — user-facing chart retrieval for one turn (S2 visualization,
2
  SPINE_V2_PLAN §4.4/§4.5).
3
 
4
- `GET /api/v1/charts?analysis_id=&message_id=` returns every chart a `render_chart`
5
- tool call produced during one assistant turn, as `dataeyond.chart.v1` envelopes
6
  (SPINE_V2_PLAN §4.2). Rows are written by the chat pipeline right before the `done`
7
- SSE event; the FE fires this GET on `done`.
 
8
 
9
- Unlike `GET /api/v1/traceability`, an **empty list is a valid 200** here (most turns
10
- don't ask for a chart) rather than a 404 — once `done` has fired there is no "not
11
- ready yet" state to distinguish from "no charts this turn".
 
 
 
12
 
13
  No auth — Go fronts Python.
14
  """
15
 
 
 
16
  from fastapi import APIRouter, Query
17
  from pydantic import BaseModel
18
 
@@ -28,6 +34,8 @@ _store = PostgresChartStore()
28
 
29
 
30
  class ChartsResponse(BaseModel):
 
 
31
  count: int
32
  charts: list[ChartRecord]
33
 
@@ -35,12 +43,30 @@ class ChartsResponse(BaseModel):
35
  @router.get("/charts", response_model=ChartsResponse)
36
  @log_execution(logger)
37
  async def get_charts(
38
- analysis_id: str = Query(..., description="Analysis/session id"),
39
  message_id: str = Query(
40
  ..., description="Assistant turn id, taken from the `done` SSE event"
41
  ),
42
  ) -> ChartsResponse:
43
- """Fetch every chart for one turn. Empty list (not 404) when the turn produced
44
- none the FE can call this unconditionally on every `done`."""
45
- charts = await _store.list_for_message(analysis_id, message_id)
46
- return ChartsResponse(count=len(charts), charts=charts)
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
1
  """Charts endpoint — user-facing chart retrieval for one turn (S2 visualization,
2
  SPINE_V2_PLAN §4.4/§4.5).
3
 
4
+ `GET /api/v1/charts?message_id=` returns every chart a `render_chart` tool call
5
+ produced during one assistant turn, as `dataeyond.chart.v1` envelopes
6
  (SPINE_V2_PLAN §4.2). Rows are written by the chat pipeline right before the `done`
7
+ SSE event; the FE fires this GET on `done` with the `message_id` from that event.
8
+ Lookup is by `message_id` alone (Python-minted UUID4 — lead decision 2026-07-13).
9
 
10
+ Every response is HTTP 200 with an explicit `status` marker (lead ask 2026-07-13):
11
+ - `success` ≥1 chart; render them.
12
+ - `empty` — the turn completed but produced no charts (the common case).
13
+ - `not_found` — no completed turn is known for this message_id (mistyped/stale
14
+ id, or an error turn — those never write a row). Distinguished
15
+ from `empty` via the turn's traceability row.
16
 
17
  No auth — Go fronts Python.
18
  """
19
 
20
+ from typing import Literal
21
+
22
  from fastapi import APIRouter, Query
23
  from pydantic import BaseModel
24
 
 
34
 
35
 
36
  class ChartsResponse(BaseModel):
37
+ status: Literal["success", "empty", "not_found"]
38
+ message: str
39
  count: int
40
  charts: list[ChartRecord]
41
 
 
43
  @router.get("/charts", response_model=ChartsResponse)
44
  @log_execution(logger)
45
  async def get_charts(
 
46
  message_id: str = Query(
47
  ..., description="Assistant turn id, taken from the `done` SSE event"
48
  ),
49
  ) -> ChartsResponse:
50
+ """Fetch every chart for one turn. Always 200 the `status` field carries the
51
+ outcome, so the FE can call this unconditionally on every `done`."""
52
+ charts = await _store.list_for_message(message_id)
53
+ if charts:
54
+ return ChartsResponse(
55
+ status="success",
56
+ message=f"{len(charts)} chart(s) for this message.",
57
+ count=len(charts),
58
+ charts=charts,
59
+ )
60
+ if await _store.turn_exists(message_id):
61
+ return ChartsResponse(
62
+ status="empty",
63
+ message="This message completed without producing charts.",
64
+ count=0,
65
+ charts=[],
66
+ )
67
+ return ChartsResponse(
68
+ status="not_found",
69
+ message="No completed turn is known for this message_id.",
70
+ count=0,
71
+ charts=[],
72
+ )
src/charts/store.py CHANGED
@@ -27,7 +27,7 @@ from pydantic import BaseModel
27
  from sqlalchemy import select
28
 
29
  from src.db.postgres.connection import AsyncSessionLocal
30
- from src.db.postgres.models import MessageChartRow
31
  from src.middlewares.logging import get_logger
32
 
33
  logger = get_logger("charts_store")
@@ -48,7 +48,10 @@ class ChartStore(Protocol):
48
  """Persist + read `render_chart` outputs for one assistant `message_id`.
49
 
50
  `save` must never raise on the caller's path. `list_for_message` returns every
51
- chart for one turn (possibly empty — a turn need not have asked for a chart).
 
 
 
52
  """
53
 
54
  async def save(
@@ -61,9 +64,9 @@ class ChartStore(Protocol):
61
  envelope: dict,
62
  ) -> None: ...
63
 
64
- async def list_for_message(
65
- self, analysis_id: str, message_id: str
66
- ) -> list[ChartRecord]: ...
67
 
68
 
69
  class NullChartStore:
@@ -84,11 +87,12 @@ class NullChartStore:
84
  chart_type=envelope.get("chart_type", "unknown"),
85
  )
86
 
87
- async def list_for_message(
88
- self, analysis_id: str, message_id: str
89
- ) -> list[ChartRecord]:
90
  return []
91
 
 
 
 
92
 
93
  class PostgresChartStore:
94
  """Writes/reads `message_charts` rows. One insert per chart (not an upsert)."""
@@ -129,16 +133,15 @@ class PostgresChartStore:
129
  error=str(exc),
130
  )
131
 
132
- async def list_for_message(
133
- self, analysis_id: str, message_id: str
134
- ) -> list[ChartRecord]:
 
 
135
  async with AsyncSessionLocal() as session:
136
  result = await session.execute(
137
  select(MessageChartRow)
138
- .where(
139
- MessageChartRow.analysis_id == analysis_id,
140
- MessageChartRow.message_id == message_id,
141
- )
142
  .order_by(MessageChartRow.created_at)
143
  )
144
  rows = result.scalars().all()
@@ -152,3 +155,18 @@ class PostgresChartStore:
152
  )
153
  for row in rows
154
  ]
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
27
  from sqlalchemy import select
28
 
29
  from src.db.postgres.connection import AsyncSessionLocal
30
+ from src.db.postgres.models import MessageChartRow, MessageTraceabilityRow
31
  from src.middlewares.logging import get_logger
32
 
33
  logger = get_logger("charts_store")
 
48
  """Persist + read `render_chart` outputs for one assistant `message_id`.
49
 
50
  `save` must never raise on the caller's path. `list_for_message` returns every
51
+ chart for one turn (possibly empty — a turn need not have asked for a chart);
52
+ the lookup is by `message_id` alone (Python-minted UUID4, globally unique —
53
+ lead decision 2026-07-13). `turn_exists` says whether the turn flushed a
54
+ traceability row, so the endpoint can tell "no charts" from "unknown id".
55
  """
56
 
57
  async def save(
 
64
  envelope: dict,
65
  ) -> None: ...
66
 
67
+ async def list_for_message(self, message_id: str) -> list[ChartRecord]: ...
68
+
69
+ async def turn_exists(self, message_id: str) -> bool: ...
70
 
71
 
72
  class NullChartStore:
 
87
  chart_type=envelope.get("chart_type", "unknown"),
88
  )
89
 
90
+ async def list_for_message(self, message_id: str) -> list[ChartRecord]:
 
 
91
  return []
92
 
93
+ async def turn_exists(self, message_id: str) -> bool:
94
+ return False
95
+
96
 
97
  class PostgresChartStore:
98
  """Writes/reads `message_charts` rows. One insert per chart (not an upsert)."""
 
133
  error=str(exc),
134
  )
135
 
136
+ async def list_for_message(self, message_id: str) -> list[ChartRecord]:
137
+ # message_id-only lookup (lead decision 2026-07-13). NOTE for the Harry
138
+ # migration: the manual DDL's composite index (analysis_id, message_id)
139
+ # does not serve this predicate — an additive index on (message_id) is
140
+ # part of the handoff.
141
  async with AsyncSessionLocal() as session:
142
  result = await session.execute(
143
  select(MessageChartRow)
144
+ .where(MessageChartRow.message_id == message_id)
 
 
 
145
  .order_by(MessageChartRow.created_at)
146
  )
147
  rows = result.scalars().all()
 
155
  )
156
  for row in rows
157
  ]
158
+
159
+ async def turn_exists(self, message_id: str) -> bool:
160
+ """True iff the turn flushed its traceability row (written before `done`).
161
+
162
+ Lets the endpoint tri-state a zero-chart GET: `empty` (completed turn, no
163
+ charts — the common case) vs `not_found` (unknown/mistyped id, or an error
164
+ turn, which never writes traceability). PK lookup — cheap.
165
+ """
166
+ async with AsyncSessionLocal() as session:
167
+ result = await session.execute(
168
+ select(MessageTraceabilityRow.message_id).where(
169
+ MessageTraceabilityRow.message_id == message_id
170
+ )
171
+ )
172
+ return result.scalar_one_or_none() is not None