Repository navigation
Scope grounded queries and return typed agent SDK pages - #2
Conversation
…ion, pipeline robustness Surface column values into the grounding/generation prompts and harden the query + ingestion pipelines so the NL→SQL agents stop guessing literals and the control flow stops reporting wrong-but-empty results as confident answers. Grounding / value anchoring: - profile: store the full distinct value set for low-cardinality columns (was top-5) - retrieval: value_fingerprint() on hits; render a "Column value catalogue" into the grounding and generation prompts; instructions to copy WHERE/CASE literals verbatim, handle tall/EAV layouts and scaled-duplicate measures - structurally detect self-referencing hierarchy columns (value containment, with a row-wise discriminator) and teach "a person's team/reports/structure" -> filter the hierarchy column Control flow / governance: - treat 0-row / degenerate aggregate results as suspicious -> clarification + confidence cap - gate auto-learning on row_count > 0 (stop poisoning the example store) - prefer a candidate that passes the firewall and returns non-empty rows over candidates[0] - exclude CTE names from AST table_refs (firewall no longer rejects valid CTE SQL) - snapshot-scope retrieval; honor and persist conversation snapshot pins - unify /stream, :explain, :validate on the rendered prompts + firewall/scope guards - thread a re-firewalled extra_filter through the semantic-metric bind() Ingestion robustness / determinism: - header-quality scoring (skip numeric spacer rows) + stable leading-blank handling - temperature=0 for the naming/describe agents (cuts re-ingest schema churn ~94%) - rewrite Parquet column names on the no-key fallback (fixes profile/sample Binder errors) - index column values into embeddings + content_tsv - warn when the cross-encoder reranker degrades to a no-op
- builder.py is a lock-step (canon-pinned) module; revert it and pass the
determinism setting via the existing extra_settings={"temperature": 0.0}
instead of a new parameter, so the lockstep gate stays green
- move the table-kinds lookup out of the query controller into TableResolver
(service layer) so the "no raw SQL in controllers" gate passes
- ruff: inline a negated return (SIM103), drop a forward-ref quote (UP037),
and apply ruff format
- tests: extend the fake TableResolvers with pins / current_snapshots /
table_kinds_by_name; seed current_snapshot_id in the retrieval integration
fixture (retrieval is now scoped to current_snapshot_id, as publish sets it)
Financial exports (Orbis/BvD) place one date-header row above a stack of sub-sections. Section-splitting orphaned that header above each sub-section's title, dropping the year columns and making the detailed financial tables (Ratios de Rentabilidad, Memo lineas, etc.) unqueryable by period. A data-first section now inherits the nearest recent period header as its column row, encoded as a backward-compatible 3-index section path ([header:data_start:end]); contiguous sections keep the legacy 2-index form. Guards against misfires: - period values are full-match only (INV-2024-0007 / Form 2020 are not years) - only a single leftmost label column may precede the inherited years - a label-headed table closes the period band (no stale-header bleed) - phantom null columns are compacted; header-row index is bounds-checked Adds unit coverage for inheritance, band-closing/no-false-inherit, path round-trip and legacy-path parsing.
…anch Reconciles the two parallel lines of the NL→SQL quality work into a single branch that carries every refinement from both, with no capability lost. Kept from this branch (execution-layer robustness): - TableResolver / AstClassifier / profile hardening and the query_controller surface, plus their unit/integration tests. - The XLSX multi-row period-header reconstruction (embedded-year rejection, band-closing, 3-index section paths) and its test. - PII-safe value rendering in the embed corpus. Brought in (the value-anchoring superset): - value_anchoring.py: the dataset-agnostic backbone (per-column value catalogue, value→column + group resolution, entity disambiguation by live row-match count + hierarchy intent, degenerate/zero-row detection, signed (mixed-sign) measure handling incl. the SUM(CASE WHEN cost THEN -m) shape, CTE-aware guards, function firewall). - query_service orchestration: candidate execution-selection, a non-destructive repair loop (never replaces a correct answer with a worse one), zero-row and signed-measure repair, group-coverage advisory. - Deterministic period/date column naming at ingest (year_YYYY / period_YYYY_MM_DD), original-header preservation, and the value-anchoring prompt rules for grounding / generation / critic / explainer. - LexicalReranker as the dependency-free default (a strict win over the no-op). - temperature=0 for the deterministic ingest agents (describe / column-name / rename / relation), applied via the existing extra_settings hook. Cross-line graft: the column hierarchy hint (references_column, computed by the profile stage) is now surfaced in the retrieval column fingerprint, so a self-referencing manager/owner column tells the agent to filter THAT column to find a person's reports instead of their own row. Gate notes: - builder.py stays byte-identical to canon (lockstep) -- temperature=0 rides the existing extra_settings path, no new parameter. - ruff check + format clean; lockstep 21/21. - test_query_service: the _FakeSettings stub gains the value-anchoring knobs (features off, so the orchestration tests stay focused on the core answer() flow), and the retry-on-error test now uses a realistic non-degenerate refined SQL with a table-aware resolver so the critic round is actually exercised.
miguelgfierro
left a comment
miguelgfierro
left a comment
There was a problem hiding this comment.
Está bien, he puesto algunos comentarios, pásaselos a la IA
| dataset_id: uuid.UUID, | ||
| table_names: list[str], | ||
| object_store_base: str | None = None, | ||
| pins: dict[str, str] | None = None, |
There was a problem hiding this comment.
resolve() tiene un parámetro pins pensado para fijar una pregunta de drill-down al snapshot del esquema con el que se respondió la primera vez, para que un re-ingest a mitad de conversación no cambie la respuesta sin avisar. Pero ninguno de los tres puntos de llamada en query_service.py (líneas 1166, 1281, 1384) pasa pins=, y current_snapshots() (línea 122) nunca se llama. prior_snapshot_pins simplemente se guarda y se recarga sin cambios (línea 988). Ahora mismo esta función no hace nada — un re-ingest entre dos mensajes de la misma conversación cambiará la respuesta al nuevo esquema sin que nadie se entere.
| END AS score | ||
| FROM flyquery_examples | ||
| WHERE workspace_id = :workspace_id AND quality = 'APPROVED' {ds_filter} | ||
| WHERE workspace_id = :workspace_id AND quality IN ('APPROVED', 'PROPOSED') {ds_filter} |
There was a problem hiding this comment.
Esta consulta ahora trae quality IN ('APPROVED', 'PROPOSED') en lugar de solo 'APPROVED'. Las filas PROPOSED vienen de auto_learner.py, que guarda un ejemplo cada vez que una consulta se ejecutó una vez, sin reintentos, y devolvió más de 0 filas — eso no es lo mismo que "correcta". Estos ejemplos se le muestran al LLM bajo el título "approved Q→SQL examples", sin forma de distinguir lo aprobado de lo no revisado. Una consulta incorrecta pero plausible ahora puede auto-enseñarse a preguntas futuras antes de que ningún humano la revise.
| gen_out = await generation_agent.run( | ||
| {"grounded": grounded, "question": body.question, "starting_point_sql": None} | ||
| gen_run = await generation_agent.run( | ||
| _render_generation_prompt( |
There was a problem hiding this comment.
answer() (la ruta principal /query) pasa los nuevos datos de grounding — catálogo de valores de columna, entidades/grupos resueltos, pistas de jerarquía, pistas de valores — a los constructores de prompts. /explain (línea 398), /validate (línea 465) y la ruta SSE /query/stream (líneas 607 y 641) llaman a los mismos constructores de prompts pero sin pasar nada de esto, así que no reciben ninguno de los aportes de grounding de este PR. El código de streaming incluso tiene un comentario que dice que coincide con la ruta síncrona, y no es cierto. Además, la línea 660 deja fijo snapshot_pins: {} en la respuesta del stream — nunca se rellena de verdad.
| mixed = False | ||
| try: | ||
| if mn is not None and mx is not None and float(mn) < 0 < float(mx): | ||
| mixed = True | ||
| except (TypeError, ValueError): | ||
| pass |
There was a problem hiding this comment.
mixed_sign se marca como verdadero cada vez que el mínimo de una columna es negativo y el máximo es positivo — no se comprueba nada más. Una columna de ingresos con 9.998 filas positivas normales y 2 filas negativas de reembolsos se marca exactamente igual que una columna que de verdad es mitad positiva, mitad negativa por diseño. Esa marca luego dispara una instrucción estricta de "SIGN ERROR — must fix" en query_service.py, incluso en casos donde restar es lo correcto. Conviene comprobar qué porcentaje de filas es negativo, no solo si hay alguna.
| reranker_top_n: int = 30 | ||
| query_expansion_enabled: bool = False | ||
|
|
||
| # Value-anchoring / grounding-quality knobs. The ingest pipeline already |
There was a problem hiding this comment.
por qué la config lo tenemos aquí en vez de en el .env?
| # computes a per-column value catalogue (profile_json.top_values, min/max, | ||
| # distinct_estimate) + a semantic_type; these control how it is surfaced to | ||
| # the SQL writer at query time. All dataset-agnostic. | ||
| value_catalog_enabled: bool = True # render real column values into prompts |
There was a problem hiding this comment.
value_catalog_enabled se define aquí pero nunca se comprueba en ningún sitio — column_value_catalog() siempre se ejecuta, sin importar esta opción. Ahora mismo no hay forma de desactivar esta función. (Relacionado: render_column_catalog_line() en value_anchoring.py línea 150 también está muerta — nada la llama; todos los puntos de llamada reales usan render_catalog_from_meta(). Vale la pena borrar ambas, o conectar la opción si de verdad debe hacer algo.)
| conn = duckdb.connect(":memory:") | ||
| conn.execute("SET threads=2") | ||
| for t in terms: | ||
| mq = t["measure_col"].replace(chr(34), chr(34) * 2) |
There was a problem hiding this comment.
chr(34) es confuso, sería más sencillo usar '"'
…nto fix/review-nl2sql-grounding-20261014
ancongui
changed the title
ancongui
changed the title
3 participants
Queries now reject missing or unauthorized dataset scope before retrieval or model work. Candidate probes stay inside registered snapshots, conversations retain their selected snapshots, and sampling/profile privacy decisions survive into embedding and retrieval projections.
Seven agent-list SDK methods previously discarded successful responses and returned
None. The API now declares its existing paginated DTOs, and Python/Java clients deserialize table, search, snapshot, object, change, dataset and conversation pages. Their changed examples use executable scoped clients.The release also applies the configured Anthropic instruction-cache hints during agent construction and consumes public fireflyframework-agentic 26.10.1 through a checked dependency lock. The qualified pyfly 26.06.113 runtime is retained.
Validation: 451 unit tests; 109 native PostgreSQL/pgvector, Redis and MinIO integration tests; SDK25 tests and type checks; Java25 Maven verification. Both installed SDKs read all seven typed pages against the actual service with nonbypass PostgreSQL roles. All fourteen changed documentation examples compile and run. Independent source/installed-SDK review passed.
Fifteen optional storage cases remain skipped and paid-model tests are excluded. Nonempty candidate selection does not establish question semantics. The engine's statement-timeout setting remains unenforced and it does not provide an operating system sandbox. Tenant/workspace headers select scope; user authentication belongs to the deployment gateway, while agent routes verify issued tokens and scopes.