Rifqi Hafizuddin Claude Opus 4.8 commited on
Commit
b8b76c4
·
1 Parent(s): 3743cfe

fix(chat): drop ENABLE_SLOW_PATH flag + make chat persistence read-only

Browse files

- Remove the ENABLE_SLOW_PATH feature flag. structured_flow now always runs the
slow analytical path (Planner -> TaskRunner -> Assembler); the single-query
QueryService fast path is retired from ChatHandler. Reports no longer depend on
flipping a flag -- every structured question persists a report_inputs record.
- DEV_PLAN #25: make Python read-only on analyses_messages. Dropped the
save_messages calls from POST /api/v2/chat/stream so Go is the sole writer,
fixing the double-write both Go and Python were producing. load_history still
reads the table.
- Sync REPO_STATUS + DEV_PLAN to the new behavior.

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

DEV_PLAN.md CHANGED
@@ -173,7 +173,7 @@ Status legend: ⬜ not started · 🔄 in progress · ✅ done · ⛔ blocked ·
173
  | 22 | Finalize `report_inputs` schema → hand to Harry for the dedorch migration | Rifqi → Harry | 🔄 | **DDL ready** (uuid `id`/`analysis_id` + FK→`analyses(id)`; `user_id`/`plan_id` text; `data` jsonb = serialized `AnalysisRecord`, shape documented). dedorch has empty `analysis_records` → rename. Resolves #16. **Action: send Harry the DDL + `data` shape** |
