fix(meta): name the branch partition on every metadata statement

[nhobin219]

Closes CHA-546.

The 6 tx-log tables already resolved by partition name. The 8 metadata tables did not: ~80 sites in penca-storage-meta and penca-api/src/query/meta_plan.rs named the catalog-wide parent and let Postgres route on branch_uuid. This makes the rule uniform — every branch-scoped access to a partitioned table targets the branch's partition directly, reads included.

What it actually buys, measured

CHA-546's premise is only half true as originally written, and the difference matters enough that the PR pins it with an integration test rather than prose.

statement on a leaf	lock taken on the parent
SELECT	none
SELECT ... FOR UPDATE	none
DELETE	none
INSERT	AccessShare, held to commit
UPDATE	AccessShare, held to commit

Reads get the total fix. Writes get a downgrade, not an elimination: evaluating the leaf's partition constraint opens the parent for its partition key, so RowExclusive becomes AccessShare and stays held to commit. This is Postgres behavior, not a leftover parent-naming bug.

The read half is the larger win. Concretely, against lock_branch_teardown_partitions:

• Its EXCLUSIVE step conflicts with RowExclusive but not with AccessShare — so the downgrade clears it.
• Its DROP TABLE needs ACCESS EXCLUSIVE, which conflicts with every mode, including AccessShare — so the downgrade does not clear that, and an in-flight writer still delays the drops. That is a wait, not a cycle.

tests/integration/integration_cha546_partition_lock_footprint_test.py asserts both halves at the mode each can actually clear: branch writes proceed under a parent EXCLUSIVE, branch reads proceed under a parent ACCESS EXCLUSIVE. A single ACCESS EXCLUSIVE fixture for both — what the file did first — is a green no correct implementation can earn; it fails on an idle stack.

Deliberate exceptions to the partition-direct rule

Two, both enumerated in code:

1. DDL in crates/penca-db/src/dialect/pg.rs — CREATE TABLE ... PARTITION OF, ATTACH/DETACH, and the teardown lock step are parent operations by definition.
2. CHA-531's refcount gate in crates/penca-storage-meta/src/compact.rs — eligible_segment_delete_set_rows and reap_referenced_segment_delete_set_rows probe the three segment-metadata parents catalog-wide, deliberately: carry-forward crosses fork edges, so narrowing to one branch would collect a file a sibling still reads. segment_delete_set itself is unpartitioned, keyed on object_uri alone.

Acceptance criterion is machine-checked

tests/static/static_cha546_partition_naming_test.py scans the metadata SQL for parent-name construction, so the rule cannot silently regress. The two exceptions above are allowlisted there with their rationale.

Also fixed: integration_snapshot_list_cache_test.py matched pg_stat_statements on the parent name. Once the read path named leaves, that needle stopped matching anything — its sanity check went red, and every == 0 assertion in the file had quietly become a tautology. Added table_snapshot_segment_metadata_partition to the client naming module with parity goldens on both sides.

Testing

• just check
• Full integration suite on a fresh stack (branch CI skips integration entirely, so this is the real gate).

[nhobin219]

Penca PR Review Summary

Reviewed at 4c4515c. CI is green (Analyze x4, Rust clippy+fmt+test, Python lint + unit, fresh-clone examples; integration skipped by path filter, as expected on a branch).

