🤖 refactor: classify replacement history with bounded row reads - #4221
🤖 refactor: classify replacement history with bounded row reads#4221ThomasK33 wants to merge 1 commit into
Conversation
|
@codex review Please review the current head. This prerequisite remains part of the complete cancellation phase. Generated with |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
🛡️ Codex Security Review · Automatically triggeredSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
|
Codex Review: Didn't find any major issues. 🚀 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". |
7c43744 to
dc2e3c8
Compare
|
@codex review Please review the current head after the complete phase was composed on current main. All known findings have fixes and replies in their owning layers; the eight-member phase stays held until all members are approved and green. Generated with |
|
Codex Review: Didn't find any major issues. Another round soon, please! 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". |
🛡️ Codex Security Review · Automatically triggeredSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
dc2e3c8 to
2c2d301
Compare
|
@codex review Please review the updated head Final integrated validation passes on #4191 Generated with |
🛡️ Codex Security Review · Automatically triggeredSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
|
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". |
--- _Generated with `xum` • Model: `unavailable` • Thinking: `unavailable` • Cost: `$unavailable`_ <!-- mux-attribution: model=unavailable thinking=unavailable costs=unavailable -->
2c2d301 to
beec25b
Compare
|
@codex review Please review updated head Final integrated validation passes on #4191 All eight members remain held until current-head review approval and required CI are complete. Generated with |
🛡️ Codex Security Review · Automatically triggeredSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: beec25b7bf
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
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".
| return await scanHistoryRows( | ||
| file, |
There was a problem hiding this comment.
Scan each history snapshot through the same file handle
When history changes between the outer handle.stat() and scanHistoryRows opening file again, the size and reverse-reset reads come from the first snapshot while tokens and row descriptors come from the second. For example, if an initially empty history is appended in that window, the new row's historySequence reaches createHistoryNumberEvidence(0) and the scan throws Invalid numeric source bound; an atomic replacement can similarly mix row evidence across inodes. File-stamp revalidation cannot handle this because the scan may fail before returning provisional evidence, so the row scanner should consume the already-open handle or otherwise capture size and bytes from one snapshot.
Useful? React with 👍 / 👎.
Adds the history-specific row adapter above #4219. It preserves ordinary-row parsing and legacy normalization, derives oversized-row identity without assembling whole payloads, and compares replay candidates using exact bounded byte ranges. The owning #4182 integration will use this adapter to finish the oversized-row review finding.
Both existing reset protections remain in force through shared scanner helpers: raw reset markers, including control-separated keys, and the reverse token recognizer. Protected or invalid-UTF-8 readable rows still count for identity collisions, but cannot grant replacement authority. Digests reject mismatches cheaply; matching digests still require exact byte comparison and rechecking the captured content digest. LF termination is excluded from replay equality. Callers retain responsibility for file provenance and publication-lock validation.
Small and oversized legacy array-coerced system rows retain collision identities but cannot qualify as replacement candidates. User and assistant role coercion remains supported; the small-row classification agrees with the streamed projection.
Validation: Final integrated validation passes on #4191
052fc084517c1a6fd8cfbecdf08c05b635358a32(4026 tests / 36442 assertions) and #420902651d365f63c9dc04d95744207f42f8065d6f35(4095 tests / 36847 assertions), across 54 affected suites each. Full source/test TypeScript andmake static-checkpass on both exact commits.Risk: classification or reset-probe drift could admit ambiguous history. Native-reader comparisons and existing provider privacy suites exercise those contracts. This layer remains inactive until #4182 is integrated and reviewed; it introduces no new persisted state or scan budget for lifetime history proofs.
This is one layer of the cancellation phase: #4214 → #4215 → #4219 → #4221 → #4182 → #4187 → #4191 → #4209. All eight PRs merge together after every member has current-head approval and passing CI.
Generated with
xum• Model:unavailable• Thinking:unavailable• Cost:$unavailable