# Query correctness verification

Date: 2026-09-05. Base commit: `6ce7648afa0b69d17b765c1bb20a6fe7c3fa061f`.
Owner: query reliability and correctness agent. Requested model: `gpt-6-astra`, reasoning `xhigh`.
The lead selected that configuration at dispatch. This worker has no separate served-model receipt.
No commits, pushes, production queries, package installs, or application deployments were made.

## Status and implementation

| Finding | Status | Implementation | Proof |
|---|---|---|---|
| F02 | Implemented and verified on synthetic fixtures | `bi/sql_source_validation.py:27` uses PostgreSQL AST scopes. Runtime loads qualified allowlist and policy identities at `services/bi_query_execution_service.py:3413`. Final rewritten SQL is checked at `:3501`, before each generated/approved/enrichment dispatch. | Seven adversarial source forms; six valid equivalents; empty policy; nested restrictions; USING/NATURAL joins; whole-row and correlated references. Real SQLite policies prove zero rejected dispatches. A real PostgreSQL role rejects an ungranted source. |
| F03 | Implemented and verified on synthetic fixtures | `bi/measure_semantics.py:16` derives known aggregation from SQL. `services/bi_federated_merge.py:872` uses explicit aggregation rules. Two-leg execution refuses absent keys. Empty authoritative groups never switch to the populated enrichment leg. Ambiguous multiple dimension values refuse rather than selecting a mode. | Independent 100+100=200 and 100+101=201 checks. Repeated client attribute=5000. IDs are never summed. Real PostgreSQL numeric values survive merge as Decimal. Empty/keyless regression tests assert the corrected behavior. Ratio 4/6 is computed from numerator 4 and denominator 6. |
| F05 | Implemented with explicit verification limits | `bi/semantic_obligations.py:75` returns satisfied/violated/unverified obligations. A verified simple input path can prove mandatory compiled predicates or their absence. Other shapes stay unverified. Inherited follow-up scope does not become a false omission. `QueryExecutionResult` carries obligations and an explicit confidence scope. | Direct and CTE predicates pass. Proven missing predicates violate. Equivalent IN syntax, OR, joins and inherited key scope remain unverified. Obligation evidence contains no customer values. Existing definition/domain checks remain in place. |
| F12 | Implemented and component-verified | `bi/source_admission.py:33` shares thread-safe per-source admission across service instances and event loops. Nested contexts do not acquire twice. SQL execution, connect and fetch consume the job deadline. A watcher cancels a running database query when the job stops. Existing query tracing now records admission, connection, execute and retrieval spans. Derived cohort SQL uses cap+1. | Twenty actual PostgreSQL tasks across separate service instances complete with peak two open connections. The deadline test stops a five-second synthetic DB statement within 1.5 seconds and closes the connection. Thirty matched observations per connection strategy appear below. |
| F14, owned portion | Verified | New value, membership, policy, denominator, implicit-column and driver tests supplement existing suites. | 462 tests passed in the combined run. A later two-file run passed 36 tests, including one new real concurrent-connection case. |

The lead owns London calendar integration, displayed totals, persisted receipt mapping, shared orchestration and full integration checks.

## Integrated review corrections

The integrated review found additional access and masking failures. They were fixed before the broader verification run.

- The shared entry point now checks the actual connection's Data vault membership before routing. A gold redirect checks the target again and uses its workspace and Data vault. Dashboard refresh checks the saved owner before creating the generator. Missing owners and revoked members are refused. Compose may intentionally start in a content vault; membership in the source Data vault authorizes the transition. `tests/test_insights_entrypoint_authorization.py` uses real isolated SQLite memberships. No forbidden case reaches routing or SQL dispatch.
- Approved CT replay now preserves the caller's date anchor, display cap, confirmation and execution flags. The lead owns that router fix. The parity fixture mocks authorization explicitly; the separate authorization tests exercise real membership checks.
- A real masking leak was reproduced. `SELECT a.email FROM public.allowed a LIMIT 1`, quoted equivalents, computed expressions and a later SELECT after a CTE returned unchanged SQL from the legacy masking helper. Final source validation still accepted that SQL. The new runtime helper, `apply_postgres_output_masking` in `bi/column_masking.py`, traces final output columns through scopes. It uses the existing mask templates. It preserves NULL values and raw filtering, ordering and grouping. Sensitive computed outputs, unproved sets, whole rows and physical column renaming are refused. Sets that do not reference sensitive columns remain valid.
- The lead challenged ordinal ordering and grouping. `ORDER BY 2` and `GROUP BY 2` now keep the original input expression when output 2 is masked. Real PostgreSQL returns the original top member and keeps all three original groups. Ambiguous renamed GROUP BY aliases are refused.
- PostgreSQL folds unquoted aliases. SQLGlot retains their written case. Scopes now normalize identifiers with PostgreSQL rules, while preserving quoted identity. User-provided SQLGlot case metadata cannot bypass normalization. Uppercase aliases and CTE output references have regressions.