No Critical findings. All five prioritized focus areas check out - details below. One Important finding (a stale comment this PR's own guard cannot catch) and five Suggestions.

Focus-area verification

1. Two-branch statement directions - correct. Traced all 7 statements in copy_inherited_snapshot and all 4 in copy_inherited_persist (fork_copy.rs), plus insert_carried_segment_indexes (segment_index.rs:495-606) and insert_carried_snapshot_segments (snapshot.rs:596-640). In every case rows are SELECTed from the parent/source partition and INSERTed into the child/this-branch partition, and the branch_uuid bind params agree with the relation names ($3 = source, $1/$2 = child). No swaps.
2. Refcount gate still catalog-wide - correct. The only surviving parent-helper call sites anywhere under crates/ (outside naming/tables.rs, naming/mod.rs, dialect/pg.rs) are compact.rs:347,355,365 and 523,524,525 - exactly the two CHA-531 gate functions. The branch-scoped sites in that file (compact.rs:57,94,138,168) were all converted.
3. read_and_classify_persist_segments uses its argument - correct. Both relation names are built from the branch_uuid parameter, and enumerate_base_cold_source demonstrably threads a fork's parent branch into it. The new comment documents exactly this.
4. No new relation does not exist - confirmed. Every converted read targets a leaf materialized earlier on the same path: create_catalog_tables -> ensure_metadata_branch_partitions for main/genesis, and ensure_branch_partitions (write/mod.rs:897) runs before materialize_fork_cold_references (write/mod.rs:960) inside one with_pg_tx. Both retention_floor.rs tests seed against the same branch_uuid they pass to create_catalog_tables. The only residual 42P01 is the pre-existing teardown race, which already applied to the hot data tables.
5. Lock-ordering docstrings accurate. Walked retire.rs:90-209 statement by statement: inside the tx it is one SELECT and four DELETEs against this branch's partitions, then insert_segment_delete_set_rows last. So the new claim that retire's tx "holds no segment-metadata parent lock at all" is exactly right, and the LAST-ordering invariant on insert_segment_delete_set_rows correctly identifies teardown as the only remaining writer subject to it.

Important

• crates/penca-api/src/write/mod.rs:1108-1117 - a parent-lock mechanism comment that CHA-546 falsified, left unrewritten while every sibling paragraph in the same function was rewritten. See the inline comment on the hunk above it.

Suggestions

• crates/penca-api/src/query/meta_plan.rs:713 - the new parse_uuid line splits a comment from the statement it describes. Inline.
• tests/integration/integration_cha546_partition_lock_footprint_test.py:257 - the module-scoped fixture docstring understates cross-test coupling. Inline.
• Redundant double-parse of branch_uuid (~50 sites, follow-up-ticket shaped). Every converted function now parses branch_uuid twice: the new panicking parse_uuid(branch_uuid) for the partition name, then SqlValue::uuid_str(branch_uuid)? for the bind param (e.g. meta_plan.rs:713 then :726; the same shape repeats throughout purge.rs, segment_index.rs, fork_copy.rs, snapshot.rs). The second parse can never fail once the first has succeeded, so it is a redundant parse plus a permanently dead error path - binding SqlValue::Uuid(branch) removes both. Mechanical but ~50 sites, so better as a follow-up than as churn on this PR. (The panicking parse itself is fine: branch_uuid is check_opt_uuid-validated in all three gRPC validation modules, and it matches the existing treatment of catalog_uuid.)
• Commit docs: correct the parent-lock claims measurement falsified is missing a scope. The other 17 commits all carry one; docs(db) or docs(lifecycle) would fit. All 18 headlines are within 72 chars and the CHA-546 footers are present.
• .claude/memory/reference_roborev_severity_threshold_met_is_clean.md rides along in this PR (commit docs(agent): note roborev show --json exits non-zero pre-review). The content is good but unrelated to CHA-546 - this is the known symlinked-memory-dir leak. Harmless to merge; flagging only so that keeping it is a choice rather than an accident.

Strengths

• The measurement discipline is the best thing here. The PR does not assert the lock behaviour it needs - it measures it against PG 17 via pg_locks, publishes the per-statement table in the integration test's module docstring, and then corrects its own earlier claims where the measurement contradicted them (docs: correct the parent-lock claims measurement falsified, docs(db): correct the static test's parent-lock rationale). The result is that the residual AccessShare an INSERT/UPDATE takes on the parent is documented as a deliberate survivor rather than quietly assumed away.
• The integration test is a genuine red test, and says why the obvious alternatives are not. Locking ONLY the 8 parents is precisely the state teardown creates for every other branch, and the docstring enumerates three rejected designs (racing a real compact; a raw SELECT ... FOR UPDATE that passes pre-fix; a single ACCESS EXCLUSIVE fixture for both halves) so a later reader cannot collapse it back into one. Splitting into EXCLUSIVE-for-writes and ACCESS EXCLUSIVE-for-reads is exactly right: the read half then proves no parent lock rather than merely a compatible one.
• _ParentLockHolder is carefully built. Dedicated connection rather than the shared pool (a pooled connection handed back mid-hold would carry the locks to another caller), all 8 parents locked in a single statement because lock_timeout is per-statement, and a failure to acquire raises instead of silently running both tests with no locks held - which is the exact failure mode that would turn a red test green.
• The static guard has a converse. test_refcount_gate_still_probes_parents_catalog_wide pins the exact three-parent set and forbids any *_metadata_partition in those bodies, so narrowing CHA-531's gate cannot slip through the forward guard's whole-function exemption. Scoping the compact.rs allowance to two named functions rather than the whole file is the right granularity.
• The pg_stat_statements needle fix in integration_snapshot_list_cache_test.py is a real catch - the parent-name needle had quietly made every == 0 assertion vacuous, and the new Python naming.py helper carries a docstring explaining exactly that trap.
• Rust/Python naming parity kept in lockstep, with a golden added on both sides.
• No new .unwrap()/.expect(), no new .clone(), no SessionContext::new(), no query-count regressions, and no function-level imports anywhere in the diff.

