[NOTICKET] revert: drop F-3 user_id scoping on /charts + /traceability (lead decision)
Browse filesReverts the F-3 half of 0cb7d53. Rifqi restated the standing decision: traceability
does not take user_id, and by the same logic neither does charts. The message_id-only
lookup for /charts (lead decision 2026-07-13) and (analysis_id, message_id) for
/traceability stand as they were.
Reverted in full so nothing half-applied is left behind:
- both endpoints back to their pre-F-3 signatures, no optional query param, no
scoped=false logging
- PostgresChartStore.list_for_message / turn_exists back to message_id-only; the
ChartStore Protocol and NullChartStore likewise
- the contract's two FE-facing "becoming required" notes removed β no FE change is
being requested, and none should be
Deliberately KEPT (not part of F-3):
- degraded_seam="chart_persist" + repr(exc) on the chart-persist seam (#42)
- PostgresTraceabilityStore.get's optional user_id parameter, which predates this
work (shipped in fd4865b). Its docstring now records that the endpoint will NOT
pass it, so a future reader does not "finish" the wiring. Kept as dead capability
for a Go-forwarded identity (#43).
Accepted consequence, recorded rather than argued: both endpoints stay unauthenticated
over real customer data β charts[].spec.plotly.data is actual table values, and the
traceability payload carries 5-row previews, the executed SQL, and the owner's user_id.
The service-secret gate is now the ONLY control protecting them, which makes setting
dataeyond__service__secret (#37) load-bearing rather than merely advisable.
Docs: DEV_PLAN #40 struck through and marked RESOLVED/DECLINED with the reasoning;
CODE_REVIEW gains a "Declined by the lead" section so F-3 is not re-raised by a future
review pass.
Verification: ruff clean on all four touched files; `import main` OK; suite 424 passed
/ 0 failed / 7 skipped (unchanged β nothing depended on the reverted parameters).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- API_CONTRACT_BE_PYTHON.md +3 -27
- CODE_REVIEW_2026-07-23.md +6 -1
- DEV_PLAN.md +1 -1
- src/api/v1/charts.py +3 -29
- src/api/v1/traceability.py +2 -29
- src/charts/store.py +10 -33
- src/traceability/store.py +17 -6
|
@@ -487,21 +487,11 @@ Query params:
|
|
| 487 |
| --- | --- | --- |
|
| 488 |
| `analysis_id` | Yes | Analysis identifier. |
|
| 489 |
| `message_id` | Yes | Assistant answer identifier returned by the stream. |
|
| 490 |
-
| `user_id` | **No β becoming Yes** | Owner of the turn. See the tenant-scoping note below. |
|
| 491 |
-
|
| 492 |
-
> **Tenant scoping β added 2026-07-23, FE action requested.** `user_id` is **optional
|
| 493 |
-
> today**: send it and the lookup is scoped to the turn's owner; omit it and the
|
| 494 |
-
> response is byte-identical to before, so nothing breaks by not changing yet.
|
| 495 |
-
> **It will become required.** Please start sending it whenever the FE has it β
|
| 496 |
-
> the server logs every unscoped call, and that count is what tells us when the flip
|
| 497 |
-
> is safe. Until then this endpoint is an unauthenticated capability URL over real
|
| 498 |
-
> customer data: the payload carries 5-row previews of every retrieved result, the
|
| 499 |
-
> executed SQL, and the owner's `user_id`.
|
| 500 |
|
| 501 |
Example:
|
| 502 |
|
| 503 |
```text
|
| 504 |
-
GET /api/v1/traceability?analysis_id=an_42&message_id=msg_88f1
|
| 505 |
```
|
| 506 |
|
| 507 |
`intent` values the frontend may see: `chat` Β· `help` Β· `check` Β· `unstructured_flow` Β· `structured_flow` Β· `out_of_scope` Β· `blocked` (`blocked` = input-guard or Azure content-filter refusal; `chat` also covers the greeting fast-path and cache replays).
|
|
@@ -704,25 +694,11 @@ Query params:
|
|
| 704 |
| Query | Required | Description |
|
| 705 |
| --- | --- | --- |
|
| 706 |
| `message_id` | Yes | Assistant answer identifier returned by the stream's `done` event. |
|
| 707 |
-
| `user_id` | **No β becoming Yes** | Owner of the turn. See the tenant-scoping note below. |
|
| 708 |
-
|
| 709 |
-
> **Tenant scoping β added 2026-07-23, FE action requested.** `user_id` is **optional
|
| 710 |
-
> today**: send it and both the chart lookup and the `empty`/`not_found` check are
|
| 711 |
-
> scoped to the turn's owner; omit it and the response is byte-identical to before, so
|
| 712 |
-
> nothing breaks by not changing yet. **It will become required.** Please start sending
|
| 713 |
-
> it whenever the FE has it β the server logs every unscoped call, and that count is
|
| 714 |
-
> what tells us when the flip is safe.
|
| 715 |
-
>
|
| 716 |
-
> Why this matters more here than elsewhere: `charts[].spec.plotly.data` contains the
|
| 717 |
-
> **actual values from the customer's tables**, and the only thing currently protecting
|
| 718 |
-
> it is that `message_id` is a UUID4. A leaked id β an error report, a shared screenshot
|
| 719 |
-
> of a network tab, a support ticket β exposes that data indefinitely, with no expiry
|
| 720 |
-
> and no ownership check.
|
| 721 |
|
| 722 |
Example:
|
| 723 |
|
| 724 |
```text
|
| 725 |
-
GET /api/v1/charts?message_id=88f10c3a-6f03-4204-bf98-41ffc20388b2
|
| 726 |
```
|
| 727 |
|
| 728 |
The `dataeyond.chart.v1` envelope (the shape of `charts[].spec`, verbatim from `render_chart`):
|
|
@@ -799,7 +775,7 @@ Field rules:
|
|
| 799 |
- `spec` is the full `dataeyond.chart.v1` envelope, unmodified β it is the source of truth, not a projection; render straight from it.
|
| 800 |
- `chart_type` / `title` are copied out of `spec` for convenience (list rendering without parsing `spec`); `title` may be `null`.
|
| 801 |
- A turn can produce more than one chart (multiple `render_chart` calls in the same plan); `charts` is ordered by creation time.
|
| 802 |
-
- The
|
| 803 |
|
| 804 |
> **DDL note (Harry / dedorch migration):** the original manual index is `(analysis_id, message_id)`, which does not serve a `message_id`-only lookup. Additive index for the migration (also safe to run manually now):
|
| 805 |
> ```sql
|
|
|
|
| 487 |
| --- | --- | --- |
|
| 488 |
| `analysis_id` | Yes | Analysis identifier. |
|
| 489 |
| `message_id` | Yes | Assistant answer identifier returned by the stream. |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| 490 |
|
| 491 |
Example:
|
| 492 |
|
| 493 |
```text
|
| 494 |
+
GET /api/v1/traceability?analysis_id=an_42&message_id=msg_88f1
|
| 495 |
```
|
| 496 |
|
| 497 |
`intent` values the frontend may see: `chat` Β· `help` Β· `check` Β· `unstructured_flow` Β· `structured_flow` Β· `out_of_scope` Β· `blocked` (`blocked` = input-guard or Azure content-filter refusal; `chat` also covers the greeting fast-path and cache replays).
|
|
|
|
| 694 |
| Query | Required | Description |
|
| 695 |
| --- | --- | --- |
|
| 696 |
| `message_id` | Yes | Assistant answer identifier returned by the stream's `done` event. |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| 697 |
|
| 698 |
Example:
|
| 699 |
|
| 700 |
```text
|
| 701 |
+
GET /api/v1/charts?message_id=88f10c3a-6f03-4204-bf98-41ffc20388b2
|
| 702 |
```
|
| 703 |
|
| 704 |
The `dataeyond.chart.v1` envelope (the shape of `charts[].spec`, verbatim from `render_chart`):
|
|
|
|
| 775 |
- `spec` is the full `dataeyond.chart.v1` envelope, unmodified β it is the source of truth, not a projection; render straight from it.
|
| 776 |
- `chart_type` / `title` are copied out of `spec` for convenience (list rendering without parsing `spec`); `title` may be `null`.
|
| 777 |
- A turn can produce more than one chart (multiple `render_chart` calls in the same plan); `charts` is ordered by creation time.
|
| 778 |
+
- The payload carries no `user_id` / `analysis_id` β charts are keyed by `message_id` alone.
|
| 779 |
|
| 780 |
> **DDL note (Harry / dedorch migration):** the original manual index is `(analysis_id, message_id)`, which does not serve a `message_id`-only lookup. Additive index for the migration (also safe to run manually now):
|
| 781 |
> ```sql
|
|
@@ -32,7 +32,6 @@ Legend: β
shipped Β· π shipped, needs a deployment action Β· β¬ not starte
|
|
| 32 |
|
| 33 |
| Ref | Finding | Sev | Note |
|
| 34 |
|---|---|---|---|
|
| 35 |
-
| **F-3** | `/charts` + `/traceability` unscoped capability URLs | High | Store-side scoping done; endpoints need the parameter β **FE contract change** |
|
| 36 |
| **F-13** | Unbounded Parquet read (OOM escapes every seam) | High | Only finding that can kill the process |
|
| 37 |
| **F-12** | Unbounded planner catalog render | High | Measured 120k tokens @ 200Γ30; ship a *safety net*, not a tight cap |
|
| 38 |
| **F-9** | PII reaches traceability + report evidence | High | Decided: mask stored artifacts, assembler keeps values |
|
|
@@ -47,6 +46,12 @@ Legend: β
shipped Β· π shipped, needs a deployment action Β· β¬ not starte
|
|
| 47 |
| **F-8** | Planner/assembler prompts lack injection resistance | High | Gated on F-29 |
|
| 48 |
| **F-11**, **F-14**, **F-16**, **F-25**, **F-6**, **F-23**, **F-27**, **F-28** | Assorted | Med/Low | Batch opportunistically |
|
| 49 |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| 50 |
### Downgraded on evidence
|
| 51 |
|
| 52 |
| Ref | Was | Now | Why |
|
|
|
|
| 32 |
|
| 33 |
| Ref | Finding | Sev | Note |
|
| 34 |
|---|---|---|---|
|
|
|
|
| 35 |
| **F-13** | Unbounded Parquet read (OOM escapes every seam) | High | Only finding that can kill the process |
|
| 36 |
| **F-12** | Unbounded planner catalog render | High | Measured 120k tokens @ 200Γ30; ship a *safety net*, not a tight cap |
|
| 37 |
| **F-9** | PII reaches traceability + report evidence | High | Decided: mask stored artifacts, assembler keeps values |
|
|
|
|
| 46 |
| **F-8** | Planner/assembler prompts lack injection resistance | High | Gated on F-29 |
|
| 47 |
| **F-11**, **F-14**, **F-16**, **F-25**, **F-6**, **F-23**, **F-27**, **F-28** | Assorted | Med/Low | Batch opportunistically |
|
| 48 |
|
| 49 |
+
### Declined by the lead
|
| 50 |
+
|
| 51 |
+
| Ref | Finding | Decision |
|
| 52 |
+
|---|---|---|
|
| 53 |
+
| **F-3** | `/charts` + `/traceability` unscoped capability URLs | **DECLINED 2026-07-23 (Rifqi).** The `message_id`-only lookup for `/charts` and `(analysis_id, message_id)` for `/traceability` are standing lead decisions; F-3 asked to reopen them and the answer is no. An optional-`user_id` implementation was built and fully reverted the same day. **Accepted consequence:** both endpoints stay unauthenticated over real customer data, so the **service-secret gate (#37) is the sole control** β that makes setting `dataeyond__service__secret` load-bearing rather than merely advisable. Not a finding to re-raise. |
|
| 54 |
+
|
| 55 |
### Downgraded on evidence
|
| 56 |
|
| 57 |
| Ref | Was | Now | Why |
|
|
@@ -241,7 +241,7 @@ plus the live report bug on analysis `966224d4β¦`. Same status legend as Β§0.
|
|
| 241 |
| 37 | **F-2 service-secret gate** β `X-Dataeyond-Service-Secret`, router-level dependency | Rifqi | π | Code shipped 2026-07-23, **inert until `dataeyond__service__secret` is set**. Go makes no outbound call to Python (verified in the Go source), so the "Go fronts Python" premise in the code comments is not wired and the surface is currently open. **Action: Rifqi sets the secret on the HF Space + the FE/Go caller, then re-verify** |
|
| 242 |
| 38 | **F-1 tenant scoping** β `user_id` predicate on the six analysis-keyed reads | Rifqi | β
| `CatalogStore.get_by_analysis` filtered on `analysis_id` alone where Go filters on both; the catalog payload carries the owner's `user_id`, so `DbExecutor`'s ownership check compared the victim's id against itself and passed β cross-tenant **query execution against a customer DB**. Defence-in-depth only until #37 is armed (`user_id` is caller-supplied, and `GET /traceability` leaks it) |
|
| 243 |
| 39 | **Stale tests resolved** β the two long-standing suite failures | Rifqi | β
| Both encoded the pre-2026-07-13 user-scope fallback that `reader.py` deliberately removed. Not product bugs. Suite is now **394 passed / 0 failed / 7 skipped** β fully green for the first time |
|
| 244 |
-
| 40 | **F-3** β scope `GET /charts` + `GET /traceability` by `user_id` | Rifqi
|
| 245 |
| 41 | **F-12 / F-13** β bound the planner catalog render and the tabular blob read | Rifqi | β
| Shipped 2026-07-23. **F-13:** neither storage backend could report an object size, so `object_size()` was added to both (S3 `head_object`β`ContentLength`, Azure `get_blob_properties().size`, each returning None rather than raising) and `TabularExecutor` now refuses >500 MB **before** downloading, with a post-download byte check as the fallback when the probe is unavailable. **F-12:** `render()` gained TWO ceilings β `_MAX_TABLES=150` and `_MAX_CATALOG_CHARS=250_000` β because a table-count cap alone leaves wide tables unbounded (20 tables Γ 300 cols is as fatal as 400 Γ 30). Measured after: 400Γ30 went from ~241k tokens to ~63k; 100Γ30 (~45k tokens) still renders **in full**, so no realistic catalog is touched. Truncation emits an explicit "N more tables not shown" line so the planner knows it saw a subset, and logs β that log is the signal to retune. 12 new tests |
|
| 246 |
| 42 | **F-20 observability** β `degraded_seam=<name>` on every never-throw / silent-drop path | Rifqi | π | The 2026-07-23 report bug was invisible by construction: the record was dropped with zero logging. **Partially shipped 2026-07-23** β the 10 seams where silent degradation is user-visible now emit a stable `degraded_seam` field (+ `repr(e)` instead of `str(e)`, so an empty-`str()` Fernet error is no longer a blank log): `input_guard_fail_open`, `analysis_catalog_read`, `report_floor_record_read`, `traceability_persist`, `traceability_flush`, `chart_persist` (Γ2), `report_input_persist` (Γ2), `analysis_state_ensure`. **Remaining:** the other ~76 `except Exception` sites, most of which are in unwired routers (`db_client`, `data_catalog`, `users`) or non-live paths β deliberately not swept, since a blanket edit across unwired code is exactly the drive-by Β§7A forbids. Control flow unchanged throughout (Β§5.4) |
|
| 247 |
| 43 | **Go identity contract** β what does Go forward, and when? | Rifqi β Harry | β¬ new | Needed to replace #37's shared secret with real per-user authorization. Until then the store predicates in #38 are defensive only |
|
|
|
|
| 241 |
| 37 | **F-2 service-secret gate** β `X-Dataeyond-Service-Secret`, router-level dependency | Rifqi | π | Code shipped 2026-07-23, **inert until `dataeyond__service__secret` is set**. Go makes no outbound call to Python (verified in the Go source), so the "Go fronts Python" premise in the code comments is not wired and the surface is currently open. **Action: Rifqi sets the secret on the HF Space + the FE/Go caller, then re-verify** |
|
| 242 |
| 38 | **F-1 tenant scoping** β `user_id` predicate on the six analysis-keyed reads | Rifqi | β
| `CatalogStore.get_by_analysis` filtered on `analysis_id` alone where Go filters on both; the catalog payload carries the owner's `user_id`, so `DbExecutor`'s ownership check compared the victim's id against itself and passed β cross-tenant **query execution against a customer DB**. Defence-in-depth only until #37 is armed (`user_id` is caller-supplied, and `GET /traceability` leaks it) |
|
| 243 |
| 39 | **Stale tests resolved** β the two long-standing suite failures | Rifqi | β
| Both encoded the pre-2026-07-13 user-scope fallback that `reader.py` deliberately removed. Not product bugs. Suite is now **394 passed / 0 failed / 7 skipped** β fully green for the first time |
|
| 244 |
+
| 40 | ~~**F-3** β scope `GET /charts` + `GET /traceability` by `user_id`~~ | Rifqi | β | **RESOLVED 2026-07-23 β DECLINED by Rifqi. Do not re-open without his sign-off.** Both endpoints keep their existing lookup keys: `/traceability` by `(analysis_id, message_id)`, `/charts` by `message_id` alone (the 2026-07-13 lead decision). F-3 proposed adding a `user_id` parameter to both; an optional-param version was implemented on 2026-07-23 and then **reverted in full** (endpoints, charts-store predicates, and the contract notes) once the decision was restated β no FE change is required and none should be requested. **Accepted consequence:** both endpoints remain unauthenticated capability URLs over real customer data (`charts[].spec.plotly.data` is actual table values; the traceability payload carries 5-row previews, the executed SQL, and the owner's `user_id`). **The service-secret gate (#37) is therefore the only control protecting them** β which raises #37 from important to load-bearing. The stores' optional `user_id` parameters are kept as dead capability for a future Go-forwarded identity (#43); `PostgresChartStore` was returned to its message_id-only form. |
|
| 245 |
| 41 | **F-12 / F-13** β bound the planner catalog render and the tabular blob read | Rifqi | β
| Shipped 2026-07-23. **F-13:** neither storage backend could report an object size, so `object_size()` was added to both (S3 `head_object`β`ContentLength`, Azure `get_blob_properties().size`, each returning None rather than raising) and `TabularExecutor` now refuses >500 MB **before** downloading, with a post-download byte check as the fallback when the probe is unavailable. **F-12:** `render()` gained TWO ceilings β `_MAX_TABLES=150` and `_MAX_CATALOG_CHARS=250_000` β because a table-count cap alone leaves wide tables unbounded (20 tables Γ 300 cols is as fatal as 400 Γ 30). Measured after: 400Γ30 went from ~241k tokens to ~63k; 100Γ30 (~45k tokens) still renders **in full**, so no realistic catalog is touched. Truncation emits an explicit "N more tables not shown" line so the planner knows it saw a subset, and logs β that log is the signal to retune. 12 new tests |
|
| 246 |
| 42 | **F-20 observability** β `degraded_seam=<name>` on every never-throw / silent-drop path | Rifqi | π | The 2026-07-23 report bug was invisible by construction: the record was dropped with zero logging. **Partially shipped 2026-07-23** β the 10 seams where silent degradation is user-visible now emit a stable `degraded_seam` field (+ `repr(e)` instead of `str(e)`, so an empty-`str()` Fernet error is no longer a blank log): `input_guard_fail_open`, `analysis_catalog_read`, `report_floor_record_read`, `traceability_persist`, `traceability_flush`, `chart_persist` (Γ2), `report_input_persist` (Γ2), `analysis_state_ensure`. **Remaining:** the other ~76 `except Exception` sites, most of which are in unwired routers (`db_client`, `data_catalog`, `users`) or non-live paths β deliberately not swept, since a blanket edit across unwired code is exactly the drive-by Β§7A forbids. Control flow unchanged throughout (Β§5.4) |
|
| 247 |
| 43 | **Go identity contract** β what does Go forward, and when? | Rifqi β Harry | β¬ new | Needed to replace #37's shared secret with real per-user authorization. Until then the store predicates in #38 are defensive only |
|
|
@@ -14,17 +14,7 @@ Every response is HTTP 200 with an explicit `status` marker (lead ask 2026-07-13
|
|
| 14 |
id, or an error turn β those never write a row). Distinguished
|
| 15 |
from `empty` via the turn's traceability row.
|
| 16 |
|
| 17 |
-
|
| 18 |
-
rollout: supplied, it scopes both reads to the turn's owner; omitted, the lookup stays
|
| 19 |
-
message_id-only and the response is identical to before, so no FE change is needed to
|
| 20 |
-
keep working. This is deliberately the same log-only-then-enforce pattern used for the
|
| 21 |
-
catalog tenant predicate β every unscoped call logs `scoped=false`, which is the signal
|
| 22 |
-
for when it is safe to make the parameter required.
|
| 23 |
-
|
| 24 |
-
Until it IS required this endpoint remains a capability URL: `spec.plotly.data` carries
|
| 25 |
-
the customer's real values, so anyone holding a leaked message_id can read them. Making
|
| 26 |
-
`user_id` required is an FE contract change (DEV_PLAN #40); the service-secret gate
|
| 27 |
-
(#37) is the control that matters in the meantime.
|
| 28 |
"""
|
| 29 |
|
| 30 |
from typing import Literal
|
|
@@ -56,26 +46,10 @@ async def get_charts(
|
|
| 56 |
message_id: str = Query(
|
| 57 |
..., description="Assistant turn id, taken from the `done` SSE event"
|
| 58 |
),
|
| 59 |
-
user_id: str | None = Query(
|
| 60 |
-
None,
|
| 61 |
-
description=(
|
| 62 |
-
"Owner of the turn. Optional during the F-3 rollout β when supplied the "
|
| 63 |
-
"lookup is scoped to this user; when omitted the response is unchanged. "
|
| 64 |
-
"Send it as soon as the FE can; it becomes required (DEV_PLAN #40)."
|
| 65 |
-
),
|
| 66 |
-
),
|
| 67 |
) -> ChartsResponse:
|
| 68 |
"""Fetch every chart for one turn. Always 200 β the `status` field carries the
|
| 69 |
outcome, so the FE can call this unconditionally on every `done`."""
|
| 70 |
-
|
| 71 |
-
# The rollout signal: when this stops appearing, the parameter can be
|
| 72 |
-
# made required without breaking a live caller.
|
| 73 |
-
logger.warning(
|
| 74 |
-
"charts read unscoped β no user_id supplied",
|
| 75 |
-
message_id=message_id,
|
| 76 |
-
scoped=False,
|
| 77 |
-
)
|
| 78 |
-
charts = await _store.list_for_message(message_id, user_id)
|
| 79 |
if charts:
|
| 80 |
return ChartsResponse(
|
| 81 |
status="success",
|
|
@@ -83,7 +57,7 @@ async def get_charts(
|
|
| 83 |
count=len(charts),
|
| 84 |
charts=charts,
|
| 85 |
)
|
| 86 |
-
if await _store.turn_exists(message_id
|
| 87 |
return ChartsResponse(
|
| 88 |
status="empty",
|
| 89 |
message="This message completed without producing charts.",
|
|
|
|
| 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
|
|
|
|
| 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",
|
|
|
|
| 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.",
|
|
@@ -7,17 +7,7 @@ chat pipeline right before the `done` SSE event; the FE fires this GET on `done`
|
|
| 7 |
|
| 8 |
Renamed from the contracted `/api/v1/observability` (team decision 2026-07-06) so it is
|
| 9 |
never confused with the Langfuse *observability* stack (engineering-only, PII-masked).
|
| 10 |
-
|
| 11 |
-
**Tenant scoping (F-3, 2026-07-23).** `user_id` is an OPTIONAL query parameter in this
|
| 12 |
-
rollout: supplied, it scopes the read to the turn's owner; omitted, behaviour is exactly
|
| 13 |
-
as before, so no FE change is needed to keep working. Every unscoped call logs
|
| 14 |
-
`scoped=false` β that count is the evidence for when the parameter can safely be made
|
| 15 |
-
required (an FE contract change, DEV_PLAN #40).
|
| 16 |
-
|
| 17 |
-
Note what an unscoped read exposes: the payload carries 5-row previews of every
|
| 18 |
-
`retrieve_data` result, the executed SQL, AND the owner's `user_id` β which is the id an
|
| 19 |
-
attacker would need to satisfy every other tenant predicate. The service-secret gate
|
| 20 |
-
(#37) is the control that matters until scoping is required.
|
| 21 |
"""
|
| 22 |
|
| 23 |
from fastapi import APIRouter, HTTPException, Query
|
|
@@ -40,28 +30,11 @@ async def get_traceability(
|
|
| 40 |
message_id: str = Query(
|
| 41 |
..., description="Assistant turn id, taken from the `done` SSE event"
|
| 42 |
),
|
| 43 |
-
user_id: str | None = Query(
|
| 44 |
-
None,
|
| 45 |
-
description=(
|
| 46 |
-
"Owner of the turn. Optional during the F-3 rollout β when supplied the "
|
| 47 |
-
"lookup is scoped to this user; when omitted the response is unchanged. "
|
| 48 |
-
"Send it as soon as the FE can; it becomes required (DEV_PLAN #40)."
|
| 49 |
-
),
|
| 50 |
-
),
|
| 51 |
) -> TraceabilityPayload:
|
| 52 |
"""Fetch one turn's provenance record. 404 while the turn is still running or if
|
| 53 |
the id is unknown (the FE never gets a `message_id` for error turns, so 404 is
|
| 54 |
the correct answer there)."""
|
| 55 |
-
|
| 56 |
-
# The rollout signal: when this stops appearing, the parameter can be
|
| 57 |
-
# made required without breaking a live caller.
|
| 58 |
-
logger.warning(
|
| 59 |
-
"traceability read unscoped β no user_id supplied",
|
| 60 |
-
analysis_id=analysis_id,
|
| 61 |
-
message_id=message_id,
|
| 62 |
-
scoped=False,
|
| 63 |
-
)
|
| 64 |
-
payload = await _store.get(analysis_id, message_id, user_id)
|
| 65 |
if payload is None:
|
| 66 |
raise HTTPException(
|
| 67 |
status_code=404,
|
|
|
|
| 7 |
|
| 8 |
Renamed from the contracted `/api/v1/observability` (team decision 2026-07-06) so it is
|
| 9 |
never confused with the Langfuse *observability* stack (engineering-only, PII-masked).
|
| 10 |
+
No auth β Go fronts Python.
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| 11 |
"""
|
| 12 |
|
| 13 |
from fastapi import APIRouter, HTTPException, Query
|
|
|
|
| 30 |
message_id: str = Query(
|
| 31 |
..., description="Assistant turn id, taken from the `done` SSE event"
|
| 32 |
),
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| 33 |
) -> TraceabilityPayload:
|
| 34 |
"""Fetch one turn's provenance record. 404 while the turn is still running or if
|
| 35 |
the id is unknown (the FE never gets a `message_id` for error turns, so 404 is
|
| 36 |
the correct answer there)."""
|
| 37 |
+
payload = await _store.get(analysis_id, message_id)
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| 38 |
if payload is None:
|
| 39 |
raise HTTPException(
|
| 40 |
status_code=404,
|
|
@@ -64,13 +64,9 @@ class ChartStore(Protocol):
|
|
| 64 |
envelope: dict,
|
| 65 |
) -> None: ...
|
| 66 |
|
| 67 |
-
async def list_for_message(
|
| 68 |
-
self, message_id: str, user_id: str | None = None
|
| 69 |
-
) -> list[ChartRecord]: ...
|
| 70 |
|
| 71 |
-
async def turn_exists(
|
| 72 |
-
self, message_id: str, user_id: str | None = None
|
| 73 |
-
) -> bool: ...
|
| 74 |
|
| 75 |
|
| 76 |
class NullChartStore:
|
|
@@ -91,12 +87,10 @@ class NullChartStore:
|
|
| 91 |
chart_type=envelope.get("chart_type", "unknown"),
|
| 92 |
)
|
| 93 |
|
| 94 |
-
async def list_for_message(
|
| 95 |
-
self, message_id: str, user_id: str | None = None
|
| 96 |
-
) -> list[ChartRecord]:
|
| 97 |
return []
|
| 98 |
|
| 99 |
-
async def turn_exists(self, message_id: str
|
| 100 |
return False
|
| 101 |
|
| 102 |
|
|
@@ -140,27 +134,15 @@ class PostgresChartStore:
|
|
| 140 |
error=repr(exc),
|
| 141 |
)
|
| 142 |
|
| 143 |
-
async def list_for_message(
|
| 144 |
-
self, message_id: str, user_id: str | None = None
|
| 145 |
-
) -> list[ChartRecord]:
|
| 146 |
# message_id-only lookup (lead decision 2026-07-13). NOTE for the Harry
|
| 147 |
# migration: the manual DDL's composite index (analysis_id, message_id)
|
| 148 |
# does not serve this predicate β an additive index on (message_id) is
|
| 149 |
# part of the handoff.
|
| 150 |
-
#
|
| 151 |
-
# `user_id` scopes the read to the turn's owner (F-3, 2026-07-23). Optional
|
| 152 |
-
# because `GET /api/v1/charts` does not require the parameter yet β making it
|
| 153 |
-
# required is an FE contract change. Until the FE sends it, an unscoped read
|
| 154 |
-
# means the chart data (which is REAL customer values, in `spec.plotly.data`)
|
| 155 |
-
# is reachable by anyone holding the message_id. See the endpoint's
|
| 156 |
-
# `scoped=false` log line.
|
| 157 |
async with AsyncSessionLocal() as session:
|
| 158 |
-
where = [MessageChartRow.message_id == message_id]
|
| 159 |
-
if user_id is not None:
|
| 160 |
-
where.append(MessageChartRow.user_id == user_id)
|
| 161 |
result = await session.execute(
|
| 162 |
select(MessageChartRow)
|
| 163 |
-
.where(
|
| 164 |
.order_by(MessageChartRow.created_at)
|
| 165 |
)
|
| 166 |
rows = result.scalars().all()
|
|
@@ -175,22 +157,17 @@ class PostgresChartStore:
|
|
| 175 |
for row in rows
|
| 176 |
]
|
| 177 |
|
| 178 |
-
async def turn_exists(self, message_id: str
|
| 179 |
"""True iff the turn flushed its traceability row (written before `done`).
|
| 180 |
|
| 181 |
Lets the endpoint tri-state a zero-chart GET: `empty` (completed turn, no
|
| 182 |
charts β the common case) vs `not_found` (unknown/mistyped id, or an error
|
| 183 |
turn, which never writes traceability). PK lookup β cheap.
|
| 184 |
-
|
| 185 |
-
Scoped by `user_id` when the caller supplies one (F-3, 2026-07-23), so another
|
| 186 |
-
tenant's turn reads as `not_found` rather than `empty` β the tri-state must not
|
| 187 |
-
become a probe that confirms a message_id exists.
|
| 188 |
"""
|
| 189 |
async with AsyncSessionLocal() as session:
|
| 190 |
-
where = [MessageTraceabilityRow.message_id == message_id]
|
| 191 |
-
if user_id is not None:
|
| 192 |
-
where.append(MessageTraceabilityRow.user_id == user_id)
|
| 193 |
result = await session.execute(
|
| 194 |
-
select(MessageTraceabilityRow.message_id).where(
|
|
|
|
|
|
|
| 195 |
)
|
| 196 |
return result.scalar_one_or_none() is not None
|
|
|
|
| 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 |
|
|
|
|
| 134 |
error=repr(exc),
|
| 135 |
)
|
| 136 |
|
| 137 |
+
async def list_for_message(self, message_id: str) -> list[ChartRecord]:
|
|
|
|
|
|
|
| 138 |
# message_id-only lookup (lead decision 2026-07-13). NOTE for the Harry
|
| 139 |
# migration: the manual DDL's composite index (analysis_id, message_id)
|
| 140 |
# does not serve this predicate β an additive index on (message_id) is
|
| 141 |
# part of the handoff.
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| 142 |
async with AsyncSessionLocal() as session:
|
|
|
|
|
|
|
|
|
|
| 143 |
result = await session.execute(
|
| 144 |
select(MessageChartRow)
|
| 145 |
+
.where(MessageChartRow.message_id == message_id)
|
| 146 |
.order_by(MessageChartRow.created_at)
|
| 147 |
)
|
| 148 |
rows = result.scalars().all()
|
|
|
|
| 157 |
for row in rows
|
| 158 |
]
|
| 159 |
|
| 160 |
+
async def turn_exists(self, message_id: str) -> bool:
|
| 161 |
"""True iff the turn flushed its traceability row (written before `done`).
|
| 162 |
|
| 163 |
Lets the endpoint tri-state a zero-chart GET: `empty` (completed turn, no
|
| 164 |
charts β the common case) vs `not_found` (unknown/mistyped id, or an error
|
| 165 |
turn, which never writes traceability). PK lookup β cheap.
|
|
|
|
|
|
|
|
|
|
|
|
|
| 166 |
"""
|
| 167 |
async with AsyncSessionLocal() as session:
|
|
|
|
|
|
|
|
|
|
| 168 |
result = await session.execute(
|
| 169 |
+
select(MessageTraceabilityRow.message_id).where(
|
| 170 |
+
MessageTraceabilityRow.message_id == message_id
|
| 171 |
+
)
|
| 172 |
)
|
| 173 |
return result.scalar_one_or_none() is not None
|
|
@@ -100,12 +100,23 @@ class PostgresTraceabilityStore:
|
|
| 100 |
) -> TraceabilityPayload | None:
|
| 101 |
"""One turn's payload, or None on a miss.
|
| 102 |
|
| 103 |
-
`user_id` scopes the read to the turn's owner
|
| 104 |
-
|
| 105 |
-
|
| 106 |
-
|
| 107 |
-
|
| 108 |
-
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| 109 |
"""
|
| 110 |
async with AsyncSessionLocal() as session:
|
| 111 |
where = [
|
|
|
|
| 100 |
) -> TraceabilityPayload | None:
|
| 101 |
"""One turn's payload, or None on a miss.
|
| 102 |
|
| 103 |
+
`user_id` scopes the read to the turn's owner when a caller supplies one.
|
| 104 |
+
|
| 105 |
+
**It stays unused by the endpoint β lead decision, reaffirmed 2026-07-23.**
|
| 106 |
+
`GET /api/v1/traceability` looks up by `(analysis_id, message_id)` alone, and
|
| 107 |
+
`GET /api/v1/charts` by `message_id` alone (the 2026-07-13 decision). Review
|
| 108 |
+
finding F-3 proposed adding the parameter to both; that was **declined**, so
|
| 109 |
+
do not "finish" this by wiring it into the endpoint β reopening it needs the
|
| 110 |
+
same sign-off path as any other locked decision.
|
| 111 |
+
|
| 112 |
+
The consequence is deliberate and accepted: this row is reachable by
|
| 113 |
+
`(analysis_id, message_id)` alone, and the payload itself carries `user_id`.
|
| 114 |
+
**The service-secret gate is the control that protects these endpoints**, not
|
| 115 |
+
this predicate β which is why `dataeyond__service__secret` being set matters
|
| 116 |
+
more than it would otherwise (DEV_PLAN #37).
|
| 117 |
+
|
| 118 |
+
The parameter is kept because it costs nothing and a future Go-forwarded
|
| 119 |
+
identity (#43) would use it.
|
| 120 |
"""
|
| 121 |
async with AsyncSessionLocal() as session:
|
| 122 |
where = [
|