Mask regression baseline, before the runtime call changed:

```sh
.venv/bin/python -m pytest tests/test_insights_scope_masking.py -q --tb=short
```

Result: **10 failed, 1 passed in 11.38s**. Log: `/tmp/insights-masking-before.log`.

Focused authorization and parity verification:

```sh
.venv/bin/python -m pytest tests/test_insights_entrypoint_authorization.py tests/test_insights_entrypoint_parity.py tests/test_compose_bi_name_splice.py -q --tb=short
```

Result: **15 passed in 24.51s**. An intermediate run had 4 failures in existing Compose name-splice doubles, because they had no metadata tables. Those fixtures now mock the shared authorization seam explicitly. The real authorization tests were not changed to allow forbidden access.

Masking and existing guard compatibility:

```sh
# Disposable PostgreSQL DSN supplied through INSIGHTS_TEST_PG_DSN.
.venv/bin/python -m pytest tests/test_insights_scope_masking.py tests/test_insights_query_postgres.py tests/test_sql_validator.py tests/test_column_masking.py tests/test_enrichment_validated_sql_transport.py tests/test_bi_query_execution_service_ssh.py -q --tb=short
```

Result: **237 passed in 14.76s**. The actual PostgreSQL test uses `execute_validated_sql`, real policy metadata, and an isolated read-only role. It checks `[[3, "[MASKED]"], [2, None]]` for a filtered CTE. Only transport configuration is injected at the real driver seam.

After ordinal/group and qualified whole-row hardening, the mask and PostgreSQL files passed **25 tests in 12.74s**. The added real checks return top member 3 when member 1 was inserted first. Three distinct source emails still yield three groups of count 1; two outputs are masked and one stays NULL. After alias-case hardening, the masking and SQL validator files passed **142 tests in 11.83s**.

Strict mypy with imported library types enabled passed the four new query modules and then the masking/source/entrypoint modules. The earlier `--follow-imports=skip` check had hidden SQLGlot type errors; those were fixed with explicit Query/Select narrowing and a typed predicate constructor map. Ruff and compilation passed. The lead owns subsequent broad checks and remaining auxiliary-source integration.

Masking has deliberate limits. Sensitive computed expressions and sensitive set projections refuse until their output policy is proved. This does not claim a complete privacy oracle. The final integrated review separately tracks trusted identity lookups, source authorization and exact cohort source selection.

The final metadata review found that SQL parsing must not fold exact stored names. `qualified_metadata_relation` now preserves case and quoted component boundaries for allowlists, discovery, restricted columns and mask policies. Real SQLite fixtures prove that `public."Accounts"` does not authorize `public.accounts`, that a dotted schema is distinct from a catalog-qualified source, and that embedded identifier quotes remain exact. The final three-file command was:

```sh
.venv/bin/python -m pytest tests/test_insights_scope_masking.py tests/test_sql_validator.py tests/test_insights_query_correctness.py -q --tb=short
```

Result: **176 passed in 13.46s**. One intermediate fixture failed because its synthetic table was named `secret`, which the unchanged credential-name guard blocks. The fixture was renamed to `items`; all identity and denial assertions were preserved.

The speed agent then completed central auxiliary-source authorization and policy enforcement. Its focused source/column-policy and driver run passed **67 tests in 13.53s**. The lead removed cohort source guessing: missing execution-source identity leaves the group context unready. These were independently assigned integrated fixes, not additional queries against production. The current integrated query review has no unresolved findings; the broader suite remains owned by the lead.

## Broader-suite fixture reconciliation

