Rifqi Hafizuddin Claude Opus 4.8 commited on
Commit
09f8451
·
1 Parent(s): b701e95

[NOTICKET] feat(slow-path): persist AnalysisRecord via PostgresAnalysisStore

Browse files

Groundwork for generate_report - the slow path now persists a real, citable
record that the report (next) renders from.

Contract gaps (schemas.py):
- stage: CrispStage added to TaskResult + TaskSummary, populated at all 3
TaskResult build sites (task_runner) + copied in assembler._build_record,
so the report can group its method appendix by CRISP-DM phase.
- AnalysisRecord gained identity: record_id (auto uuid), analysis_id/user_id
(optional; stamped at persist time).

Persistence:
- PostgresAnalysisStore (store.py): save() never-throw idempotent upsert +
list_for_analysis() oldest-first (the report render order). NullAnalysisStore
kept for tests. AnalysisStore Protocol gained list_for_analysis.
- analysis_records jsonb table (models.py), indexed by analysis_id + user_id;
registered in init_db, created by create_all on startup (no migration).
- chat_handler default store -> PostgresAnalysisStore; user_id stamped at save.

Open: analysis_id is NULL until the Analysis State reaches the slow path
(session-id handoff from Harry needed to group records per analysis).

Docs: checkpoint plan + PROGRESS updated.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

