Merge

PROGRESS.md

@@ -2,7 +2,7 @@
 
 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.
 
-**Last updated**: 2026-06-10 (principal review findings logged + PR-DB1 latency work started)
+**Last updated**: 2026-06-10 (tool layer complete + hardening/DRY + Langfuse tracing + gated slow-path wiring)
 **Current open PR**: `pr/2` — active.
 
 ---
@@ -83,8 +83,10 @@ unchanged (flags default off).
   take a `data` `${t<id>}` placeholder from an upstream `query_structured`).
 - **Verified live E2E (2026-06-09):** real `query_structured` against a user's Neon
   Postgres → `analyze_trend` → Assembler. `analyze_contribution` surfaced a real tool
-  bug (Decimal vs float in `decomposition.py`) — degrade-and-continue held; teammate's
-  domain to fix. **Directive:** agent side does NOT modify `src/tools/` without confirmation.
+  bug (Decimal vs float in `decomposition.py`) — degrade-and-continue held; **now fixed
+  by the tool owner** (`_coerce_decimals` in `invoker._materialize`, KM-630 / commit
+  1195870), so the whole `analyze_*` family is covered in one place. **Directive:** agent
+  side does NOT modify `src/tools/` without confirmation.
 
 **Planner — realigned to the real tools (KM-626).** `registry.py::default_registry()`
 composes the real `analytics_registry()` + a local stub for the 4 data-access tools.
@@ -125,6 +127,44 @@ or keep the planner stub; 4o → GPT-mini deployment swap; flip `enable_slow_pat
 
 ---
 
+## What just shipped (2026-06-10 — TAB: tool-layer hardening + DRY)
+
+Owner-side companion to the agent block above. After the live E2E surfaced real-data
+edge cases, the tool layer got a round of correctness hardening. All in TAB-owned paths
+(`src/tools/`, `src/catalog/`); no agent-side or API change.
+
+**JSON-safety across the `analyze_*` family.** Real DB rows carry scalar types that
+don't survive the jsonb / SSE round-trip:
+- `[KM-630] coerce DB Decimal → float` (commit 1195870) — `_coerce_decimals` in
+  `invoker._materialize` converts object-columns holding `decimal.Decimal` (asyncpg
+  returns NUMERIC as `Decimal`) to `float64` before any compute runs. Fixes the
+  `float + Decimal` TypeError in `decomposition.analyze_contribution` **and** the whole
+  family in one seam — only touches columns that actually contain a `Decimal`.
+- `[KM-624] non-JSON-safe scalars in mode & top_value` (commit 6981ed3) — normalize
+  numpy / non-native scalars so descriptive + top-value outputs serialize cleanly.
+
+**Planner↔Tools registry alignment + Timestamp keys** (commit 4bb7623, `fix(tools)`):
+- `registry.py` — `analyze_descriptive.required` corrected `["data"]` → `["data",
+  "column_ids"]` to match the compute signature (`column_ids` has no default). Prevents
+  the Planner from emitting a call that's missing a required arg. `analyze_profile` stays
+  `["data"]` (its `column_ids` defaults to `None`).
+- `aggregation._clean` — group-by over a datetime column produced `pd.Timestamp` group
+  keys that aren't JSON-safe; now normalized to `.isoformat()` alongside the existing
+  numpy `.item()` branch.
+
+**DRY: single `SAMPLE_LIMIT` constant** (commit 6d46ba5, `[NOTICKET] refactor(catalog)`):
+- One source of truth in `catalog/introspect/base.py` (`SAMPLE_LIMIT = 3`, down from 5 —
+  token cost: sample values feed the planner prompt). Both introspection paths import it:
+  `catalog/introspect/tabular.py` and `pipeline/db_pipeline/extractor.py` (which dropped
+  its own local `= 3`). Dependency direction is pipeline→catalog (no circular import).
+  Stale test `test_sample_values_capped_at_five` updated to assert the real cap (3).
+
+**Audit result:** Planner↔Tools arg alignment swept end-to-end — 7/8 `analyze_*` tools
+already matched; the 1 mismatch (`analyze_descriptive`) is the fix above. Pattern A holds
+across all of them.
+
+---
+
 ## What just shipped (2026-06-08 — KM-626: slow-path agent layer)
 
 The rest of the slow path after the Planner (KM-567), from
@@ -276,7 +316,7 @@ per the architecture doc: Orchestrator slow-path expansion + TaskRunner + Assemb
 | # | Item | Owner | Status | Notes |
 |---|---|---|---|---|
 | 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). |
-| 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). |
+| 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). |
 | 7 | `BaseIntrospector` ABC (`catalog/introspect/base.py`) | B | `[x]` | Pre-existing; signature locked |
 
 ### Ingestion — shared catalog plumbing
@@ -324,8 +364,8 @@ per the architecture doc: Orchestrator slow-path expansion + TaskRunner + Assemb
 
 | # | Item | Status | Notes |
 |---|---|---|---|
-| 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 |
-| 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` |
+| 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.) |
+| 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`. |
 | 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`. |
 
 ### Agents + chat
@@ -334,7 +374,20 @@ per the architecture doc: Orchestrator slow-path expansion + TaskRunner + Assemb
 |---|---|---|---|
 | 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. |
 | 33 | Guardrails prompt (`config/prompts/guardrails.md`) | `[x]` | PR7-bundle — appended to `chatbot_system.md` so guardrails take precedence in conflict. |
-| — | 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. |
+| — | 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). |
+
+### Tools — slow-path "Tools" component (TAB)
+
+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).
+
+| # | Item | Owner | Status | Notes |
+|---|---|---|---|---|
+| — | 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). |
+| — | 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). |
+| — | Analytics registry (`tools/registry.py`) | TAB | `[x]` | KM-628 — `analytics_registry()`. `analyze_descriptive.required` = `["data","column_ids"]` (aligned to compute signature, commit 4bb7623). |
+| — | 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). |
+| — | 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). |
+| — | Tool tests (`tests/unit/tools/`) | TAB | `[x]` | analytics + data-access + invoker tests (gitignored). Incl. regression `test_decimal_columns_coerced_for_analyze_contribution`. |
 
 ### API surface
 
@@ -351,7 +404,7 @@ per the architecture doc: Orchestrator slow-path expansion + TaskRunner + Assemb
 | # | Item | Owner | Status | Notes |
 |---|---|---|---|---|
 | 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. |
-| 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). |
+| 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). |
 | 40 | IR validator tests (`tests/query/ir/test_validator.py`) | B | `[x]` | PR1 — 19 tests, all rules covered |
 | — | PII detector tests (`tests/catalog/test_pii_detector.py`) | B | `[x]` | PR1 — 26 tests (parametrized) |
 | — | Catalog validator tests (`tests/catalog/test_validator.py`) | B | `[x]` | PR1 — 5 tests |