ci(backend): replace shards with exact-custody semantic lanes#198
Conversation
Co-authored-by: Konan <kixeyems0@gmail.com>
|
Temporarily closing and reopening to retrigger missing GitHub Actions check suites. The exact reviewed head remains d3b2a65; no code or evidence changed. |
📝 WalkthroughWalkthroughThe PR replaces backend CI sharding with four concurrent semantic lanes, adds isolated PostgreSQL/MinIO custody and fail-closed evidence validation, restructures the hosted workflow, updates regression tests and operations guidance, and records chunk review artifacts. ChangesSemantic-lane backend CI
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant GitHub Actions
participant run_test_lanes
participant run_isolated_tests
participant MinIO
participant Evidence Validator
GitHub Actions->>run_test_lanes: collect exact pytest nodes
run_test_lanes->>run_isolated_tests: start four isolated lanes
run_isolated_tests->>MinIO: provision lane bucket and prefix
MinIO-->>run_isolated_tests: return probe and cleanup state
run_test_lanes->>Evidence Validator: validate nodes, digests, coverage, and timing
Evidence Validator-->>GitHub Actions: return fail-closed validation result
Possibly related issues
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
|
Exact-head hosted evidence for The repository owner explicitly accepted this measured timing risk on 2026-07-24 and directed that further runtime optimization be deferred. This acceptance does not waive or weaken test execution, coverage, isolation, evidence custody, or future timing measurement. PR #198 still requires the normal human review/merge decision. |
There was a problem hiding this comment.
Actionable comments posted: 14
🧹 Nitpick comments (10)
backend/scripts/run_isolated_tests.py (2)
262-285: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
prefixis unused in_drop_minio.The parameter is accepted but the loop deletes every object in the bucket regardless of prefix. That is safe for runner-owned buckets, but the signature implies prefix scoping. Either scope the listing with
Prefix=prefixor drop the parameter to keep the contract honest.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/scripts/run_isolated_tests.py` around lines 262 - 285, Update _drop_minio so its cleanup contract matches the prefix parameter: include prefix scoping in the list_objects_v2 request, or remove prefix from the function signature and all callers if bucket-wide deletion is intentional. Preserve the existing pagination, deletion, and bucket-removal verification behavior.
37-40: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low value
shared_foundationsreuses a fixed bucket, so crash residue blocks later local runs.Every other lane gets a suffixed bucket, but
shared_foundationsalways claimsworkstream-artifacts. Against an ephemeral per-job MinIO that's fine; against a persistent local MinIO, a bucket left behind by a crashed run makes_create_miniofail withminio_namespace_collisionon every subsequent run with no recovery path. Consider documenting the reclaim step (or adding an explicit opt-in reclaim) in the operations guide.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/scripts/run_isolated_tests.py` around lines 37 - 40, Document the recovery procedure for the fixed shared_foundations bucket used by S3_TRAFFIC_BUCKET, including how operators can safely reclaim workstream-artifacts after a crashed local run before retrying _create_minio. Keep the existing bucket behavior unchanged and place the guidance in the operations documentation.backend/tests/test_isolated_database_runner.py (3)
138-151: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd the missing-port and query/fragment cases to the endpoint matrix.
http://localhost:not-a-portexercises theValueErrorpath, not theparsed_port is Noneorquery/fragmentbranches of_minio_client. Two more parameters close the boundary on this fail-closed check.💚 Suggested additions
"http://minio.example.com:9000", "http://localhost:not-a-port", + "http://localhost", + "http://localhost:9000?probe=1", ],🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/tests/test_isolated_database_runner.py` around lines 138 - 151, Add endpoint parameters to test_minio_boundary_rejects_nonlocal_or_credentialed_endpoints for a localhost URL with no explicit port and URLs containing query or fragment components, ensuring _minio_client exercises the parsed_port is None, query, and fragment rejection branches while preserving the existing unsafe_minio_endpoint assertion.
188-192: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueThe fake clock is exhaustible, so it fails as
StopIterationrather than an assertion.
iter((0.0, 0.0, 60.0, 120.0))encodes the exact number oftime.monotonic()reads inside_run; adding one read to the implementation breaks this test withStopIterationinstead of a meaningful failure. A stepping fake that keeps returning its final value would be less brittle while still driving the timeout path.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/tests/test_isolated_database_runner.py` around lines 188 - 192, Update the fake clock used by the _run test to return successive timestamps while repeating the final timestamp for any additional monotonic() calls, instead of exhausting an iterator. Preserve the existing timing sequence so the timeout path remains exercised without raising StopIteration.
85-93: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueLane names are duplicated from
run_test_lanes.LANES.This parametrization hardcodes the four committed lane names, and
_minio_namespaceaccepts any grammatically valid lane, so a lane rename inbackend/scripts/run_test_lanes.pyleaves this test green against stale names. Deriving the lane list fromLANESwould keep bucket grammar bound to the real inventory.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/tests/test_isolated_database_runner.py` around lines 85 - 93, Update the parametrization associated with the isolated database bucket tests to derive lane names from run_test_lanes.LANES instead of hardcoding the four names. Preserve the expected bucket mapping and ensure the test exercises every lane in the actual LANES inventory, keeping bucket grammar validation tied to the committed lane definitions.backend/tests/test_ci_test_lanes.py (1)
284-345: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winNo coverage for a lane whose isolation metadata was never written.
Both
_finalize_lanetests pre-create{lane}.database.json, so the crash path flagged inbackend/scripts/run_test_lanes.py(Line 596 reads it unconditionally) is untested. Add a case where the isolation metadata file is absent and assert the lane row is still produced with a nonzeroexecution_exit_code.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/tests/test_ci_test_lanes.py` around lines 284 - 345, Add a _finalize_lane test that does not create the lane’s isolation metadata file, then assert finalization still returns a lane row with a nonzero execution_exit_code. Keep the existing coverage setup appropriate for the lane and target the unconditional isolation-metadata read path in _finalize_lane.backend/scripts/run_test_lanes.py (1)
210-228: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueConsider caching resolved caller paths.
_deterministic_uuid4runsPath(...).resolve()(a filesystem syscall) on everyuuid4()call while the patch is active. Keying a small cache oncaller.f_code.co_filenamewould remove repeated resolution during collection of thousands of nodes.♻️ Suggested caching
+_RESOLVED_CALLSITES: dict[str, str | None] = {} + + def _deterministic_uuid4() -> uuid.UUID: """Return a stable UUIDv4 for one repository callsite and ordinal.""" frame = inspect.currentframe() caller = frame.f_back if frame is not None else None try: if caller is None or _ORIGINAL_UUID4 is None: return uuid.UUID(bytes=os.urandom(16), version=4) - try: - relative = Path(caller.f_code.co_filename).resolve().relative_to(ROOT).as_posix() - except (OSError, ValueError): + filename = caller.f_code.co_filename + if filename not in _RESOLVED_CALLSITES: + try: + _RESOLVED_CALLSITES[filename] = ( + Path(filename).resolve().relative_to(ROOT).as_posix() + ) + except (OSError, ValueError): + _RESOLVED_CALLSITES[filename] = None + relative = _RESOLVED_CALLSITES[filename] + if relative is None: return _ORIGINAL_UUID4()🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/scripts/run_test_lanes.py` around lines 210 - 228, Update _deterministic_uuid4 to cache resolved caller paths keyed by caller.f_code.co_filename, and reuse the cached relative path on subsequent calls instead of invoking Path(...).resolve() each time. Preserve the existing fallback behavior for paths that cannot be resolved relative to ROOT and keep callsite ordinal generation unchanged..github/workflows/backend.yml (1)
395-401: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winPass
tree_shathroughenv:instead of expanding it into the shell.The value is a 40-hex SHA produced earlier in the same job, so this is not currently exploitable, but direct
${{ }}expansion intorunis what zizmor flags and it diverges from theenv:pattern already used by the hosted evidence step at Lines 249-251.♻️ Suggested change
- name: Reassert exact tree custody if: ${{ always() }} shell: bash + env: + EXPECTED_TREE_SHA: ${{ steps.identity.outputs.tree_sha }} run: | set -euo pipefail - test "$(git rev-parse HEAD)" = "${{ steps.identity.outputs.tree_sha }}" - test "$(git rev-parse HEAD^{tree})" = "$(git rev-parse "${{ steps.identity.outputs.tree_sha }}^{tree}")" + test "$(git rev-parse HEAD)" = "${EXPECTED_TREE_SHA}" + test "$(git rev-parse HEAD^{tree})" = "$(git rev-parse "${EXPECTED_TREE_SHA}^{tree}")"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/backend.yml around lines 395 - 401, Update the “Reassert exact tree custody” step to pass steps.identity.outputs.tree_sha through its env: mapping, then reference the environment variable in both shell checks instead of directly expanding the GitHub Actions expression in run. Preserve the existing commit and tree equality validations.Source: Linters/SAST tools
backend/scripts/validate_test_lane_evidence.py (1)
582-597: 🗄️ Data Integrity & Integration | 🔵 Trivial | 💤 Low valueConsider binding
elapsed_secondsto the lane timings.
elapsed_secondsis only checked for finiteness/non-negativity. Since the four lanes run concurrently, wall time must be at least the slowest lane and at most the aggregate (plus orchestration overhead); assertingslowest <= elapsed_secondswould catch fabricated or mismatched summaries that currently pass.♻️ Optional tightening
expected_aggregate = math.fsum(lane_elapsed_seconds) expected_slowest = max(lane_elapsed_seconds) + if float(summary["elapsed_seconds"]) + 1e-9 < expected_slowest: + raise EvidenceError("summary_timing_mismatch") if not math.isclose(🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/scripts/validate_test_lane_evidence.py` around lines 582 - 597, Bind summary["elapsed_seconds"] to the validated lane timings in the summary validation block: after confirming numeric validity, require it to be at least expected_slowest (with the existing floating-point tolerance) before accepting the summary. Preserve the aggregate and slowest consistency checks and raise EvidenceError("summary_timing_mismatch") for violations.backend/tests/test_test_lane_evidence.py (1)
303-321: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd cases for the remaining custody branches.
Uncovered validator rejections include
shared_isolation_namespace(two lanes reporting the same database/role/bucket/prefix),invalid_collect_mode_artifacts(collect-mode evidence carrying coverage or isolation metadata), andzero_current_test_modules. Those are the custody guarantees most worth pinning, and the coverage guideline expects ≥90% for new backend subsystems.As per coding guidelines: "New or materially changed backend subsystems must maintain at least 90% test coverage".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/tests/test_test_lane_evidence.py` around lines 303 - 321, Extend test_rejects_recorded_database_environment_or_shared_coverage with cases for shared_isolation_namespace, invalid_collect_mode_artifacts, and zero_current_test_modules. Construct each bundle mutation to trigger the corresponding validator.EvidenceError and assert its matching error code, preserving the existing custody rejection coverage and meeting the backend coverage requirement.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
@.agent-loop/initiatives/WS-CI-001-backend-ci-acceleration/reviews/WS-CI-001-02B-internal-review-evidence.md:
- Around line 63-65: Update the evidence record’s technical wording by replacing
“parametrized” with “parameterized” and “full service” with “full-service” in
all applicable occurrences, including the additional referenced occurrence.
In
@.agent-loop/initiatives/WS-CI-001-backend-ci-acceleration/reviews/WS-CI-001-02B-pr-trust-bundle.md:
- Around line 56-59: Update the documentation in the shard-topology rationale to
replace the unsupported “broke every PR” claim with either a citation to the
relevant failing run or a qualified description of the observed Ruff
resolver-drift failure mode. Preserve the surrounding claims about CI
acceleration and trust boundaries.
- Around line 48-51: Clarify the sentence describing the four-process rebalance
so it does not contradict the earlier shard-matrix replacement: if the workflow
structure changed but its required-job contract and gates did not, state that it
occurred without changing the final workflow contract or gates; otherwise revise
the sentence to accurately describe the workflow change.
In @.agent-loop/initiatives/WS-CI-001-backend-ci-acceleration/STATUS.md:
- Around line 10-16: Synchronize the gate evidence with the 02B review
artifacts: in
.agent-loop/initiatives/WS-CI-001-backend-ci-acceleration/STATUS.md lines 10-16,
describe the unfinished four-process gate as current work/history rather than
completed proof; in reviews/WS-CI-001-02B-internal-review-evidence.md lines
15-17 and reviews/WS-CI-001-02B-pr-trust-bundle.md lines 27-28, use the final
verified head bf16f1a6d01f270e8d25d3deee037431a8cb8676 and 2,049-node count.
In @.github/workflows/backend.yml:
- Around line 364-367: The timing policy must be made consistent across both
sites: in .github/workflows/backend.yml lines 364-367, either enforce the
controlled lane elapsed_seconds threshold while recording total wall time as
evidence, or implement the documented explicit-acceptance path instead of
unconditional SystemExit; in docs/operations_backend_testing.md lines 128-132,
if retaining the hard 480-second bound, remove the explicit-acceptance exception
and state that the required job fails outright above eight minutes.
In `@backend/scripts/run_isolated_tests.py`:
- Around line 181-188: Update _tree_sha to catch subprocess.SubprocessError from
git rev-parse and raise RunnerError with the appropriate stable error code,
preserving the existing invalid SHA validation. Ensure main continues converting
repository or Git failures into the standard “isolated-test runner failed”
response instead of allowing a traceback.
- Around line 239-242: Update the create_bucket exception handler in the
isolated test runner to re-raise KeyboardInterrupt and SystemExit before
converting ordinary failures into RunnerError("minio_namespace_collision"),
matching the probe block’s handling. Preserve cleanup state tracking so an
interrupt after successful bucket creation still allows _drop_minio to remove
the bucket.
In `@backend/scripts/run_test_lanes.py`:
- Around line 754-774: Update the cleanup flow around _signal_lane,
item.process.wait(), and unit_results so every force-killed process records a
synthetic interrupted unit before results aggregation. Ensure all four lanes
receive a finalized result, allowing _timing_summary and run-summary.json
generation to complete while preserving a nonzero execution outcome.
- Around line 591-599: The _finalize_lane evidence generation must tolerate
missing isolation metadata. In backend/scripts/run_test_lanes.py lines 591-599,
only include the metadata filename and SHA-256 when isolation_path is a regular
file; otherwise emit null values so finalization still writes run-summary
evidence. In backend/tests/test_ci_test_lanes.py lines 284-345, add coverage for
_finalize_lane without the database file and assert it returns a lane row with a
nonzero execution_exit_code.
- Around line 656-662: Move the collection failure guard in the flow around
collect_nodes above the build_manifest call and manifest file writes. When
collection_exit is nonzero or deselected is nonempty, raise
LaneError("pytest_collection_failed") immediately; otherwise continue building
and hashing the manifest.
In `@backend/tests/test_test_lane_evidence.py`:
- Around line 406-436: Ensure the test always restores the process-global UUID
patch and related state by moving cleanup into unconditional teardown around
pytest_collection_finish, including assertion or hook failures. Update the
test_collection_finish_restores_uuid4_and_repository_aliases flow to restore
uuid.uuid4, repository_module.imported_uuid4, and validator._UUID_* state before
propagating failures, while preserving the existing assertions and successful
collection behavior.
- Around line 353-403: Update _collect_current_nodes to remove inherited
PYTEST_ADDOPTS and PYTEST_PLUGINS from the child process environment before
spawning pytest, alongside the existing autoload and validator environment
overrides. Preserve the remaining copied environment so hosted evidence
collection stays independent of external pytest runner configuration.
In `@docs/operations_backend_testing.md`:
- Around line 68-73: Update the “Hosted semantic-lane full-suite proof”
documentation to distinguish MinIO from the PostgreSQL services container: state
that MinIO is launched with docker run --detach --rm, published on
127.0.0.1:9000, and validated by a step-level curl health loop, while only
PostgreSQL is configured as a digest-pinned services entry.
In `@scripts/test_agent_gates.py`:
- Around line 6494-6500: Update the regression test for the semantic-lane
workflow to locate and assert the presence of the separate “Require
semantic-lane execution success” step. Verify that this step reads or evaluates
.ci/test-lanes/run.exit and remains the gate that fails the job when a lane
fails, alongside the existing assertions for “Execute four semantic lanes.”
---
Nitpick comments:
In @.github/workflows/backend.yml:
- Around line 395-401: Update the “Reassert exact tree custody” step to pass
steps.identity.outputs.tree_sha through its env: mapping, then reference the
environment variable in both shell checks instead of directly expanding the
GitHub Actions expression in run. Preserve the existing commit and tree equality
validations.
In `@backend/scripts/run_isolated_tests.py`:
- Around line 262-285: Update _drop_minio so its cleanup contract matches the
prefix parameter: include prefix scoping in the list_objects_v2 request, or
remove prefix from the function signature and all callers if bucket-wide
deletion is intentional. Preserve the existing pagination, deletion, and
bucket-removal verification behavior.
- Around line 37-40: Document the recovery procedure for the fixed
shared_foundations bucket used by S3_TRAFFIC_BUCKET, including how operators can
safely reclaim workstream-artifacts after a crashed local run before retrying
_create_minio. Keep the existing bucket behavior unchanged and place the
guidance in the operations documentation.
In `@backend/scripts/run_test_lanes.py`:
- Around line 210-228: Update _deterministic_uuid4 to cache resolved caller
paths keyed by caller.f_code.co_filename, and reuse the cached relative path on
subsequent calls instead of invoking Path(...).resolve() each time. Preserve the
existing fallback behavior for paths that cannot be resolved relative to ROOT
and keep callsite ordinal generation unchanged.
In `@backend/scripts/validate_test_lane_evidence.py`:
- Around line 582-597: Bind summary["elapsed_seconds"] to the validated lane
timings in the summary validation block: after confirming numeric validity,
require it to be at least expected_slowest (with the existing floating-point
tolerance) before accepting the summary. Preserve the aggregate and slowest
consistency checks and raise EvidenceError("summary_timing_mismatch") for
violations.
In `@backend/tests/test_ci_test_lanes.py`:
- Around line 284-345: Add a _finalize_lane test that does not create the lane’s
isolation metadata file, then assert finalization still returns a lane row with
a nonzero execution_exit_code. Keep the existing coverage setup appropriate for
the lane and target the unconditional isolation-metadata read path in
_finalize_lane.
In `@backend/tests/test_isolated_database_runner.py`:
- Around line 138-151: Add endpoint parameters to
test_minio_boundary_rejects_nonlocal_or_credentialed_endpoints for a localhost
URL with no explicit port and URLs containing query or fragment components,
ensuring _minio_client exercises the parsed_port is None, query, and fragment
rejection branches while preserving the existing unsafe_minio_endpoint
assertion.
- Around line 188-192: Update the fake clock used by the _run test to return
successive timestamps while repeating the final timestamp for any additional
monotonic() calls, instead of exhausting an iterator. Preserve the existing
timing sequence so the timeout path remains exercised without raising
StopIteration.
- Around line 85-93: Update the parametrization associated with the isolated
database bucket tests to derive lane names from run_test_lanes.LANES instead of
hardcoding the four names. Preserve the expected bucket mapping and ensure the
test exercises every lane in the actual LANES inventory, keeping bucket grammar
validation tied to the committed lane definitions.
In `@backend/tests/test_test_lane_evidence.py`:
- Around line 303-321: Extend
test_rejects_recorded_database_environment_or_shared_coverage with cases for
shared_isolation_namespace, invalid_collect_mode_artifacts, and
zero_current_test_modules. Construct each bundle mutation to trigger the
corresponding validator.EvidenceError and assert its matching error code,
preserving the existing custody rejection coverage and meeting the backend
coverage requirement.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: f882f692-9fa7-489e-8b81-00d5cd56fb78
📒 Files selected for processing (15)
.agent-loop/initiatives/WS-CI-001-backend-ci-acceleration/STATUS.md.agent-loop/initiatives/WS-CI-001-backend-ci-acceleration/reviews/WS-CI-001-02B-internal-review-evidence.md.agent-loop/initiatives/WS-CI-001-backend-ci-acceleration/reviews/WS-CI-001-02B-pr-trust-bundle.md.agent-loop/merge-intents/WS-CI-001-02B.json.github/workflows/backend.ymlbackend/scripts/ci_test_shards.pybackend/scripts/run_isolated_tests.pybackend/scripts/run_test_lanes.pybackend/scripts/validate_test_lane_evidence.pybackend/tests/test_ci_test_lanes.pybackend/tests/test_ci_test_shards.pybackend/tests/test_isolated_database_runner.pybackend/tests/test_test_lane_evidence.pydocs/operations_backend_testing.mdscripts/test_agent_gates.py
💤 Files with no reviewable changes (2)
- backend/tests/test_ci_test_shards.py
- backend/scripts/ci_test_shards.py
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
backend/scripts/run_isolated_tests.py (1)
240-248: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winPreserve async cancellation in MinIO provisioning.
_create_miniocatchesBaseException, soasyncio.CancelledErrorcan be translated intominio_namespace_collisionand stop cancellation from propagating. Catch onlyExceptionhere; if you need cleanup before cancel propagation, handle signals/termination explicitly or use afinallyblock.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/scripts/run_isolated_tests.py` around lines 240 - 248, Update _create_minio to catch only Exception when translating bucket-creation failures into RunnerError, allowing asyncio.CancelledError and other cancellation signals to propagate unchanged. Preserve the existing explicit KeyboardInterrupt and SystemExit handling and namespace-collision error behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
@.agent-loop/initiatives/WS-CI-001-backend-ci-acceleration/reviews/WS-CI-001-02B-external-review-response.md:
- Around line 51-59: Update the “Commands rerun” block to remove placeholder
inputs: replace the literal pytest selector in the test command and the
“<changed Markdown files>” argument with the exact selector and Markdown file
paths used, or explicitly label the block as pseudocode instead of executable
verification commands.
In `@backend/scripts/run_test_lanes.py`:
- Line 695: Update the exception handling in run_lanes to preserve and surface
unexpected BaseException details before continuing, including the message and
traceback using the module’s existing or newly imported traceback/sys utilities.
Ensure main() or the relevant failure result receives enough information to
report the original orchestration cause instead of only a nonzero exit code,
while retaining normal lane cleanup and continuation behavior.
---
Outside diff comments:
In `@backend/scripts/run_isolated_tests.py`:
- Around line 240-248: Update _create_minio to catch only Exception when
translating bucket-creation failures into RunnerError, allowing
asyncio.CancelledError and other cancellation signals to propagate unchanged.
Preserve the existing explicit KeyboardInterrupt and SystemExit handling and
namespace-collision error behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 88a9d01b-a726-4e64-889e-6df14f65d77d
📒 Files selected for processing (13)
.agent-loop/initiatives/WS-CI-001-backend-ci-acceleration/STATUS.md.agent-loop/initiatives/WS-CI-001-backend-ci-acceleration/reviews/WS-CI-001-02B-external-review-response.md.agent-loop/initiatives/WS-CI-001-backend-ci-acceleration/reviews/WS-CI-001-02B-internal-review-evidence.md.agent-loop/initiatives/WS-CI-001-backend-ci-acceleration/reviews/WS-CI-001-02B-pr-trust-bundle.md.github/workflows/backend.ymlbackend/scripts/run_isolated_tests.pybackend/scripts/run_test_lanes.pybackend/scripts/validate_test_lane_evidence.pybackend/tests/test_ci_test_lanes.pybackend/tests/test_isolated_database_runner.pybackend/tests/test_test_lane_evidence.pydocs/operations_backend_testing.mdscripts/test_agent_gates.py
🚧 Files skipped from review as they are similar to previous changes (7)
- docs/operations_backend_testing.md
- scripts/test_agent_gates.py
- .github/workflows/backend.yml
- .agent-loop/initiatives/WS-CI-001-backend-ci-acceleration/reviews/WS-CI-001-02B-internal-review-evidence.md
- backend/scripts/validate_test_lane_evidence.py
- .agent-loop/initiatives/WS-CI-001-backend-ci-acceleration/reviews/WS-CI-001-02B-pr-trust-bundle.md
- backend/tests/test_test_lane_evidence.py
…xact-custody-semantic-lanes
…xact-custody-semantic-lanes
…xact-custody-semantic-lanes
|
Addressed the refreshed CodeRabbit outside-diff cancellation finding in |
PR Trust Bundle
Chunk
WS-CI-001-02B— Exact-Custody Semantic Test LanesMerge intent:
.agent-loop/merge-intents/WS-CI-001-02B.jsonGoal
Replace arbitrary Backend shards with four concurrent dependency lanes while
proving every canonical pytest node, isolated resource, coverage byte, and
hosted timing outcome on the exact PR head.
Human-approved intent
../PLAN.md../chunks/WS-CI-001-02B-exact-custody-semantic-test-lanes.mdSigned Start Provenance
30101104403bcf1292e1a591e3e84bf8ee212ee7191d80741faimplementation.agent-loop/initiatives/WS-CI-001-backend-ci-acceleration/chunks/WS-CI-001-02B-exact-custody-semantic-test-lanes.md784bae53f72f6460b4b74799c1c9b1519565dabe2dfcf06f21e4dac843c768f5c7fce85d2a8e2a89Only independently verified signed automation state is canonical authority.
PR prose and checked boxes are navigation evidence, not authorization.
What changed
orchestrating four concurrent semantic test lanes.
authenticated per-lane evidence, and exactly one coverage combination.
namespace custody, redaction, heartbeat, and bounded cleanup evidence.
0.15.22in the workflow to remove resolver drift.a blocking 480-second wall-time ceiling.
Why it changed
The prior shard topology duplicated services and artifact fan-in while Backend
CI remained slow. A newly released unbounded Ruff version also broke every PR.
The replacement must improve critical-path time without hiding tests, sharing
mutable resources, weakening coverage, or trusting self-authored evidence.
Design chosen
One job starts pinned PostgreSQL and MinIO services, then a repository-owned
orchestrator runs four dependency lanes concurrently. Ordinary lanes delegate
resource custody to
run_isolated_tests.py; its own self-tests execute as aseparately classified admin phase inside canonical node evidence. An independent
validator recollects current pytest nodes and verifies exact head, nodes,
resources, coverage bytes, failures, and timings before combination.
Alternatives rejected
e22e9fba/c73b2893: rejected because it changedforbidden product tests, initiative artifacts, and lacked exact node custody.
artifact fan-in rather than dependency ownership.
producing self-consistent evidence.
Scope control
Allowed files changed
.github/workflows/backend.ymlbackend/scripts/ci_test_shards.py(deleted)backend/scripts/run_isolated_tests.pybackend/scripts/run_test_lanes.pybackend/scripts/validate_test_lane_evidence.pybackend/tests/test_ci_test_shards.py(deleted)backend/tests/test_ci_test_lanes.pybackend/tests/test_isolated_database_runner.pybackend/tests/test_test_lane_evidence.pydocs/operations_backend_testing.mdscripts/test_agent_gates.pyFiles outside scope
Product Behavior
Acceptance criteria proof
independently recollected and validated at the reviewed head.
bucket/prefix custody — implementation and adversarial tests pass; hosted
service proof remains required.
digest-drifted, and shared-custody evidence fails in focused tests.
coverage combine; global 78 and every protected 90 floor remain blocking.counts, coverage, and digests and blocks above 480 seconds.
Co-authored-bytrailer on implementation commit
161417ff; eligible source commits aree22e9fbaandc73b2893. Later custody repairs are Abiorh-authored.Tests/checks run
Result summary: Ruff passed; 60 focused tests passed; 100 Agent Gates passed;
two independent full collections agreed on 2,046 exact nodes; merge intent,
links, stale wording, and diff integrity passed.
Test delta
Tests added
Tests modified
Tests removed/skipped
test_ci_test_shards.pywith its obsolete shard implementation.CI integrity
continue-on-error, or path suppressionExternal review
External review response file, if findings are posted:
WS-CI-001-02B-external-review-response.mdReviewer results
Reviewed code SHA:
2dfcf06f21e4dac843c768f5c7fce85d2a8e2a89Reviewed at:
2026-07-24T16:03:45ZReviewer run IDs:
ci02b_senior_review,ci02b_qa_review,ci02b_security_review,ci02b_product_ops_review,ci02b_restart_arch_review,ci02b_restart_ci_review,ci02b_contract_gap,ci02b_source_audit,ci02b_test_delta_reviewRemaining risks
480-second target remain unproven until GitHub runs the exact PR head.
cleanup exceeds its bounded grace; evidence fails and operators must inspect
only the recorded resource identities.
Follow-up work
After merge and automated memory reconciliation, stop.
WS-CI-001-03remainsfuture planning only and is not declared as a successor.
Human review focus
Human ownership
Summary by CodeRabbit