The first broader run ended with 48 failures, 27,515 passes and 21 skips in 649.13s. The lead assigned this worker seven failures across four files. A focused reproduction confirmed **7 failed, 33 passed in 22.19s**. No runtime defect was found in these seven cases.

| File | Failures | Exact cause | Correction |
|---|---:|---|---|
| `tests/test_bi_confirm_fast_path.py` | 2 | The success fixture created an empty allowlist. Empty now correctly denies physical sources. | Add the exact `public.affiliate_revenue` grant. Add two confirmation denial tests: empty policy and another schema. Check that neither costing nor SQL runs for denied sources. Preserve the exact successful rows. |
| `tests/test_bic010_bi_metric_rollups.py` | 2 | The fixture returned `MagicMock` as a validation context. Final validation copies a real dataclass. | Return `SQLValidationContext(allowed_tables=frozenset())`. Remove the validator mock. The actual validator now checks the constant query before and after its rewrite; the result stays `[[42]]`. |
| `tests/test_pd_aggregate_member_drill.py` | 1 | The fixture used bare table names for SQL that reads `pipedrive` and `ct` schemas. | Grant the exact qualified relations. Add a same-basename private-schema denial. Keep both valid drill shapes and the original missing-LIMIT rejection. |
| `tests/test_architectural_invariants.py` | 2 | Text searches required implementation literals in their old function/module after scoped validation and admission extraction. | Keep the shared execution-boundary checks. Exercise acquisition to prove the validated literal IP reaches the connection call and denied hosts never connect. Exercise the real validator for bare, qualified and CTE wildcard rejection plus a safe explicit-column query. |

Only these four test files changed. Runtime guards were not relaxed.

```sh
.venv/bin/python -m pytest tests/test_bi_confirm_fast_path.py tests/test_bic010_bi_metric_rollups.py tests/test_pd_aggregate_member_drill.py tests/test_architectural_invariants.py -q --tb=short
```

After correction: **42 passed, 18 deprecation warnings in 22.79s**. Ruff passed for all four files. Logs: `/tmp/insights-query-triage-before.log` and `/tmp/insights-query-triage-after.log`. The warnings concern existing Pydantic default timestamps. No network or production database was used. Other broader-suite failures remain with their assigned owners.

## Final timing acceptance audit

The previous federated result reported `max(gold.execution_time_ms, ct.execution_time_ms)`. That omitted parent admission, retries, parent checks and merge work. The wrapper now creates a parent using the existing `QueryTracer`, measures the complete call with `time.perf_counter`, and returns its actual wall duration. `trace.child_trace_ids` and each federation source's `trace_id` retain the source traces. Child durations are neither added nor substituted for the parent duration. Inline source execution restores the caller's trace context. Success, refused results, exceptions, timeout and cancellation finalize the parent in `finally`, preserving per-leg SQL on domain failures.

The existing timing snapshot now includes `required_checks_ms` and `merge_ms`. At federation level, required checks measure source authorization, the CT SQL gate, join-key validation, key/grain scans and disjoint-membership checking. Merge measures frame reduction, merge construction and dimension aggregation. Nested check intervals are subtracted so these two fields do not count the same work twice. Per-source validation and EXPLAIN retain their existing child phase fields. The parent check field does **not** claim a sum of concurrent source checks. There is no new tracing infrastructure.

```sh
.venv/bin/python -m pytest tests/test_prd003_federation_dispatch.py tests/test_query_tracing.py tests/test_prd004_federated_merge.py -q --tb=short
.venv/bin/python -m mypy --follow-imports=silent --strict src/institutional_kb/bi/query_tracing.py src/institutional_kb/services/bi_federated_execution_service.py src/institutional_kb/services/bi_federated_merge.py
.venv/bin/ruff check src/institutional_kb/bi/query_tracing.py src/institutional_kb/services/bi_federated_execution_service.py src/institutional_kb/services/bi_federated_merge.py tests/test_prd003_federation_dispatch.py
```

Results: **142 passed in 14.05s**, strict mypy passed all three source files, and Ruff passed. The initial direct `.venv/bin/mypy` launcher had a stale interpreter path (exit 127); running the installed module with the workspace Python succeeded. Logs: `/tmp/insights-federation-timing.log` and `/tmp/insights-federation-timing-mypy.log`.

