sofhiaazzhr commited on
Commit
8467ab8
Β·
1 Parent(s): 0e2bb5c

[NOTICKET] docs: sync PROGRESS with current tool layer + TAB status

Record the tool-layer hardening + DRY work and reconcile the All-items
table with the shipped code (TAB-owned paths only):

- Fix stale note: analyze_contribution Decimal bug is fixed (KM-630 /
1195870), not open.
- Add TAB "What just shipped (2026-06-10)" block: Decimal/JSON-safe
coercion, registry alignment + Timestamp keys, single SAMPLE_LIMIT.
- Flip TAB items 6/29/30/39 [~] -> [x] (their PR rows already shipped).
- Add missing "Tools" category (whole src/tools/, KM-608/624-631).
- Note enable_slow_path/enable_tracing gate on chat_handler row.

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

Files changed (1) hide show
  1. PROGRESS.md +61 -8
PROGRESS.md CHANGED
@@ -2,7 +2,7 @@
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-10 (tool layer complete + Langfuse tracing + gated slow-path wiring)
6
  **Current open PR**: `pr/2` β€” active.
7
 
8
  ---
@@ -28,8 +28,10 @@ unchanged (flags default off).
28
  take a `data` `${t<id>}` placeholder from an upstream `query_structured`).
29
  - **Verified live E2E (2026-06-09):** real `query_structured` against a user's Neon
30
  Postgres β†’ `analyze_trend` β†’ Assembler. `analyze_contribution` surfaced a real tool
31
- bug (Decimal vs float in `decomposition.py`) β€” degrade-and-continue held; teammate's
32
- domain to fix. **Directive:** agent side does NOT modify `src/tools/` without confirmation.
 
 
33
 
34
  **Planner β€” realigned to the real tools (KM-626).** `registry.py::default_registry()`
35
  composes the real `analytics_registry()` + a local stub for the 4 data-access tools.
