fix(backfill): address Gemini review on /backfill-age/indexes (#206 follow-up)

[jphein]

Follow-up to #206 (squash-merged as c8df975) — addresses the two correctness points from Gemini's review that landed after the merge.

1. Presence probe checks pg_index.indisvalid (was pg_indexes view)

A CREATE INDEX CONCURRENTLY that fails partway leaves an INVALID index in the catalog. The pg_indexes view lists it, but the planner can't use it. The original _existing_age_indexes would see that name, report it already_present, and skip the rebuild — silently leaving the latency bug unfixed. Now we join pg_class/pg_namespace/pg_index and filter i.indisvalid, so an invalid index reads as absent → the route re-attempts it, and the rebuild's clear "already exists" error surfaces in the response to alert the operator to DROP the invalid one.

Verified the new probe SQL is valid against a real apache/age:release_PG16_1.6.0 graph (returns only the valid index).

2. Connection-leak hardening

If psycopg2.connect() succeeded but conn.autocommit = True raised, the original code leaked the connection — the finally lived in a separate nested try that was never entered. Now conn = None up front and the whole connect + autocommit + execute sequence is wrapped in a single try/finally with a conn is not None guard.

Tests

Two new tests on top of the existing 7 (9/9 green against merged main):

• test_presence_probe_filters_invalid_indexes — asserts the probe queries pg_index/indisvalid, not the view.
• test_connection_closed_when_autocommit_raises — asserts the connection is closed even when setting autocommit raises.

Branched fresh off current main (post-#206-squash) and cherry-picked only the review-fix commit, so this is a clean +74/−14 diff with no phantom re-introduction of the merged #206 change.

🤖 Generated with Claude Code

[gemini-code-assist]

Summary of Changes

Hello, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request addresses two correctness issues identified in the post-merge review of the backfill index functionality. It improves the reliability of index detection by filtering for valid indexes in the PostgreSQL catalog and ensures robust connection lifecycle management to prevent potential leaks during error states.

Highlights

• Index Presence Detection: Updated the backfill logic to query 'pg_index.indisvalid' directly instead of the 'pg_indexes' view, ensuring that invalid indexes are correctly identified as absent and can be properly rebuilt.
• Connection Management: Hardened the database connection handling by initializing the connection variable to None and wrapping the connection setup in a unified try/finally block to prevent resource leaks if setting autocommit fails.
• Testing: Added two new test cases to verify that invalid indexes are correctly detected and that database connections are reliably closed even when errors occur during initialization.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	/gemini review	Performs a code review for the current pull request in its current state.
Pull Request Summary	/gemini summary	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	/gemini help	Displays a list of available commands.

Customization

To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here.

Footnotes

1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

[Copilot]

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

[gemini-code-assist]

Code Review

This pull request improves the robustness of database index backfilling in backfill_routes.py by checking pg_index.indisvalid to filter out invalid indexes and restructuring connection management to prevent connection leaks if errors occur during initialization. Corresponding unit tests were added to verify these behaviors. The reviewer suggested wrapping the autocommit configuration and initial setup queries inside the inner try block to ensure that psycopg2.OperationalError exceptions (such as when the database is in recovery mode) are properly caught and recorded.

[gemini-code-assist]

The inner try/except psycopg2.OperationalError block currently only wraps psycopg2.connect. However, if the database is in recovery mode (e.g., post-OOM restart), the connection might succeed but subsequent setup queries (LOAD 'age' or SET search_path) will raise a psycopg2.OperationalError with a message like the database system is in recovery mode.

To ensure these recovery-mode operational errors are captured and recorded via _record_db_error(e) (preserving the observability pattern established in #97 and #108), the setup queries and autocommit configuration should also be wrapped inside the inner try block.

Suggested change

	try:
	conn = psycopg2.connect(dsn, connect_timeout=5)
	except psycopg2.OperationalError as e:
	_record_db_error(e)
	raise
	# CONCURRENTLY forbids an open transaction; autocommit each DDL.
	conn.autocommit = True
	with conn.cursor() as cur:
	cur.execute("LOAD 'age'")
	cur.execute('SET search_path = ag_catalog, "$user", public')
	try:
	conn = psycopg2.connect(dsn, connect_timeout=5)
	# CONCURRENTLY forbids an open transaction; autocommit each DDL.
	conn.autocommit = True
	with conn.cursor() as cur:
	cur.execute("LOAD 'age'")
	cur.execute('SET search_path = ag_catalog, "$user", public')
	except psycopg2.OperationalError as e:
	_record_db_error(e)
	raise