174
  | 23 | Report markdown formatting: tables, **bold**, *italic*, horizontal separators | Sofhia | ✅ | Done 2026-06-25. Added `---` separators between header + each section in `_render_markdown`. Tables (EDA) / bold (method labels) / italic (meta + citations) already emitted. Relaxed `report_summary.md` to allow inline `**bold**`/`*italic*` for emphasis (kept no-headings/no-bullets so it doesn't duplicate the section structure / Key Findings). Compile + ruff clean |
175
  | 24 | Clarify report input contract: records table (+ `last_report` for edit mode?) | Rifqi/Sofhia ↔ Harry | ⬜ new | Edit-mode input left open at the checkpoint |
176
- | 25 | Migrate Python chat path to Go `analyses_messages` (+ `analyses`) | Rifqi ↔ Harry | | **Bigger than "confirm" (verified 2026-06-25):** dedorch `rooms` + `chat_messages` are **deprecated** (`zdeprecated_*`). Python's `Room`/`ChatMessage` models + `chat.py` `load_history`/`save_messages` target them → **break post-cutover**. Move history read/write to `analyses_messages` before the conn-string cutover |
177
  | 26 | **Charts (DEFERRED):** store Plotly JSON in a future `chart` table (not matplotlib PNG) | — | ⏸️ | After the markdown path is done end-to-end |
178
  | 27 | **Images (DEFERRED):** image table (id, analysis_id, msg/report ref, order) + originals in a bucket | — | ⏸️ | Maintenance-heavy; parked |
179
  | 28 | **UI research** (FE): new-analysis form, knowledge menu (user vs analysis level), report artifacts + version selector | Team | ⬜ new | No dedicated UI person; interview + old analysis UI removed |
 
173
  | 22 | Finalize `report_inputs` schema → hand to Harry for the dedorch migration | Rifqi → Harry | 🔄 | **DDL ready** (uuid `id`/`analysis_id` + FK→`analyses(id)`; `user_id`/`plan_id` text; `data` jsonb = serialized `AnalysisRecord`, shape documented). dedorch has empty `analysis_records` → rename. Resolves #16. **Action: send Harry the DDL + `data` shape** |
174
  | 23 | Report markdown formatting: tables, **bold**, *italic*, horizontal separators | Sofhia | ✅ | Done 2026-06-25. Added `---` separators between header + each section in `_render_markdown`. Tables (EDA) / bold (method labels) / italic (meta + citations) already emitted. Relaxed `report_summary.md` to allow inline `**bold**`/`*italic*` for emphasis (kept no-headings/no-bullets so it doesn't duplicate the section structure / Key Findings). Compile + ruff clean |
175
  | 24 | Clarify report input contract: records table (+ `last_report` for edit mode?) | Rifqi/Sofhia ↔ Harry | ⬜ new | Edit-mode input left open at the checkpoint |
176
+ | 25 | Migrate Python chat path to Go `analyses_messages` (+ `analyses`) | Rifqi ↔ Harry | | Done 2026-07-02. Read path already on `analyses_messages` (commit `0066161`). This change makes Python **read-only**: removed the `save_messages` calls from `/api/v2/chat/stream` so **Go is the sole writer** fixes the double-write both Go+Python were producing. `load_history` still reads `analyses_messages`. v1 `/chat/stream` is unwired so left untouched |
177
  | 26 | **Charts (DEFERRED):** store Plotly JSON in a future `chart` table (not matplotlib PNG) | — | ⏸️ | After the markdown path is done end-to-end |
178
  | 27 | **Images (DEFERRED):** image table (id, analysis_id, msg/report ref, order) + originals in a bucket | — | ⏸️ | Maintenance-heavy; parked |
179
  | 28 | **UI research** (FE): new-analysis form, knowledge menu (user vs analysis level), report artifacts + version selector | Team | ⬜ new | No dedicated UI person; interview + old analysis UI removed |
REPO_STATUS.md CHANGED
@@ -129,8 +129,10 @@ GET /report/{analysis_id}/{ver} → fetch one version
129
  ```
130
 
131
  Two facts to internalise:
132
- - **Records only exist on the slow path.** With `ENABLE_SLOW_PATH=false` (the default) no
133
- records accumulate, so generation 409s by design, not a bug.
 
 
134
  - **dedorch `reports` stores markdown only.** Structured report fields are computed at
135
  generation, rendered into `rendered_markdown`, and only the markdown is persisted; on
136
  read-back the structured fields come back empty.
@@ -289,7 +291,7 @@ only.
289
 
290
  | Flag | Where | Default | Effect |
291
  |---|---|---|---|
292
- | `ENABLE_SLOW_PATH` | `settings.enable_slow_path` | **off** | Route `structured_flow` through Planner/TaskRunner/Assembler (vs single-query `QueryService`). Records persist only on the slow path reports require this on. |
293
  | `ENABLE_GATE` | `settings.enable_gate` | **off** | **Deprecated 2026-06-25** — gate neutered; the flag has no effect. Kept to avoid `.env` churn. |
294
  | `SKIP_INIT_DB` | `settings.skip_init_db` (.env/env) | **on** | Skip `init_db()` on startup — the dedorch cutover switch. **Defaults TRUE** (Go owns the dedorch schema); set `false` only for a local Python-owned DB. |
295
  | `enable_tracing` | hardcoded `True` in `chat.py` | on (endpoint) | Langfuse tracing. |
 
129
  ```
130
 
131
  Two facts to internalise:
132
+ - **Records only exist on the slow path.** The slow path is now **always on** for
133
+ `structured_flow` (the `ENABLE_SLOW_PATH` flag was removed 2026-07-02), so every
134
+ structured question persists a record. Reports still 409 until at least one `analyze_*`
135
+ task has actually succeeded (chat/help/check/unstructured turns write no record).
136
  - **dedorch `reports` stores markdown only.** Structured report fields are computed at
137
  generation, rendered into `rendered_markdown`, and only the markdown is persisted; on
138
  read-back the structured fields come back empty.
 
291
 
292
  | Flag | Where | Default | Effect |
293
  |---|---|---|---|
294
+ | ~~`ENABLE_SLOW_PATH`~~ | | **removed 2026-07-02** | Flag deleted. `structured_flow` now **always** runs Planner/TaskRunner/Assembler (the single-query `QueryService` fast path was retired from the chat handler), so records always persist. `extra="allow"` ignores a stale `ENABLE_SLOW_PATH` left in any `.env`. |
295
  | `ENABLE_GATE` | `settings.enable_gate` | **off** | **Deprecated 2026-06-25** — gate neutered; the flag has no effect. Kept to avoid `.env` churn. |
296
  | `SKIP_INIT_DB` | `settings.skip_init_db` (.env/env) | **on** | Skip `init_db()` on startup — the dedorch cutover switch. **Defaults TRUE** (Go owns the dedorch schema); set `false` only for a local Python-owned DB. |
297
  | `enable_tracing` | hardcoded `True` in `chat.py` | on (endpoint) | Langfuse tracing. |
src/agents/chat_handler.py CHANGED
@@ -5,7 +5,8 @@ End-to-end flow per user message:
5
  1. `OrchestratorAgent.classify` → RouterDecision (one of six intents).
6
  2. Route by intent:
7
  - `chat` → no context. Pass straight to ChatbotAgent.
8
- - `structured_flow` → CatalogReader → slow path / QueryService.
 
9
  - `unstructured_flow` → DocumentRetriever (RAG over PGVector) →
10
  list[DocumentChunk].
11
  - `check` → check_data / check_knowledge tool → rendered table.
@@ -48,7 +49,6 @@ from .orchestration import OrchestratorAgent
48
 
49
  if TYPE_CHECKING:
50
  from ..catalog.reader import CatalogReader
51
- from ..query.service import QueryService
52
  from ..retrieval.router import RetrievalRouter
53
  from .gate import AnalysisState
54
  from .slow_path.coordinator import SlowPathCoordinator
@@ -75,10 +75,8 @@ class ChatHandler:
75
  intent_router: OrchestratorAgent | None = None,
76
  answer_agent: ChatbotAgent | None = None,
77
  catalog_reader: CatalogReader | None = None,
78
- query_service: QueryService | None = None,
79
  document_retriever: RetrievalRouter | None = None,
80
  *,
81
- enable_slow_path: bool = False,
82
  slow_path_coordinator_factory: (
83
  Callable[[str], SlowPathCoordinator] | None
84
  ) = None,
@@ -94,16 +92,13 @@ class ChatHandler:
94
  self._intent_router = intent_router
95
  self._answer_agent = answer_agent
96
  self._catalog_reader = catalog_reader
97
- self._query_service = query_service
98
  self._document_retriever = document_retriever
99
  # Langfuse tracing (tokens + latency). OFF by default so tests never hit
100
  # Langfuse; the live endpoint opts in with ChatHandler(enable_tracing=True).
101
  self._enable_tracing = enable_tracing
102
- # Slow analytical path (Planner -> TaskRunner -> Assembler). OFF by default:
103
- # gated until the lead's real BusinessContext lands. When True, `structured`
104
- # intents route here instead of the single-query QueryService path. The
105
  # factory + store are injectable for tests.
106
- self._enable_slow_path = enable_slow_path
107
  self._slow_path_factory = slow_path_coordinator_factory
108
  self._analysis_store = analysis_store
109
  # `check` skill: builds the data-access invoker (check_data/check_knowledge)
@@ -144,13 +139,6 @@ class ChatHandler:
144
  self._catalog_reader = CatalogReader(CatalogStore())
145
  return self._catalog_reader
146
 
147
- def _get_query_service(self) -> QueryService:
148
- if self._query_service is None:
149
- from ..query.service import QueryService
150
-
151
- self._query_service = QueryService()
152
- return self._query_service
153
-
154
  def _get_document_retriever(self) -> RetrievalRouter:
155
  if self._document_retriever is None:
156
  from ..retrieval.router import RetrievalRouter
@@ -351,15 +339,13 @@ class ChatHandler:
351
  bound = await self._bound_source_ids(analysis_id)
352
  reader = _ScopedCatalogReader(req_reader, bound) if bound else req_reader
353
  catalog = await reader.read(user_id, "structured")
354
- if self._enable_slow_path:
355
- async for event in self._run_slow_path(
356
- user_id, rewritten, catalog, tracer, reader, analysis_id
357
- ):
358
- yield event
359
- return
360
- query_result = await self._get_query_service().run(
361
- user_id, rewritten, catalog
362
- )
363
  except Exception as e:
364
  logger.error(
365
  "structured route failed",
 
5
  1. `OrchestratorAgent.classify` → RouterDecision (one of six intents).
6
  2. Route by intent:
7
  - `chat` → no context. Pass straight to ChatbotAgent.
8
+ - `structured_flow` → CatalogReader → slow analytical path
9
+ (Planner → TaskRunner → Assembler).
10
  - `unstructured_flow` → DocumentRetriever (RAG over PGVector) →
11
  list[DocumentChunk].
12
  - `check` → check_data / check_knowledge tool → rendered table.
 
49
 
50
  if TYPE_CHECKING:
51
  from ..catalog.reader import CatalogReader
 
52
  from ..retrieval.router import RetrievalRouter
53
  from .gate import AnalysisState
54
  from .slow_path.coordinator import SlowPathCoordinator
 
75
  intent_router: OrchestratorAgent | None = None,
76
  answer_agent: ChatbotAgent | None = None,
77
  catalog_reader: CatalogReader | None = None,
 
78
  document_retriever: RetrievalRouter | None = None,
79
  *,
 
80
  slow_path_coordinator_factory: (
81
  Callable[[str], SlowPathCoordinator] | None
82
  ) = None,
 
92
  self._intent_router = intent_router
93
  self._answer_agent = answer_agent
94
  self._catalog_reader = catalog_reader
 
95
  self._document_retriever = document_retriever
96
  # Langfuse tracing (tokens + latency). OFF by default so tests never hit
97
  # Langfuse; the live endpoint opts in with ChatHandler(enable_tracing=True).
98
  self._enable_tracing = enable_tracing
99
+ # Slow analytical path (Planner -> TaskRunner -> Assembler): the only route for
100
+ # `structured_flow` now (the ENABLE_SLOW_PATH flag was removed 2026-07-02). The
 
101
  # factory + store are injectable for tests.
 
102
  self._slow_path_factory = slow_path_coordinator_factory
103
  self._analysis_store = analysis_store
104
  # `check` skill: builds the data-access invoker (check_data/check_knowledge)
 
139
  self._catalog_reader = CatalogReader(CatalogStore())
140
  return self._catalog_reader
141
 
 
 
 
 
 
 
 
142
  def _get_document_retriever(self) -> RetrievalRouter:
143
  if self._document_retriever is None:
144
  from ..retrieval.router import RetrievalRouter
 
339
  bound = await self._bound_source_ids(analysis_id)
340
  reader = _ScopedCatalogReader(req_reader, bound) if bound else req_reader
341
  catalog = await reader.read(user_id, "structured")
342
+ # structured_flow always runs the slow analytical path (the
343
+ # ENABLE_SLOW_PATH flag was removed 2026-07-02).
344
+ async for event in self._run_slow_path(
345
+ user_id, rewritten, catalog, tracer, reader, analysis_id
346
+ ):
347
+ yield event
348
+ return
 
 
349
  except Exception as e:
350
  logger.error(
351
  "structured route failed",
src/api/v1/chat.py CHANGED
@@ -26,11 +26,10 @@ router = APIRouter(prefix="/api/v1", tags=["Chat"])
26
  # is passed into handle()), and lazily builds + caches the Orchestrator/Chatbot
27
  # chains — so reusing it keeps the Azure OpenAI clients (and their httpx/TLS pools)
28
  # warm across requests instead of re-handshaking on the first call of every request.
29
- # enable_slow_path is env-gated (ENABLE_SLOW_PATH): when on, structured intents route
30
- # Orchestrator -> Planner -> TaskRunner -> Assembler so the team can test e2e here.
31
  _chat_handler = ChatHandler(
32
  enable_tracing=True,
33
- enable_slow_path=settings.enable_slow_path,
34
  enable_gate=settings.enable_gate,
35
  )
36
 
 
26
  # is passed into handle()), and lazily builds + caches the Orchestrator/Chatbot
27
  # chains — so reusing it keeps the Azure OpenAI clients (and their httpx/TLS pools)
28
  # warm across requests instead of re-handshaking on the first call of every request.
29
+ # Structured intents always route Orchestrator -> Planner -> TaskRunner -> Assembler
30
+ # (the analytical slow path); the ENABLE_SLOW_PATH flag was removed 2026-07-02.
31
  _chat_handler = ChatHandler(
32
  enable_tracing=True,
 
33
  enable_gate=settings.enable_gate,
34
  )
35
 
src/api/v2/chat.py CHANGED
@@ -12,10 +12,11 @@
12
  Only chat moves to v2; the tools group + observability stay on `/api/v1` (contract:
13
  API_ENDPOINTS_RESTRUCTURE.md §1).
14
 
15
- ⚠️ Persistence (transitional). This mirrors v1: it still load/saves turn history via the
16
- analysis-keyed message tables so multi-turn context works in the playground. Moving the
17
- read/write to Go-owned `analyses_messages` (and making Python read-only) is DEV_PLAN #25.
18
- Note Sofhia's `/tools/help` is already generative-only align chat with that under #25.
 
19
  """
20
 
21
  import json
@@ -38,7 +39,6 @@ from src.api.v1.chat import (
38
  cache_response,
39
  get_cached_response,
40
  load_history,
41
- save_messages,
42
  )
43
  from src.db.postgres.connection import get_db
44
  from src.db.redis.connection import get_redis
@@ -88,7 +88,6 @@ async def chat_stream(request: ChatRequest, db: AsyncSession = Depends(get_db)):
88
  logger.info("Returning cached response")
89
  cached_text = cached["response"]
90
  cached_sources = cached["sources"]
91
- await save_messages(db, analysis_id, request.user_id, request.message, cached_text)
92
 
93
  async def stream_cached():
94
  yield {"event": "sources", "data": json.dumps(cached_sources)}
@@ -103,7 +102,6 @@ async def chat_stream(request: ChatRequest, db: AsyncSession = Depends(get_db)):
103
  direct = _fast_intent(request.message)
104
  if direct:
105
  await cache_response(redis, cache_key, direct, sources=[])
106
- await save_messages(db, analysis_id, request.user_id, request.message, direct)
107
 
108
  async def stream_direct():
109
  yield {"event": "sources", "data": json.dumps([])}
@@ -142,12 +140,8 @@ async def chat_stream(request: ChatRequest, db: AsyncSession = Depends(get_db)):
142
  # Only cache stateless `chat` replies (see _CACHEABLE_INTENTS).
143
  if effective_intent in _CACHEABLE_INTENTS:
144
  await cache_response(redis, cache_key, full_response, sources=sources)
145
- try:
146
- await save_messages(
147
- db, analysis_id, request.user_id, request.message, full_response
148
- )
149
- except Exception as e:
150
- logger.error("save_messages failed", analysis_id=analysis_id, error=str(e))
151
  yield done_event
152
  elif event["event"] == "status":
153
  # slow-path progress: forward so the client shows activity.
 
12
  Only chat moves to v2; the tools group + observability stay on `/api/v1` (contract:
13
  API_ENDPOINTS_RESTRUCTURE.md §1).
14
 
15
+ Persistence (DEV_PLAN #25 done). Python is **read-only** on the Go-owned
16
+ `analyses_messages` table: it *reads* turn history (so multi-turn context works) but no
17
+ longer writes the user/AI turns Go is the sole writer. This removes the double-write
18
+ that appeared when both Go and Python's stream persisted the same turn. Aligns chat with
19
+ `/tools/help`, which was already generative-only.
20
  """
21
 
22
  import json
 
39
  cache_response,
40
  get_cached_response,
41
  load_history,
 
42
  )
43
  from src.db.postgres.connection import get_db
44
  from src.db.redis.connection import get_redis
 
88
  logger.info("Returning cached response")
89
  cached_text = cached["response"]
90
  cached_sources = cached["sources"]
 
91
 
92
  async def stream_cached():
93
  yield {"event": "sources", "data": json.dumps(cached_sources)}
 
102
  direct = _fast_intent(request.message)
103
  if direct:
104
  await cache_response(redis, cache_key, direct, sources=[])
 
105
 
106
  async def stream_direct():
107
  yield {"event": "sources", "data": json.dumps([])}
 
140
  # Only cache stateless `chat` replies (see _CACHEABLE_INTENTS).
141
  if effective_intent in _CACHEABLE_INTENTS:
142
  await cache_response(redis, cache_key, full_response, sources=sources)
143
+ # Persistence is Go's job now (DEV_PLAN #25): Python reads history but
144
+ # no longer writes turns, so Go stays the sole writer of analyses_messages.
 
 
 
 
145
  yield done_event
146
  elif event["event"] == "status":
147
  # slow-path progress: forward so the client shows activity.
src/config/settings.py CHANGED
@@ -17,12 +17,10 @@ class Settings(BaseSettings):
17
  )
18
 
19
  # Feature flags
20
- # Route `structured` chat intents through the analytical SLOW PATH
21
- # (Planner -> TaskRunner -> Assembler) instead of the single-query QueryService.
22
- # Off by default; the team flips ENABLE_SLOW_PATH=true to test end-to-end from
23
- # the /chat/stream endpoint. BusinessContext is still a stub until the lead's
24
- # real source lands, so this stays opt-in.
25
- enable_slow_path: bool = Field(alias="enable_slow_path", default=False)
26
 
27
  # DEPRECATED 2026-06-24: the problem_validated gate was removed (the goal is now
28
  # user-entered objective + business_questions, no agent validation). This flag no
 
17
  )
18
 
19
  # Feature flags
20
+ # REMOVED 2026-07-02: the former ENABLE_SLOW_PATH flag is gone. `structured_flow`
21
+ # now always runs the analytical slow path (Planner -> TaskRunner -> Assembler), so
22
+ # every structured question persists a report_inputs record (reports no longer
23
+ # depend on flipping a flag). extra="allow" ignores a stale ENABLE_SLOW_PATH in .env.
 
 
24
 
25
  # DEPRECATED 2026-06-24: the problem_validated gate was removed (the goal is now
26
  # user-entered objective + business_questions, no agent validation). This flag no