Skip to content

fix(db): prevent nested-loop observation scoring - #4168

Closed
r266-tech wants to merge 1 commit into
vectorize-io:mainfrom
r266-tech:r266/fix-observation-expansion-plan-4163
Closed

fix(db): prevent nested-loop observation scoring#4168
r266-tech wants to merge 1 commit into
vectorize-io:mainfrom
r266-tech:r266/fix-observation-expansion-plan-4163

Conversation

@r266-tech

@r266-tech r266-tech commented Sep 6, 2026

Copy link
Copy Markdown
Contributor

Summary

  • materialize the observation scored CTE and use an equality FULL OUTER JOIN, which PostgreSQL must execute as a hash or merge join even when connected_sources is estimated at one row
  • preserve the existing distinct shared-source score; unmatched connected sources collapse into a null-id group that the downstream candidates join discards
  • pin the planner-safe SQL shape with a structural regression test

This intentionally does not add MATERIALIZED to connected_sources: it is already referenced twice and therefore materialized by PostgreSQL, and an explicit keyword did not change the bad cardinality estimate in PostgreSQL 18.

Closes #4163

Validation

  • PostgreSQL 18 live semantic check: candidates sharing 3 and 1 connected sources still score 3.0 and 1.0
  • PostgreSQL 18 EXPLAIN ANALYZE, normal statistics: 73ms total versus 75ms for the original query in the same 37.7k-unit/150.8k-edge fixture
  • PostgreSQL 18 forced one-row estimate (actual ~10k): Hash Full Join retained; scoring join ~1.3ms
  • uv run --extra embedded-db pytest tests/test_observation_expansion_single_fetch.py tests/test_observation_expansion_scoring.py -q -n0 (3 passed, 7 skipped because local ML extras are unavailable)
  • uv run --frozen ruff check hindsight_api/engine/db/ops_postgresql.py tests/test_observation_expansion_single_fetch.py
  • uv run --frozen ruff format --check hindsight_api/engine/db/ops_postgresql.py tests/test_observation_expansion_single_fetch.py
  • uv run --frozen ty check hindsight_api
  • git diff --check

The repository-wide lint hook also ran; all Python checks passed, while the unrelated control-plane ESLint task could not start because @eslint/js is not installed in this checkout.

CI follow-up

The only red job is the free-threaded 3.14 full-suite run. Two of its three failures are the same untouched OpenCode/named-result failures present on recent unrelated runs for #4164, #4159, and #4158. The remaining test_parallel_fans_out_across_schemas short count is also outside this diff and passed 3/3 isolated repetitions locally. All other 14 executed jobs passed, including generated-file verification and Python 3.11–3.14 builds; no changed SQL/test failure was reported, so the verified head is unchanged.

@koriyoshi2041 koriyoshi2041 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Validated the exact head locally against PostgreSQL: the fused-query and scoring suites pass 10/10, including distinct shared-source scores, wide source arrays, per-entity caps, ordering, and the new plan-shape guard. Ruff check/format and diff hygiene also pass. The FULL JOIN preserves matched candidate scores while preventing an equality full join from degrading to the reported nested-loop shape.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

3 participants