@@ -70,6 +72,44 @@ or keep the planner stub; 4o β†’ GPT-mini deployment swap; flip `enable_slow_pat
70
 
71
  ---
72
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
73
  ## What just shipped (2026-06-08 β€” KM-626: slow-path agent layer)
74
 
75
  The rest of the slow path after the Planner (KM-567), from
@@ -221,7 +261,7 @@ per the architecture doc: Orchestrator slow-path expansion + TaskRunner + Assemb
221
  | # | Item | Owner | Status | Notes |
222
  |---|---|---|---|---|
223
  | 5 | DB introspector (`catalog/introspect/database.py`) | DB | `[x]` | PR1 β€” reuses Phase 1 `database_client_service`, `db_credential_encryption`, `db_pipeline_service.engine_scope`, `extractor.get_schema/profile_column/get_row_count`. PR2a wired FK extraction (was discarded before). |
224
- | 6 | Tabular introspector (`catalog/introspect/tabular.py`) | TAB | `[~]` | PR1-tab β€” downloads original blob (CSV/XLSX/Parquet), one Table per sheet (XLSX) or one Table (CSV/Parquet). `source_id = document_id`. `fetch_doc`/`fetch_blob` injectable for unit tests (no Settings). |
225
  | 7 | `BaseIntrospector` ABC (`catalog/introspect/base.py`) | B | `[x]` | Pre-existing; signature locked |
226
 
227
  ### Ingestion β€” shared catalog plumbing
@@ -269,8 +309,8 @@ per the architecture doc: Orchestrator slow-path expansion + TaskRunner + Assemb
269
 
270
  | # | Item | Status | Notes |
271
  |---|---|---|---|
272
- | 29 | Pandas compiler (`query/compiler/pandas.py`) | `[~]` | PR3-TAB β€” `CompiledPandas` dataclass; all 12 filter ops; all 6 aggs; group_by via `pd.concat` of Series; alias-aware order_by; `_like_to_regex` (`%`β†’`.*`, `_`β†’`.`); pure module-level helpers |
273
- | 30 | Tabular executor (`query/executor/tabular.py`) | `[~]` | PR3-TAB β€” `fetch_blob` injectable for tests; blob path: single-table β†’ `{uid}/{did}.parquet`, multi-table β†’ `{uid}/{did}__{table.name}.parquet`; `asyncio.to_thread`; 10k row hard cap; errors β†’ `QueryResult.error` |
274
  | 31 | Parquet upload/download wrapper | `[x]` | Moved `knowledge/parquet_service.py` β†’ `storage/parquet.py`. Updated 4 import sites: `pipeline/document_pipeline.py`, `knowledge/processing_service.py`, `query/executor/tabular.py`, `query/executors/tabular.py`. |
275
 
276
  ### Agents + chat
@@ -279,7 +319,20 @@ per the architecture doc: Orchestrator slow-path expansion + TaskRunner + Assemb
279
  |---|---|---|---|
280
  | 32 | Chatbot agent + prompt (`agents/chatbot.py`, `config/prompts/chatbot_system.md`) | `[x]` | PR7-bundle β€” `ChatbotAgent` (was `AnswerAgent`) streams tokens, accepts `QueryResult` or list[`DocumentChunk`] or neither. **Cleanup PR**: renamed `answer_agent.py` β†’ `chatbot.py`, `AnswerAgent` β†’ `ChatbotAgent`; Phase 1 `agents/chatbot.py` deleted. |
281
  | 33 | Guardrails prompt (`config/prompts/guardrails.md`) | `[x]` | PR7-bundle β€” appended to `chatbot_system.md` so guardrails take precedence in conflict. |
282
- | β€” | Chat handler / orchestrator (`agents/chat_handler.py`) | `[x]` | PR4-bundle β€” top-level Phase 2 orchestrator. Routes by `source_hint`: chat β†’ AnswerAgent direct; structured β†’ CatalogReader + QueryService; unstructured β†’ DocumentRetriever placeholder + AnswerAgent. Yields `intent` / `chunk` / `done` / `error` SSE-style events. Phase 1 chat.py NOT touched β€” cleanup PR rewires the API to call this. |
 
 
 
 
 
 
 
 
 
 
 
 
 
283
 
284
  ### API surface
285
 
@@ -296,7 +349,7 @@ per the architecture doc: Orchestrator slow-path expansion + TaskRunner + Assemb
296
  | # | Item | Owner | Status | Notes |
297
  |---|---|---|---|---|
298
  | 38 | DB compiler golden tests (`tests/query/compiler/test_sql.py`) | DB | `[x]` | PR3-DB β€” 36 tests across all whitelisted ops, identifier quoting, agg / count_distinct / count(*), order_by alias resolution, parameter sequencing, error paths. Pure-Python, no LLM, no DB. |
299
- | 39 | Pandas compiler golden tests (`tests/unit/query/compiler/test_pandas_compiler.py`) | TAB | `[~]` | PR3-TAB β€” 43 tests: all 12 filter ops, all 6 aggs, group_by, order_by, limit, aliases, empty DataFrame, error paths. `test_tabular_executor.py` adds 12 more (blob name resolution + happy path + error paths). |
300
  | 40 | IR validator tests (`tests/query/ir/test_validator.py`) | B | `[x]` | PR1 β€” 19 tests, all rules covered |
301
  | β€” | PII detector tests (`tests/catalog/test_pii_detector.py`) | B | `[x]` | PR1 β€” 26 tests (parametrized) |
302
  | β€” | Catalog validator tests (`tests/catalog/test_validator.py`) | B | `[x]` | PR1 β€” 5 tests |
 
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-10 (tool layer complete + hardening/DRY + Langfuse tracing + gated slow-path wiring)
6
  **Current open PR**: `pr/2` β€” active.
7
 
8
  ---
 
28
  take a `data` `${t<id>}` placeholder from an upstream `query_structured`).
29
  - **Verified live E2E (2026-06-09):** real `query_structured` against a user's Neon
30
  Postgres β†’ `analyze_trend` β†’ Assembler. `analyze_contribution` surfaced a real tool
31
+ bug (Decimal vs float in `decomposition.py`) β€” degrade-and-continue held; **now fixed
32
+ by the tool owner** (`_coerce_decimals` in `invoker._materialize`, KM-630 / commit
33
+ 1195870), so the whole `analyze_*` family is covered in one place. **Directive:** agent
34
+ side does NOT modify `src/tools/` without confirmation.
35
 
36
  **Planner β€” realigned to the real tools (KM-626).** `registry.py::default_registry()`
37
  composes the real `analytics_registry()` + a local stub for the 4 data-access tools.
 
72
 
73
  ---
74
 
75
+ ## What just shipped (2026-06-10 β€” TAB: tool-layer hardening + DRY)
76
+
77
+ Owner-side companion to the agent block above. After the live E2E surfaced real-data
78
+ edge cases, the tool layer got a round of correctness hardening. All in TAB-owned paths
79
+ (`src/tools/`, `src/catalog/`); no agent-side or API change.
80
+
81
+ **JSON-safety across the `analyze_*` family.** Real DB rows carry scalar types that
82
+ don't survive the jsonb / SSE round-trip:
83
+ - `[KM-630] coerce DB Decimal β†’ float` (commit 1195870) β€” `_coerce_decimals` in
84
+ `invoker._materialize` converts object-columns holding `decimal.Decimal` (asyncpg
85
+ returns NUMERIC as `Decimal`) to `float64` before any compute runs. Fixes the
86
+ `float + Decimal` TypeError in `decomposition.analyze_contribution` **and** the whole
87
+ family in one seam β€” only touches columns that actually contain a `Decimal`.
88
+ - `[KM-624] non-JSON-safe scalars in mode & top_value` (commit 6981ed3) β€” normalize
89
+ numpy / non-native scalars so descriptive + top-value outputs serialize cleanly.
90
+
91
+ **Planner↔Tools registry alignment + Timestamp keys** (commit 4bb7623, `fix(tools)`):
92
+ - `registry.py` β€” `analyze_descriptive.required` corrected `["data"]` β†’ `["data",
93
+ "column_ids"]` to match the compute signature (`column_ids` has no default). Prevents
94
+ the Planner from emitting a call that's missing a required arg. `analyze_profile` stays
95
+ `["data"]` (its `column_ids` defaults to `None`).
96
+ - `aggregation._clean` β€” group-by over a datetime column produced `pd.Timestamp` group
97
+ keys that aren't JSON-safe; now normalized to `.isoformat()` alongside the existing
98
+ numpy `.item()` branch.
99
+
100
+ **DRY: single `SAMPLE_LIMIT` constant** (commit 6d46ba5, `[NOTICKET] refactor(catalog)`):
101
+ - One source of truth in `catalog/introspect/base.py` (`SAMPLE_LIMIT = 3`, down from 5 β€”
102
+ token cost: sample values feed the planner prompt). Both introspection paths import it:
103
+ `catalog/introspect/tabular.py` and `pipeline/db_pipeline/extractor.py` (which dropped
104
+ its own local `= 3`). Dependency direction is pipeline→catalog (no circular import).
105
+ Stale test `test_sample_values_capped_at_five` updated to assert the real cap (3).
106
+
107
+ **Audit result:** Planner↔Tools arg alignment swept end-to-end β€” 7/8 `analyze_*` tools
108
+ already matched; the 1 mismatch (`analyze_descriptive`) is the fix above. Pattern A holds
109
+ across all of them.
110
+
111
+ ---
112
+
113
  ## What just shipped (2026-06-08 β€” KM-626: slow-path agent layer)
114
 
115
  The rest of the slow path after the Planner (KM-567), from
 
261
  | # | Item | Owner | Status | Notes |
262
  |---|---|---|---|---|
263
  | 5 | DB introspector (`catalog/introspect/database.py`) | DB | `[x]` | PR1 β€” reuses Phase 1 `database_client_service`, `db_credential_encryption`, `db_pipeline_service.engine_scope`, `extractor.get_schema/profile_column/get_row_count`. PR2a wired FK extraction (was discarded before). |
264
+ | 6 | Tabular introspector (`catalog/introspect/tabular.py`) | TAB | `[x]` | PR1-tab β€” downloads original blob (CSV/XLSX/Parquet), one Table per sheet (XLSX) or one Table (CSV/Parquet). `source_id = document_id`. `fetch_doc`/`fetch_blob` injectable for unit tests (no Settings). **2026-06-10**: sample cap now imports the shared `SAMPLE_LIMIT` (=3) from `catalog/introspect/base.py` β€” single source of truth across the tabular + DB introspection paths (commit 6d46ba5). |
265
  | 7 | `BaseIntrospector` ABC (`catalog/introspect/base.py`) | B | `[x]` | Pre-existing; signature locked |
266
 
267
  ### Ingestion β€” shared catalog plumbing
 
309
 
310
  | # | Item | Status | Notes |
311
  |---|---|---|---|
312
+ | 29 | Pandas compiler (`query/compiler/pandas.py`) | `[x]` | PR3-TAB β€” `CompiledPandas` dataclass; all 12 filter ops; all 6 aggs; group_by via `pd.concat` of Series; alias-aware order_by; `_like_to_regex` (`%`β†’`.*`, `_`β†’`.`); pure module-level helpers. (`polars` for large files still deferred β€” see Planned dependencies.) |
313
+ | 30 | Tabular executor (`query/executor/tabular.py`) | `[x]` | PR3-TAB β€” `fetch_blob` injectable for tests; blob path: single-table β†’ `{uid}/{did}.parquet`, multi-table β†’ `{uid}/{did}__{table.name}.parquet`; `asyncio.to_thread`; 10k row hard cap; errors β†’ `QueryResult.error`. Dispatcher routes to it by `source_type`. |
314
  | 31 | Parquet upload/download wrapper | `[x]` | Moved `knowledge/parquet_service.py` β†’ `storage/parquet.py`. Updated 4 import sites: `pipeline/document_pipeline.py`, `knowledge/processing_service.py`, `query/executor/tabular.py`, `query/executors/tabular.py`. |
315
 
316
  ### Agents + chat
 
319
  |---|---|---|---|
320
  | 32 | Chatbot agent + prompt (`agents/chatbot.py`, `config/prompts/chatbot_system.md`) | `[x]` | PR7-bundle β€” `ChatbotAgent` (was `AnswerAgent`) streams tokens, accepts `QueryResult` or list[`DocumentChunk`] or neither. **Cleanup PR**: renamed `answer_agent.py` β†’ `chatbot.py`, `AnswerAgent` β†’ `ChatbotAgent`; Phase 1 `agents/chatbot.py` deleted. |
321
  | 33 | Guardrails prompt (`config/prompts/guardrails.md`) | `[x]` | PR7-bundle β€” appended to `chatbot_system.md` so guardrails take precedence in conflict. |
322
+ | — | Chat handler / orchestrator (`agents/chat_handler.py`) | `[x]` | PR4-bundle — top-level Phase 2 orchestrator. Routes by `source_hint`: chat → AnswerAgent direct; structured → CatalogReader + QueryService; unstructured → DocumentRetriever placeholder + AnswerAgent. Yields `intent` / `chunk` / `done` / `error` SSE-style events. Phase 1 chat.py NOT touched — cleanup PR rewires the API to call this. **2026-06-09**: gained the gated `structured→slow` branch (`enable_slow_path=False`) + `enable_tracing` (KM-626/631). |
323
+
324
+ ### Tools β€” slow-path "Tools" component (TAB)
325
+
326
+ New scope after the original 42-item table; added as the tool layer landed (KM-608/624–631). All TAB-owned (`src/tools/`), all never-throw (Β§8.4).
327
+
328
+ | # | Item | Owner | Status | Notes |
329
+ |---|---|---|---|---|
330
+ | β€” | Analytics compute fns (`tools/analytics/`) | TAB | `[x]` | KM-608/624/625 β€” 8 **composite** `analyze_*` fns (descriptive, aggregate, comparison, contribution, profile, correlation, segment, trend) + prompt-style DESCRIPTIONs. Pure pandas, no I/O. JSON-safe outputs (numpy/Decimal/Timestamp normalized β€” KM-624 + commit 4bb7623). |
331
+ | β€” | Tool contracts (`tools/contracts.py`) | TAB | `[x]` | KM-627 β€” canonical `ToolSpec` / `ToolRegistry` / `ToolOutput`. `agents/planner/contracts.py` re-exports them (+ keeps the lead's `BusinessContext` stub). |
332
+ | β€” | Analytics registry (`tools/registry.py`) | TAB | `[x]` | KM-628 β€” `analytics_registry()`. `analyze_descriptive.required` = `["data","column_ids"]` (aligned to compute signature, commit 4bb7623). |
333
+ | β€” | Invoker layer (`tools/invoker.py`) | TAB | `[x]` | KM-629 β€” `AnalyticsToolInvoker` (Pattern A: `analyze_*` take a `data` `${t<id>}` placeholder from upstream `query_structured`; `_materialize` β†’ DataFrame, `_coerce_decimals` covers the whole family) + `CompositeToolInvoker` (routes data-access vs analytics by name). |
334
+ | β€” | Data-access tools (`tools/data_access.py`) | TAB | `[x]` | KM-630 β€” `DataAccessToolInvoker`: `list_sources` / `describe_source` / `query_structured` / `retrieve_documents`. Per-request DI (`user_id` + `CatalogReader`). `query_structured` calls `IRValidator` + `ExecutorDispatcher` (planner skipped β€” IR pre-built by the agent Planner). |
335
+ | β€” | Tool tests (`tests/unit/tools/`) | TAB | `[x]` | analytics + data-access + invoker tests (gitignored). Incl. regression `test_decimal_columns_coerced_for_analyze_contribution`. |
336
 
337
  ### API surface
338
 
 
349
  | # | Item | Owner | Status | Notes |
350
  |---|---|---|---|---|
351
  | 38 | DB compiler golden tests (`tests/query/compiler/test_sql.py`) | DB | `[x]` | PR3-DB β€” 36 tests across all whitelisted ops, identifier quoting, agg / count_distinct / count(*), order_by alias resolution, parameter sequencing, error paths. Pure-Python, no LLM, no DB. |
352
+ | 39 | Pandas compiler golden tests (`tests/unit/query/compiler/test_pandas_compiler.py`) | TAB | `[x]` | PR3-TAB β€” 43 tests: all 12 filter ops, all 6 aggs, group_by, order_by, limit, aliases, empty DataFrame, error paths. `test_tabular_executor.py` adds 12 more (blob name resolution + happy path + error paths). |
353
  | 40 | IR validator tests (`tests/query/ir/test_validator.py`) | B | `[x]` | PR1 β€” 19 tests, all rules covered |
354
  | β€” | PII detector tests (`tests/catalog/test_pii_detector.py`) | B | `[x]` | PR1 β€” 26 tests (parametrized) |
355
  | β€” | Catalog validator tests (`tests/catalog/test_validator.py`) | B | `[x]` | PR1 β€” 5 tests |