[nhobin219]

Important: The paragraph immediately below this hunk (lines 1108-1117, starting // It cannot move inside.) is falsified by this PR and was not updated - even though every sibling paragraph in this same function was rewritten (this hunk at 1086-1106, and the delete-set ordering comment at 1248-1260).

It currently claims:

list_table_uuids_for_branch plans through QueryManager::plan, which reads the catalog-wide metadata parents BY NAME - so running it in the transaction takes ACCESS SHARE on every one of them [...] and it sets up the ACCESS SHARE -> ACCESS EXCLUSIVE upgrade at the drops that deadlocks two concurrent teardowns of DIFFERENT branches.

Post-CHA-546 that plan path names leaves, not parents: list_table_uuids_for_branch (crates/penca-api/src/query/meta_resolve.rs:872) -> resolve_table_metadata -> the meta_plan.rs reads this PR converted. So planning inside the transaction no longer takes ACCESS SHARE on any parent, and the cross-branch ACCESS SHARE -> ACCESS EXCLUSIVE upgrade cycle it describes can no longer form. The closing clause - "Partition-scoping the enumerations bought exactly that property; planning inside the lock gives it back" - is falsified for the same reason.

The conclusion still holds, but on different grounds worth stating explicitly: list_table_uuids_for_branch is a cold-capable read that can block on object storage, so moving it inside would stretch a transaction that holds EXCLUSIVE on this branch's leaves under a 5s lock_timeout.

This is worth fixing in this PR specifically because the PR's own guard cannot catch it: tests/static/static_cha546_partition_naming_test.py::test_no_open_cha546_todos greps only for the TODO(CHA-546) marker (correctly removed from this block), while that test's own comment names "the comments in pg.rs::lock_branch_teardown_partitions and write/mod.rs" as exactly the prose that "describe[s] something that no longer happens." One of the two named sites still does.

(Anchored here because 1108-1117 fall just outside the diff hunk.)

[nhobin219]

Suggestion: This new line landed between the comment and the statement that comment describes. The comment above ("Parse fallibly (unlike the panicking parse_uuid above): a malformed table_uuid surfaces as the same typed protocol error...") is about parse_meta_uuid(table_uuid, "table_uuid")? on the next line, but it now reads as annotating a panicking parse_uuid call - the opposite of its point.

Moving let branch = parse_uuid(branch_uuid); up one paragraph, next to let catalog = parse_uuid(catalog_uuid); on line 709, restores the pairing and groups the two panicking parses together.

Related, non-blocking: 13 lines further down this same function binds the same string fallibly via SqlValue::uuid_str(branch_uuid)?, so branch_uuid is now parsed twice with two different failure modes and the fallible one is unreachable for malformed input. SqlValue::Uuid(branch) would reuse the value already parsed here. The same shape repeats across the converted call sites - called out as a follow-up in the review summary rather than as churn here.

[nhobin219]

Suggestion: "neither test mutates state the other reads" is not quite accurate for this module-scoped fixture. test_branch_writes_proceed_under_parent_exclusive appends a row via write_and_persist and then runs snapshot, compact_persist_segments, and purge against the same table that test_branch_reads_proceed_under_parent_access_exclusive subsequently reads.

It is benign today - the read asserts only num_rows > 0, which survives anything the write test does - but the pair is order-dependent, and this docstring is the thing that would stop someone from strengthening the read assertion later and getting a surprise.

Either restate it accurately ("the read test asserts only non-emptiness, so it tolerates whatever the write test leaves behind") or make the fixture function-scoped and pay the DDL twice.