Four added regressions use a real in-memory `QueryTracer` and synthetic dispatch:

- Two source threads rendezvous on a barrier and each wait 40 ms. Source-reported durations are deliberately 9,000 ms each. The parent matches an independent outer wall measurement within 25 ms and is below 9,000 ms, proving neither source maximum nor sum is substituted. Both completed children remain reachable from parent and source metadata.
- Merge dependencies inject two 20 ms holds inside key/grain checking and a separate 30 ms hold inside merge construction. Parent timing reports at least 40 ms of checks and 30 ms of merge. A deterministic clock test additionally proves a 70 ms enclosing interval with a 40 ms check records exactly 30 ms of merge.
- A rejected CT wildcard completes the parent with ERROR, records check work, performs no source dispatch, retains both leg statements and leaves no active phase timer.
- Cancellation during an inline source query completes parent and child as CANCELLED and leaves no active phase timer.

This audit ran no database, network or production call. It establishes instrumentation behavior, not production percentiles or a 10× improvement. A driver thread already running at cancellation remains governed by the separately tested driver deadline/cancellation mechanism; the inline regression does not emulate that transport.

## Before and after

The first new suite ran before behavior edits. Command:

```sh
.venv/bin/python -m pytest tests/test_insights_query_correctness.py -q --tb=short
```

Result: **11 failed, 6 passed in 12.12s**.

Observed failures:

- An empty allowlist accepted a physical table.
- Qualified allowed relations and quoted valid equivalents were rejected.
- The CTE name collision did not produce the required source rejection.
- Two separate deals of 100 returned 100.
- Deal identifiers 1 and 2 were summed to 3.

Some rejection tests were already red-safe for the wrong reason: the old extractor rejected the allowed qualified relation itself. The valid-equivalent tests exposed that distinction. The later runtime policy test verifies the complete allowlist-to-dispatch seam.

Existing tests also pinned unsafe behavior. The old cohort rescue expected 72 unrelated rows from an empty authoritative cohort. The old keyless tests expected a single-leg success from a two-leg plan. Those expectations now require empty membership or explicit refusal. Existing numeric fixtures now declare their intended grain. Assertions still check exact amounts and membership.

A definition-resolution fixture already mocked every post-compilation stage. Its mock now targets the structured verification stage. The new semantic tests run that stage without mocks.

## Exact test runs

Combined final behavior run:

```sh
# INSIGHTS_TEST_PG_DSN was set to the lead's disposable localhost PostgreSQL 16 container.
.venv/bin/python -m pytest \
  tests/test_insights_query_postgres.py \
  tests/test_insights_query_correctness.py \
  tests/test_prd004_federated_merge.py \
  tests/test_churn_tier_routing_and_merge_rescue.py \
  tests/test_dimension_aggregate_breakdown.py \
  tests/test_aggregate_cohort_materialization.py \
  tests/test_bi_query_execution_service_ssh.py \
  tests/test_sql_validator.py \
  tests/test_prd003_federation_dispatch.py \
  tests/test_enrichment_validated_sql_transport.py \
  tests/test_bi_definition_resolution.py \
  tests/test_rt018_bi_cost_estimator_runtime.py \
  tests/test_bi_zero_row_guard.py \
  tests/test_query_tracing.py \
  tests/test_filter_spec_validation.py -q --tb=short
```

Result: **462 passed in 19.89s**.

After adding the actual concurrent-connection test and a type-only string conversion:

```sh
.venv/bin/python -m pytest tests/test_insights_query_postgres.py tests/test_insights_query_correctness.py -q --tb=short
```

Result: **36 passed in 13.52s**. The PostgreSQL fixture creates and removes its own `insights_query_*` schema and restricted role. All rows are synthetic. One process ran each test command. No full suite ran in this worker.

Intermediate runs exposed fixture drift, two long messages, and one empty import produced while removing a temporary reexport. These were fixed. Final `py_compile` passes on all owned implementation files.

Final lint command:

```sh
.venv/bin/ruff check \
  src/institutional_kb/bi/{sql_source_validation,measure_semantics,semantic_obligations,source_admission,sql_validator,query_tracing,aggregate_cohort_materialization}.py \
  src/institutional_kb/services/{bi_query_execution_service,bi_federated_execution_service,bi_federated_merge}.py \
  tests/test_insights_query_correctness.py tests/test_insights_query_postgres.py
```

