fix(ci): restore main coverage across PostgreSQL and workspace E2E - #6893
Conversation
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe PR adds a conditional PostgreSQL migration for bytewise ordering of root filesystem paths, wires it into the migration schema, broadens the related ordered-query test, and updates a file-download accessibility E2E expectation. ChangesRoot filesystem collation
File download accessibility state
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested reviewers: Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
🚅 Deployed to the ironclaw-pr-6893 environment in ironclaw-ci-preview
|
github-actions
Bot
added
scope: db/postgres
🔎 Review · PR #6893
Submitted review →Reviewed the complete trusted base-to-head comparison. The PostgreSQL collation migration is correctly wired after V33, conditionally updates existing schemas, and aligns ordered projection range comparisons with authoritative paths. The ordered-query and workspace accessibility test changes match their surrounding contracts. No actionable findings identified. Automatic · PR opened · attempt 1 of 3 · completed in 1m 42s Run details
|
![ironloopai[bot]](https://avatars.githubusercontent.com/in/4116490?s=60&v=4){kind=small}
There was a problem hiding this comment.
🔍 Review complete · PR #6893
✅ No actionable findings
Reviewed the complete trusted base-to-head comparison. The PostgreSQL collation migration is correctly wired after V33, conditionally updates existing schemas, and aligns ordered projection range comparisons with authoritative paths. The ordered-query and workspace accessibility test changes match their surrounding contracts. No actionable findings identified.
Validation and technical details
- Inspected all four changed files and surrounding PostgreSQL migration, ordered-query, projection, and E2E code.
- Verified the trusted comparison refs: 62cf5fe..19faaca.
- Confirmed PostgreSQL supports changing a text column's collation while retaining the foreign-key relationship used by the ordered projection table.
git diff --check refs/ironloop/base..refs/ironloop/headpassed.python3 -m py_compile tests/e2e/scenarios/test_reborn_v2_file_download.pypassed.- Rust and database-backed tests were not rerun because
cargoand a PostgreSQL test connection were unavailable in the review environment. - Base:
main - Head:
agent/fix-main-coverage-collationat19faaca - Run:
ba3cfbe8-d2c9-4fec-9bbb-86b7a20d1b6a
Coverage ratchetReborn integration-tier coverageLine coverage (Reborn crates): 85.53% — 314903 / 368162 lines Per-crate breakdown (60 crates, lowest-covered first)
This table itself is informational and never gates the PR on its own — not the percentage, not the per-crate holes, not the 0-coverage callout. A separate coverage ratchet (dry-run until enforce=true; see tests/integration/coverage-floor.toml) can fail the build on specific configured floors. Exemptions (3 entry/entries excluded from the accounting above)
|
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
This branch was successfully deployed
1 participant
Summary
"C"collation as authoritative filesystem pathsChange Type
Linked Issue
Related #6890
Validation
cargo fmt --all -- --checkcargo clippy --all --benches --tests --examples --all-features -- -D warningscargo buildironclaw_filesystemcrate suite against PostgreSQL; the exact filesystem suite under LLVM coverage; the failing Playwright scenariocargo test --features integrationif database-backed or integration behavior changedpgvector/pgvector:pg16database and verified the projection path collation isC; also applied the embedded migration chain to a fresh databasereview-prorpr-shepherd --fixwas run before requesting reviewAdditional lint evidence:
cargo clippy -p ironclaw_filesystem --all-targets --all-features -- -D warnings.Test Strategy
User behavior: Ordered filesystem queries return projected descendants consistently regardless of the PostgreSQL database locale. Selecting a workspace file keeps manually expanded sibling branches visible while maintaining one keyboard tab stop.
Risk areas:
Tests added or updated:
postgres_ordered_index_projects_rows_under_long_prefixes; existing keyset and no-backfill PostgreSQL contracts reproduce the locale regression and pass after V34.test_workspace_tree_keyboard_navigation_and_accessibilityto assert the preserved expanded item remains visible, unselected, and outside the roving tab stop.pgvector/pgvector:pg16image as CI.What the tests prove: V34 repairs bytewise descendant ranges on both fresh and populated locale-backed databases; ordered projections remain write-maintained without backfill; valid keyset pagination returns rows; the workspace tree preserves expansion without creating a second tab stop.
Commands run:
DATABASE_URL=postgres://... IRONCLAW_REQUIRE_POSTGRES=1 cargo llvm-cov test -p ironclaw_filesystem --test db_root_filesystem_contract -- --nocaptureDATABASE_URL=postgres://... IRONCLAW_REQUIRE_POSTGRES=1 cargo test -p ironclaw_filesystempython3 -m pytest tests/e2e/scenarios/test_reborn_v2_file_download.py::test_workspace_tree_keyboard_navigation_and_accessibility -v --timeout=120cargo fmt --all -- --checkgit diff --checkcargo clippy -p ironclaw_filesystem --all-targets --all-features -- -D warningsSecurity Impact
None. This does not change permissions, network access, secrets, sandboxing, or tool execution.
Reborn Trust-Boundary Checklist
N/A: this patch changes a PostgreSQL path collation migration and test assertions only. It introduces no trust-bearing types, prompt content, hashes, status/error variants, queues, drivers, or host/sandbox boundaries.
Database Impact
Adds PostgreSQL migration V34. It changes
root_filesystem_ordered_index_rows.pathfrom the database-default collation to"C", matchingroot_filesystem_entries.path. Stored values and projection rows are unchanged. PostgreSQL may rebuild the dependent primary-key index and takes the schema lock required byALTER COLUMN; the migration is conditional and idempotent. libSQL is unaffected because its ordered path comparisons are already bytewise.Blast Radius
PostgreSQL ordered exact/prefix filesystem projections and the workspace-tree E2E assertion. The migration can briefly block concurrent access while PostgreSQL alters the column and rebuilds dependent indexes. No trait, dependency, serialization, or product contract changes.
Rollback Plan
Revert the code/test commit after stopping ordered readers and writers, then apply the documented reverse DDL to restore
root_filesystem_ordered_index_rows.pathtoCOLLATE "default"; recreate dependent indexes if PostgreSQL reports them invalidated. Existing rows require no data conversion. Rolling back reintroduces locale-dependent descendant-query failures, so it is operationally safe only where the database default collation has compatible bytewise ordering.Review Follow-Through
Reviewer judgment requested on the migration lock window for unusually large ordered-projection tables. No data rewrite or backfill is introduced, and the exact main-coverage failures are reproduced and fixed locally.
Review track: C (DB/CI)