D1-44-F1 — reject unreadable optional fields - #45
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe read model now distinguishes absent optional properties from present properties that fail during presence checks or getter access. Pull-request, finding, and repository fields reject unreadable values. Tests cover rejection and null normalization. ChangesFail-closed optional property reads
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to The PR narrowly changes optional-field validation so present-but-unreadable values are rejected while genuinely absent fields remain accepted; reported validation checks pass, and no actionable merge-blocking risk remains after normal review. Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
|
@codex review Review ONLY PR #45 at exact CURRENT HEAD: This child repairs exactly one verified defect: D1-44-F1 Scope is exactly:
Review mandate: Verify only whether this repair correctly enforces:
Do NOT search for unrelated defects. Do NOT report:
For any claimed defect provide:
Reviewer output is evidence, not authority. |
|
@coderabbitai review Review ONLY PR #45 at exact CURRENT HEAD: This is a bounded two-file repair for: D1-44-F1 Review only:
Do NOT search for:
Reviewer output is evidence, not authority. |
🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
expected='72ee36e43371211a382f286557c974398b1fd0dc'
parent='04f751e5488b6ce8cf4f140f82a1f3dd740c8ed2'
echo '== HEAD =='
git rev-parse HEAD
echo '== expected commit exists =='
git rev-parse "$expected"
echo '== changed paths =='
git diff-tree --no-commit-id --name-only -r "$expected"
echo '== repair diff =='
git diff --no-ext-diff --unified=80 "$parent" "$expected" -- \
src/cockpit/read-model.ts \
tests/cockpit/read-model-invariants.test.ts
echo '== implementation outline =='
ast-grep outline src/cockpit/read-model.ts --items all
echo '== test outline =='
ast-grep outline tests/cockpit/read-model-invariants.test.ts --items allLength of output: 29540 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '== read-model helpers and affected readers =='
sed -n '80,115p;430,590p;630,760p' src/cockpit/read-model.ts
echo '== optional-field test references and surrounding assertions =='
rg -n -C 5 --glob 'read-model-*.test.ts' \
'defaultBranchRef|baseRef|filePath|pullRequests.*title|title.*pullRequests' \
tests/cockpit
echo '== changed test file remaining content =='
sed -n '1,360p' tests/cockpit/read-model-invariants.test.ts
echo '== exact diff statistics and whitespace check =='
git diff --check 04f751e5488b6ce8cf4f140f82a1f3dd740c8ed2 72ee36e43371211a382f286557c974398b1fd0dc
git diff --stat 04f751e5488b6ce8cf4f140f82a1f3dd740c8ed2 72ee36e43371211a382f286557c974398b1fd0dcLength of output: 30958 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '== shared reader import and declaration =='
sed -n '45,82p' src/cockpit/read-model.ts
rg -n -C 8 --glob '*.{ts,tsx}' \
'function readOwnProperty\b|const readOwnProperty\b|export .*readOwnProperty\b' \
src
echo '== focused cockpit read-model test files =='
fd -t f -e ts . tests/cockpit | sort | grep 'read-model'
echo '== malformed optional-value cases =='
rg -n -C 6 --glob '*read-model*.test.ts' \
'defaultBranchRef|baseRef|filePath' \
tests/cockpit
echo '== public exports that expose the snapshot reader =='
rg -n -C 3 --glob '*.ts' 'readCockpitSnapshot|from .*/read-model' src/cockpitLength of output: 18278
Static review of commit
I did not execute repository tests because the review sandbox does not permit execution of repository code. ✅ Action performedReview finished.
|
|
Codex Review: Didn't find any major issues. Nice work! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
1 participant
Purpose
This stacked validation PR repairs exactly one verified finding from parent PR #44.
Finding:
D1-44-F1Classification on parent HEAD:
CURRENTSeverity:
P2Repair candidate status:
FIXEDProtected parent
Parent PR:
#44
Parent branch:
cockpit/d1-read-model-contractExact affected parent HEAD:
04f751e5488b6ce8cf4f140f82a1f3dd740c8ed2This child must not mutate the parent directly.
Repair identity
Repair commit:
72ee36e43371211a382f286557c974398b1fd0dcRepair tree SHA:
a8b385e49bc715721f3a4bd4d5646be3e9b98d07Validated repair patch SHA-256:
FC373FF37C5F345E6471976F127656079064E641F1480C506680DC439D85614EPatch size:
7,964 bytesVerified defect
D1 optional-field handling collapsed:
into the same value.
That allowed throwing own getters on these optional fields to be accepted as valid null:
repository.defaultBranchRefpullRequests[].baseRefpullRequests[].titlefindings[].filePathProtected invariant
A genuinely absent optional property may be accepted as absence.
A PRESENT-BUT-UNREADABLE optional property must fail closed.
Exact repair scope
Exactly 2 changed files:
src/cockpit/read-model.tstests/cockpit/read-model-invariants.test.tsNo other files.
No package changes.
No dependency changes.
No public API changes.
No shared
readOwnPropertychange.No Autoflow changes.
No transport changes.
No adapter changes.
No PR #9 changes.
Repair shape
The repair adds only a D1-local private distinction between:
Semantics:
ABSENT
→ accepted as legitimate optional absence
VALUE
→ existing validation behavior
UNREADABLE
→ invalid / snapshot rejected
The shared domain reader remains untouched.
Independent validation
Fresh independent validation:
PASS
Finding status on candidate:
FIXED
Reproducer closure:
Tests
Focused invariant:
16 passedCockpit:
43 passed / 3 filesFull suite:
928 passed / 18 filesTypecheck:
PASS
Lint:
PASS
Build:
PASS
git diff --check:PASS
Authority boundary
This PR is intentionally DRAFT.
It is a quarantined validation child.
Do not merge it directly to main.
If validated, human integration must merge it upward into:
cockpit/d1-read-model-contractusing CREATE A MERGE COMMIT.
Parent PR #44 remains protected and untouched until that gate.
No Ready authority is granted by this PR creation.
Human merge authority is preserved.
Summary by CodeRabbit
Bug Fixes
Tests