Background
Post-review analysis of PR #1459 found three classes of finding that CodeRabbit did not surface. All three map to established review principles and apply across the codebase, not just that PR.
Gaps and proposed instructions
1. Dead code after refactor (TS path)
When a refactor adds early-return guards that redirect a subset of inputs to a new code path, the old branches for those inputs can become unreachable. CodeRabbit's current instruction asks for exhaustive behavioral coverage but does not prompt it to trace call chains and flag dead switch arms.
Proposed addition to **/*.ts path instruction:
When a refactor adds early-return guards that redirect a subset of inputs to a new code path, confirm the old branches for those inputs are removed or unreachable.
2. Weak test assertions on fallback code (spec path)
Tests that assert only mock call counts can pass even when a regression silently returns stale fallback data instead of freshly-fetched data. Similarly, tests covering code with fallback behavior often cover only the warm-cache case; the cold-start case (no prior state) exercises a different branch and should be covered separately.
Proposed addition to spec/test path instruction:
For tests that assert only mock call counts, confirm a corresponding return-value assertion exists; a regression that silently returns stale fallback data can satisfy a call-count check. For code with fallback behavior, verify both the cold-start case (no prior state) and the warm case (prior state exists) are covered, as they exercise different branches.
3. Async read-modify-write race on shared state (cache/state path)
When an async function reads shared mutable state before an await and writes it back afterward, a concurrent mutation during the await can cause the pre-capture snapshot to be written back as newer state, violating the mutation contract.
Proposed addition to the cache/state path instruction:
For async functions that read shared mutable state before an await and write it back after, verify the captured reference is still valid when the write executes; a concurrent mutation during the await can cause a stale snapshot to overwrite a newer state.
Status
Background
Post-review analysis of PR #1459 found three classes of finding that CodeRabbit did not surface. All three map to established review principles and apply across the codebase, not just that PR.
Gaps and proposed instructions
1. Dead code after refactor (TS path)
When a refactor adds early-return guards that redirect a subset of inputs to a new code path, the old branches for those inputs can become unreachable. CodeRabbit's current instruction asks for exhaustive behavioral coverage but does not prompt it to trace call chains and flag dead switch arms.
Proposed addition to
**/*.tspath instruction:2. Weak test assertions on fallback code (spec path)
Tests that assert only mock call counts can pass even when a regression silently returns stale fallback data instead of freshly-fetched data. Similarly, tests covering code with fallback behavior often cover only the warm-cache case; the cold-start case (no prior state) exercises a different branch and should be covered separately.
Proposed addition to spec/test path instruction:
3. Async read-modify-write race on shared state (cache/state path)
When an async function reads shared mutable state before an
awaitand writes it back afterward, a concurrent mutation during the await can cause the pre-capture snapshot to be written back as newer state, violating the mutation contract.Proposed addition to the cache/state path instruction:
Status
fix/zoo-gateway-models-session-cache(change is in.coderabbit.yaml)main