Result: **All checks passed.**

Focused type command:

```sh
.venv/bin/python -m mypy --follow-imports=skip --strict \
  src/institutional_kb/bi/sql_source_validation.py \
  src/institutional_kb/bi/measure_semantics.py \
  src/institutional_kb/bi/semantic_obligations.py \
  src/institutional_kb/bi/source_admission.py
```

Result: **Success: no issues found in 4 source files.** This skips imported modules. The lead owns the repository type check.

## Measured connection component

Fixture: PostgreSQL 16 on localhost. Three synthetic deals. Every SQL call computes the same sum, 301. Same restricted role, read-only session, SQL, data and freshness. Sequential concurrency=1. New connections are compared with an explicitly reused test connection. Production connection reuse was not enabled.

Recorded run, started at `2026-09-05T20:30:34.482756+00:00`:

| Component | Successful observations | Median | p95 |
|---|---:|---:|---:|
| New connection plus query and retrieval | 30 | 11.302 ms | 13.885 ms |
| Query and retrieval on the same open connection | 30 | 0.888 ms | 1.146 ms |

The later masking integration run also repeated the matched component fixture at `2026-09-05T20:52:23.464724+00:00`. New connection: median 11.407 ms, p95 13.546 ms, 30 observations. Reused connection: median 0.853 ms, p95 1.179 ms, 30 observations. The component trace recorded connection 311.36 ms, SQL execution 47.70 ms and retrieval 1.03 ms across that run. It did not measure end-to-end feature latency.

The existing trace recorded connection=308.09 ms, SQL execute=47.58 ms, retrieval=0.76 ms across this run. Its top-level total remains unset because this is a driver component fixture, not a full query-service request. Ordinary psycopg2 execute includes server execution and initial result transfer. Retrieval measures the fetch/row conversion loop.

The prior matched run measured new-connection median=12.371 ms/p95=13.701 ms and reused median=0.673 ms/p95=1.185 ms. These are component measurements. They do not measure HTTP, model work, tunnels, cold operating-system caches, or first useful UI result. They do not establish a full-feature speedup.

**Hypothesis:** Safe connection reuse may help where connection setup dominates. Production pooling requires separate reset, revocation, TLS/tunnel and transaction evidence. It was not enabled on these component measurements alone.

## Limits and rollout

- The source gate is process-wide. A deployment with multiple application processes needs shared external admission or a divided per-process budget. The tests do not verify a multi-process production budget.
- SQL cap+1 bounds returned cohort members. PostgreSQL may still scan or sort an entire input for DISTINCT. The actual statement deadline bounds duration. No claim is made that LIMIT alone bounds every scanned row.
- Cohort derivation refuses unsupported HAVING/QUALIFY shapes rather than deleting those filters. Their complete key set remains unavailable until a safe cohort query is supplied. Over-cap key sets remain explicitly incomplete; no arbitrary prefix becomes a complete group.
- Aggregation refuses unknown numeric grain, unsupported averages, rates without denominator columns, distinct counts without member identity, and distinct sums across groups. Those requests need corrected SQL or an explicit safe contract. Refusal is intentional.
- Structured verification does not prove every natural-language intent, date interpretation, ambiguity, business definition or follow-up constraint. These are unverified unless a listed check proves them. The existing London and domain guards still run. Generation confidence is not an answer certificate.
- A default pure-validator context uses `allowed_tables=None` for syntax-only callers. An explicit empty set denies all physical sources. Runtime contexts always supply a set. A server-pinned CT connection with no policy rows uses the checked CT source map. Any explicit allowlist policy takes precedence. Ordinary empty connections remain deny-all.
- Unqualified physical names resolve to public. PostgreSQL sessions now pin `search_path=public` and `default_transaction_read_only=on`. Other schemas must be named explicitly. This closes schema shadowing. Validate any operator-authored unqualified SQL for other schemas before rollout.
- No schema migration is needed for these owned changes. The lead persists the additional result metadata through the shared answer-record contract.
- Read-only role checks were executed in a disposable database. Production grants and tunnels were not inspected.