CHECKPOINT_PLAN_2026-06-17.md CHANGED
@@ -22,6 +22,8 @@ Dated snapshot of what landed this session. Live task status (incl. what's left)
22
  - ✅ **Planner tool list verified** against the trimmed registry — no references to old tool names or deferred analytics anywhere in `src/` (Rifqi).
23
  - 📌 **Decision:** `tests/` stays gitignored — team decided not to push tests to origin (closes PROGRESS.md R3 as won't-do).
24
  - 📌 **Ownership:** Rifqi owns `generate_report` development + the `analysis_records` table / real `AnalysisStore` (contract still co-designed with Sofhia).
 
 
25
 
26
  ---
27
 
@@ -68,8 +70,12 @@ Dated snapshot of what landed this session. Live task status (incl. what's left)
68
 
69
  ### Rifqi
70
  - [x] **Redis Cloud free tier** (~30–50 MB): create instance, set TTL = 1 h, share env vars in the group. (done 12 Jun)
71
- - [ ] **Own `generate_report` development** (contract co-designed with Sofhia, see §3) — includes the `analysis_records` table + real `AnalysisStore` (replaces `NullAnalysisStore`); required for the DoD "record persisted" step.
 
 
 
72
  - [x] Verify planner tool list matches the trimmed registry (4 analytics + 4 data/knowledge) and few-shots don't reference removed tools. (verified 12 Jun — no stale tool names in `src/`)
 
73
 
74
  ### Shared (Sofhia + Rifqi)
75
  - [ ] `generate_report` design + skeleton: input = AnalysisRecords for the session + Problem Statement from Analysis State; output = versioned artifact; bumps `report_status`. Agree on the contract even if rendering is stubbed for Wednesday. (Development: Rifqi.)
 
22
  - ✅ **Planner tool list verified** against the trimmed registry — no references to old tool names or deferred analytics anywhere in `src/` (Rifqi).
23
  - 📌 **Decision:** `tests/` stays gitignored — team decided not to push tests to origin (closes PROGRESS.md R3 as won't-do).
24
  - 📌 **Ownership:** Rifqi owns `generate_report` development + the `analysis_records` table / real `AnalysisStore` (contract still co-designed with Sofhia).
25
+ - ✅ **R5 cache fix** (Rifqi, `b701e95`): chat cache scoped by `user_id`, TTL 24h→1h.
26
+ - ✅ **AnalysisRecord persistence landed** (Rifqi): `stage` now flows to the record (CRISP-DM grouping for the report) + identity fields (`record_id`/`analysis_id`/`user_id`); `PostgresAnalysisStore` + `analysis_records` table replace `NullAnalysisStore`, wired into `ChatHandler`. Unblocks the `generate_report` renderer and the DoD "record persisted" step. Open: `analysis_id` handoff from Harry's Analysis State.
27
 
28
  ---
29
 
 
70
 
71
  ### Rifqi
72
  - [x] **Redis Cloud free tier** (~30–50 MB): create instance, set TTL = 1 h, share env vars in the group. (done 12 Jun)
73
+ - [x] **R5 cache fix**: chat cache key scoped by `user_id`, TTL 24h→1h (urgent on shared Redis). (12 Jun, commit `b701e95`)
74
+ - [x] **AnalysisRecord contract gaps closed**: `stage` (CRISP-DM) now flows Task→TaskResult→TaskSummary so the report can group the method appendix; `AnalysisRecord` gained `record_id`/`analysis_id`/`user_id` identity fields. (12 Jun)
75
+ - [x] **`analysis_records` table + real `AnalysisStore`**: `PostgresAnalysisStore` (save + `list_for_analysis`, never-throw) replaces `NullAnalysisStore`; wired into `ChatHandler`, `user_id` stamped at save. Satisfies the DoD "record persisted" step. (12 Jun)
76
+ - [ ] **Own `generate_report` development** (contract co-designed with Sofhia, see §3) — reads `analysis_records` + Problem Statement; versioned, bumps `report_status`. *(persistence half done above; renderer + contract doc next)*
77
  - [x] Verify planner tool list matches the trimmed registry (4 analytics + 4 data/knowledge) and few-shots don't reference removed tools. (verified 12 Jun — no stale tool names in `src/`)
78
+ - ⚠️ **Blocked-on-Harry**: `analysis_id` is `NULL` on persisted records until the Analysis State reaches the slow path — need the session-ID handoff so `generate_report` can group records per analysis.
79
 
80
  ### Shared (Sofhia + Rifqi)
81
  - [ ] `generate_report` design + skeleton: input = AnalysisRecords for the session + Problem Statement from Analysis State; output = versioned artifact; bumps `report_status`. Agree on the contract even if rendering is stubbed for Wednesday. (Development: Rifqi.)
PROGRESS.md CHANGED
@@ -2,11 +2,35 @@
2
 
3
  Persistent tracker mirroring the 42-item ownership table in `REPO_CONTEXT.md` "Team — division of work". Update as PRs land. Future Claude Code sessions read this to know what's already done.
4
 
5
- **Last updated**: 2026-06-12 (Redis Cloud live; R3 closed as won't-do per team decision; Rifqi owns `analysis_records`/`AnalysisStore` + `generate_report`)
6
  **Current open PR**: `pr/3` — active.
7
 
8
  ---
9
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
10
  ## Principal architecture review (2026-06-10) — findings + fix tracker
11
 
12
  A full external review (read the context docs + the slow path, tool layer, query
 
2
 
3
  Persistent tracker mirroring the 42-item ownership table in `REPO_CONTEXT.md` "Team — division of work". Update as PRs land. Future Claude Code sessions read this to know what's already done.
4
 
5
+ **Last updated**: 2026-06-12 (Redis Cloud live; R3 closed as won't-do; R5 cache fix; AnalysisRecord persistence landed — `PostgresAnalysisStore` + `analysis_records` table)
6
  **Current open PR**: `pr/3` — active.
7
 
8
  ---
9
 
10
+ ## What just shipped (2026-06-12 — AnalysisRecord persistence, Rifqi)
11
+
12
+ Groundwork for `generate_report`. The slow path now persists a real, citable
13
+ record; the report (next) renders from it.
14
+
15
+ - **Contract gaps closed** (`agents/slow_path/schemas.py`): `stage: CrispStage`
16
+ added to `TaskResult` + `TaskSummary` and populated at all 3 `TaskResult` build
17
+ sites in `task_runner.py` + copied in `assembler._build_record` — so the report
18
+ can group its method appendix by CRISP-DM phase. `AnalysisRecord` gained identity:
19
+ `record_id` (auto uuid), `analysis_id`/`user_id` (optional; stamped at persist).
20
+ - **Real store** (`agents/slow_path/store.py`): `PostgresAnalysisStore` —
21
+ `save()` (never-throw, idempotent upsert) + `list_for_analysis()` (oldest-first,
22
+ the report's render order). `NullAnalysisStore` kept (tests / disabled persistence).
23
+ `AnalysisStore` Protocol gained `list_for_analysis`.
24
+ - **Table** (`db/postgres/models.py`): `analysis_records` jsonb table (one row per
25
+ run, indexed by `analysis_id` + `user_id`); registered in `init_db.py`, created by
26
+ `create_all` on startup (no migration — `data_catalog` precedent).
27
+ - **Wired** (`agents/chat_handler.py`): default store flipped to `PostgresAnalysisStore`;
28
+ `user_id` stamped onto the record at the save site (in scope there).
29
+ - **Open**: `analysis_id` is `NULL` until Harry's Analysis State reaches the slow
30
+ path (session-ID handoff needed to group records per analysis).
31
+
32
+ ---
33
+
34
  ## Principal architecture review (2026-06-10) — findings + fix tracker
35
 
36
  A full external review (read the context docs + the slow path, tool layer, query
src/agents/chat_handler.py CHANGED
@@ -282,9 +282,9 @@ class ChatHandler:
282
 
283
  def _get_analysis_store(self) -> AnalysisStore:
284
  if self._analysis_store is None:
285
- from .slow_path.store import NullAnalysisStore
286
 
287
- self._analysis_store = NullAnalysisStore()
288
  return self._analysis_store
289
 
290
  async def _run_slow_path(
@@ -368,7 +368,10 @@ class ChatHandler:
368
  yield {"event": "sources", "data": json.dumps([])} # TODO: derive from record
369
  yield {"event": "chunk", "data": result.chat_answer}
370
  try:
371
- await self._get_analysis_store().save(result.analysis_record)
 
 
 
372
  except Exception as e: # persistence must never break the user's answer
373
  logger.error("analysis_record persist failed", user_id=user_id, error=str(e))
374
  tracer.end() # output omitted (chat_answer may contain PII on Cloud)
 
282
 
283
  def _get_analysis_store(self) -> AnalysisStore:
284
  if self._analysis_store is None:
285
+ from .slow_path.store import PostgresAnalysisStore
286
 
287
+ self._analysis_store = PostgresAnalysisStore()
288
  return self._analysis_store
289
 
290
  async def _run_slow_path(
 
368
  yield {"event": "sources", "data": json.dumps([])} # TODO: derive from record
369
  yield {"event": "chunk", "data": result.chat_answer}
370
  try:
371
+ # Stamp the owner now (in scope here); analysis_id is wired once the
372
+ # Analysis State reaches the slow path. The store is also never-throw.
373
+ record = result.analysis_record.model_copy(update={"user_id": user_id})
374
+ await self._get_analysis_store().save(record)
375
  except Exception as e: # persistence must never break the user's answer
376
  logger.error("analysis_record persist failed", user_id=user_id, error=str(e))
377
  tracer.end() # output omitted (chat_answer may contain PII on Cloud)
src/agents/slow_path/assembler.py CHANGED
@@ -120,6 +120,7 @@ def _build_record(narrative: AssemblerNarrative, run_state: RunState) -> Analysi
120
  tasks_run = [
121
  TaskSummary(
122
  task_id=task_id,
 
123
  objective=result.objective,
124
  status=result.status,
125
  tools_used=[o.tool for o in result.outputs],
 
120
  tasks_run = [
121
  TaskSummary(
122
  task_id=task_id,
123
+ stage=result.stage,
124
  objective=result.objective,
125
  status=result.status,
126
  tools_used=[o.tool for o in result.outputs],
src/agents/slow_path/schemas.py CHANGED
@@ -21,10 +21,12 @@ from __future__ import annotations
21
 
22
  from datetime import datetime
23
  from typing import Literal
 
24
 
25
  from pydantic import BaseModel, Field
26
 
27
  from ..planner.contracts import ToolOutput
 
28
 
29
  TaskStatus = Literal["success", "partial", "failure"]
30
 
@@ -36,6 +38,7 @@ TaskStatus = Literal["success", "partial", "failure"]
36
 
37
  class TaskResult(BaseModel):
38
  task_id: str
 
39
  status: TaskStatus
40
  objective: str
41
  outputs: list[ToolOutput] = Field(default_factory=list) # one per tool_call
@@ -57,12 +60,21 @@ class RunState(BaseModel):
57
 
58
  class TaskSummary(BaseModel):
59
  task_id: str
 
60
  objective: str
61
  status: TaskStatus
62
  tools_used: list[str] = Field(default_factory=list)
63
 
64
 
65
  class AnalysisRecord(BaseModel):
 
 
 
 
 
 
 
 
66
  # Narrative fields — authored by the Assembler LLM.
67
  goal_restated: str
68
  findings: list[str] = Field(default_factory=list)
 
21
 
22
  from datetime import datetime
23
  from typing import Literal
24
+ from uuid import uuid4
25
 
26
  from pydantic import BaseModel, Field
27
 
28
  from ..planner.contracts import ToolOutput
29
+ from ..planner.schemas import CrispStage
30
 
31
  TaskStatus = Literal["success", "partial", "failure"]
32
 
 
38
 
39
  class TaskResult(BaseModel):
40
  task_id: str
41
+ stage: CrispStage # copied from the plan Task; carries CRISP-DM grouping to the report
42
  status: TaskStatus
43
  objective: str
44
  outputs: list[ToolOutput] = Field(default_factory=list) # one per tool_call
 
60
 
61
  class TaskSummary(BaseModel):
62
  task_id: str
63
+ stage: CrispStage # lets the report group the method appendix by CRISP-DM phase
64
  objective: str
65
  status: TaskStatus
66
  tools_used: list[str] = Field(default_factory=list)
67
 
68
 
69
  class AnalysisRecord(BaseModel):
70
+ # Identity. `record_id` is the unit the report cites and snapshots
71
+ # (`record_ids`); `analysis_id`/`user_id` scope the record to one analysis
72
+ # session + owner and are stamped by the composition root / AnalysisStore at
73
+ # persist time (they depend on the Analysis State that lives outside the slow
74
+ # path), so they default to None when the Assembler first builds the record.
75
+ record_id: str = Field(default_factory=lambda: uuid4().hex)
76
+ analysis_id: str | None = None
77
+ user_id: str | None = None
78
  # Narrative fields — authored by the Assembler LLM.
79
  goal_restated: str
80
  findings: list[str] = Field(default_factory=list)
src/agents/slow_path/store.py CHANGED
@@ -2,21 +2,28 @@
2
 
3
  The Assembler produces an `AnalysisRecord` (the faithful, structured record of a
4
  run — §8.3, INV-4). Persisting it is a separate concern from streaming the answer,
5
- so it sits behind this one-method seam.
 
6
 
7
- `NullAnalysisStore` is the default: it logs that a record was produced but stores
8
- nothing, because the backing table does not exist yet. The plan is to store records
9
- in the **same catalog DB** (Neon `dataeyond`, `settings.postgres_connstring`).
 
10
 
11
- TODO(persistence): add a Postgres-backed `AnalysisStore` writing an
12
- `analysis_records` table in the catalog DB, keyed on
13
- (business_context_id, plan_id, created_at), then inject it into ChatHandler.
14
  """
15
 
16
  from __future__ import annotations
17
 
18
  from typing import Protocol, runtime_checkable
19
 
 
 
 
 
 
20
  from src.middlewares.logging import get_logger
21
 
22
  from .schemas import AnalysisRecord
@@ -26,19 +33,78 @@ logger = get_logger("analysis_store")
26
 
27
  @runtime_checkable
28
  class AnalysisStore(Protocol):
29
- """Persist a completed analysis. Implementations must never raise on the
30
- caller's path — a persistence failure must not break the user's answer."""
 
 
 
31
 
32
  async def save(self, record: AnalysisRecord) -> None: ...
33
 
 
 
34
 
35
  class NullAnalysisStore:
36
- """Default no-op store: logs the record, persists nothing (no table yet)."""
37
 
38
  async def save(self, record: AnalysisRecord) -> None:
39
  logger.info(
40
- "analysis_record produced (not persisted — no store configured)",
 
41
  plan_id=record.plan_id,
42
- business_context_id=record.business_context_id,
43
  n_tasks=len(record.tasks_run),
44
  )
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
2
 
3
  The Assembler produces an `AnalysisRecord` (the faithful, structured record of a
4
  run — §8.3, INV-4). Persisting it is a separate concern from streaming the answer,
5
+ so it sits behind this seam. `generate_report` later reads records back by
6
+ `analysis_id` (oldest-first) and renders from them — never from chat history.
7
 
8
+ - `NullAnalysisStore` logs and stores nothing (kept for tests / when persistence
9
+ is intentionally disabled).
10
+ - `PostgresAnalysisStore` writes one `analysis_records` row per run in the catalog
11
+ DB (Neon `dataeyond`, `settings.postgres_connstring`).
12
 
13
+ `save` must never raise on the caller's path — a persistence failure must not break
14
+ the user's answer (§8.3). `list_for_analysis` is a read for the report generator and
15
+ is allowed to surface errors to its caller.
16
  """
17
 
18
  from __future__ import annotations
19
 
20
  from typing import Protocol, runtime_checkable
21
 
22
+ from sqlalchemy import select
23
+ from sqlalchemy.dialects.postgresql import insert
24
+
25
+ from src.db.postgres.connection import AsyncSessionLocal
26
+ from src.db.postgres.models import AnalysisRecordRow
27
  from src.middlewares.logging import get_logger
28
 
29
  from .schemas import AnalysisRecord
 
33
 
34
  @runtime_checkable
35
  class AnalysisStore(Protocol):
36
+ """Persist + read completed analyses.
37
+
38
+ `save` must never raise on the caller's path. `list_for_analysis` returns the
39
+ records for one analysis session, oldest-first (the order the report renders in).
40
+ """
41
 
42
  async def save(self, record: AnalysisRecord) -> None: ...
43
 
44
+ async def list_for_analysis(self, analysis_id: str) -> list[AnalysisRecord]: ...
45
+
46
 
47
  class NullAnalysisStore:
48
+ """No-op store: logs the record, persists nothing. Reads return empty."""
49
 
50
  async def save(self, record: AnalysisRecord) -> None:
51
  logger.info(
52
+ "analysis_record produced (not persisted — NullAnalysisStore)",
53
+ record_id=record.record_id,
54
  plan_id=record.plan_id,
 
55
  n_tasks=len(record.tasks_run),
56
  )
57
+
58
+ async def list_for_analysis(self, analysis_id: str) -> list[AnalysisRecord]:
59
+ return []
60
+
61
+
62
+ class PostgresAnalysisStore:
63
+ """Writes/reads `analysis_records` jsonb rows in the catalog DB.
64
+
65
+ Mirrors `CatalogStore`: each call opens its own `AsyncSession`. One row per
66
+ record (vs. one-per-user for the catalog) since records accumulate per analysis.
67
+ """
68
+
69
+ async def save(self, record: AnalysisRecord) -> None:
70
+ try:
71
+ payload = record.model_dump(mode="json")
72
+ async with AsyncSessionLocal() as session:
73
+ stmt = insert(AnalysisRecordRow).values(
74
+ id=record.record_id,
75
+ analysis_id=record.analysis_id,
76
+ user_id=record.user_id,
77
+ plan_id=record.plan_id,
78
+ data=payload,
79
+ created_at=record.created_at,
80
+ )
81
+ # Re-running the same plan id-collides only if record_id repeats;
82
+ # treat that as idempotent (overwrite) rather than erroring the user.
83
+ stmt = stmt.on_conflict_do_update(
84
+ index_elements=[AnalysisRecordRow.id],
85
+ set_={"data": stmt.excluded.data},
86
+ )
87
+ await session.execute(stmt)
88
+ await session.commit()
89
+ logger.info(
90
+ "analysis_record persisted",
91
+ record_id=record.record_id,
92
+ analysis_id=record.analysis_id,
93
+ user_id=record.user_id,
94
+ )
95
+ except Exception as exc: # never break the user's answer (§8.3)
96
+ logger.error(
97
+ "analysis_record persist failed",
98
+ record_id=record.record_id,
99
+ error=str(exc),
100
+ )
101
+
102
+ async def list_for_analysis(self, analysis_id: str) -> list[AnalysisRecord]:
103
+ async with AsyncSessionLocal() as session:
104
+ result = await session.execute(
105
+ select(AnalysisRecordRow.data)
106
+ .where(AnalysisRecordRow.analysis_id == analysis_id)
107
+ .order_by(AnalysisRecordRow.created_at.asc())
108
+ )
109
+ rows = result.scalars().all()
110
+ return [AnalysisRecord.model_validate(row) for row in rows]
src/agents/slow_path/task_runner.py CHANGED
@@ -53,6 +53,7 @@ class TaskRunner:
53
  for tid in list(remaining):
54
  results[tid] = TaskResult(
55
  task_id=tid,
 
56
  status="failure",
57
  objective=tasks_by_id[tid].objective,
58
  error="unresolved dependency; task could not run",
@@ -68,6 +69,7 @@ class TaskRunner:
68
  if failed:
69
  results[tid] = TaskResult(
70
  task_id=tid,
 
71
  status="failure",
72
  objective=task.objective,
73
  error=f"skipped: upstream {failed} did not succeed",
@@ -110,6 +112,7 @@ class TaskRunner:
110
  error = errs[0] if errs else "all tool calls failed"
111
  return TaskResult(
112
  task_id=task.id,
 
113
  status=status,
114
  objective=task.objective,
115
  outputs=outputs,
 
53
  for tid in list(remaining):
54
  results[tid] = TaskResult(
55
  task_id=tid,
56
+ stage=tasks_by_id[tid].stage,
57
  status="failure",
58
  objective=tasks_by_id[tid].objective,
59
  error="unresolved dependency; task could not run",
 
69
  if failed:
70
  results[tid] = TaskResult(
71
  task_id=tid,
72
+ stage=task.stage,
73
  status="failure",
74
  objective=task.objective,
75
  error=f"skipped: upstream {failed} did not succeed",
 
112
  error = errs[0] if errs else "all tool calls failed"
113
  return TaskResult(
114
  task_id=task.id,
115
+ stage=task.stage,
116
  status=status,
117
  objective=task.objective,
118
  outputs=outputs,
src/db/postgres/init_db.py CHANGED
@@ -3,6 +3,7 @@
3
  from sqlalchemy import text
4
  from src.db.postgres.connection import engine, Base
5
  from src.db.postgres.models import (
 
6
  Catalog,
7
  ChatMessage,
8
  DatabaseClient,
 
3
  from sqlalchemy import text
4
  from src.db.postgres.connection import engine, Base
5
  from src.db.postgres.models import (
6
+ AnalysisRecordRow,
7
  Catalog,
8
  ChatMessage,
9
  DatabaseClient,
src/db/postgres/models.py CHANGED
@@ -115,4 +115,25 @@ class Catalog(Base):
115
  schema_version = Column(String, nullable=False, default="1.0")
116
  generated_at = Column(DateTime(timezone=True), server_default=func.now())
117
  updated_at = Column(DateTime(timezone=True), onupdate=func.now())
118
-
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
115
  schema_version = Column(String, nullable=False, default="1.0")
116
  generated_at = Column(DateTime(timezone=True), server_default=func.now())
117
  updated_at = Column(DateTime(timezone=True), onupdate=func.now())
118
+
119
+
120
+ class AnalysisRecordRow(Base):
121
+ """One row per completed slow-path analysis (the report's source of truth).
122
+
123
+ `data` holds the full Pydantic AnalysisRecord
124
+ (src/agents/slow_path/schemas.py:AnalysisRecord) serialized via
125
+ `model_dump(mode="json")`; the read path rehydrates with
126
+ `AnalysisRecord.model_validate(...)`. Many records accumulate per analysis
127
+ session — `generate_report` reads them by `analysis_id`, oldest-first.
128
+
129
+ `analysis_id` is nullable until the Analysis State (owned upstream) is wired
130
+ into the slow path; records still persist (and carry `user_id`) before then.
131
+ """
132
+ __tablename__ = "analysis_records"
133
+
134
+ id = Column(String, primary_key=True) # AnalysisRecord.record_id
135
+ analysis_id = Column(String, index=True) # FK to the analysis session (nullable for now)
136
+ user_id = Column(String, nullable=False, index=True)
137
+ plan_id = Column(String, nullable=False)
138
+ data = Column(JSONB, nullable=False)
139
+ created_at = Column(DateTime(timezone=True), server_default=func.now())