Skip to content

🤖 refactor: classify replacement history with bounded row reads - #4221

Open
ThomasK33 wants to merge 1 commit into
codex/effect-history-message-evidencefrom
codex/effect-history-replacement-rows
Open

🤖 refactor: classify replacement history with bounded row reads#4221
ThomasK33 wants to merge 1 commit into
codex/effect-history-message-evidencefrom
codex/effect-history-replacement-rows

Conversation

@ThomasK33

@ThomasK33 ThomasK33 commented Sep 11, 2026

Copy link
Copy Markdown
Member

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 #4209 02651d365f63c9dc04d95744207f42f8065d6f35 (4095 tests / 36847 assertions), across 54 affected suites each. Full source/test TypeScript and make static-check pass 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

@ThomasK33
ThomasK33 added this pull request to stack #4216 September 11, 2026 13:52
@ThomasK33

Copy link
Copy Markdown
Member Author

@codex review

Please review the current head. This prerequisite remains part of the complete cancellation phase.


Generated with xum • Model: unavailable • Thinking: unavailable • Cost: $unavailable

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 11, 2026

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review Completed 2026-09-11T19:37:23.937915Z beec25b Manual request
🔒 Security Review Completed 2026-09-11T19:32:52.923701Z beec25b Manual request
ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector

Copy link
Copy Markdown

🛡️ Codex Security Review · Automatically triggered

Security review completed. No security issues were found in this pull request.

Reviewed commit: 7c437440f5

View security finding report

Only the user who started this review can view the report in Codex.

ℹ️ About Codex security reviews in GitHub

This is an experimental Codex feature. Security reviews are triggered when:

  • You comment "@codex security review"
  • A regular code review gets triggered (for example, "@codex review" or when a PR is opened), and you’re opted in so security review runs alongside code review

Once complete, Codex will leave suggestions, or a comment if no findings are found.

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. 🚀

Reviewed commit: 7c437440f5

ℹ️ 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".

@ThomasK33

Copy link
Copy Markdown
Member Author

@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 xum • Model: unavailable • Thinking: unavailable • Cost: $unavailable

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Another round soon, please!

Reviewed commit: dc2e3c84bb

ℹ️ 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".

@chatgpt-codex-connector

Copy link
Copy Markdown

🛡️ Codex Security Review · Automatically triggered

Security review completed. No security issues were found in this pull request.

Reviewed commit: dc2e3c84bb

View security finding report

Only the user who started this review can view the report in Codex.

ℹ️ About Codex security reviews in GitHub

This is an experimental Codex feature. Security reviews are triggered when:

  • You comment "@codex security review"
  • A regular code review gets triggered (for example, "@codex review" or when a PR is opened), and you’re opted in so security review runs alongside code review

Once complete, Codex will leave suggestions, or a comment if no findings are found.

@ThomasK33
ThomasK33 force-pushed the codex/effect-history-replacement-rows branch from dc2e3c8 to 2c2d301 Compare September 11, 2026 18:38
@ThomasK33

Copy link
Copy Markdown
Member Author

@codex review

Please review the updated head 2c2d301fb94363f0a09bdb7cac4616a6df83f2af against its stack base. 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.

Final integrated validation passes on #4191 266cf35aac (4,009 tests) and #4209 d8c34d12ce (4,078 tests), each across 54 affected suites. Full source/test TypeScript and make static-check pass on both exact commits. The eight-member cancellation phase remains held until every member has current-head approval and passing CI.


Generated with xum • Model: unavailable • Thinking: unavailable • Cost: $unavailable

@chatgpt-codex-connector

Copy link
Copy Markdown

🛡️ Codex Security Review · Automatically triggered

Security review completed. No security issues were found in this pull request.

Reviewed commit: 2c2d301fb9

View security finding report

Only the user who started this review can view the report in Codex.

ℹ️ About Codex security reviews in GitHub

This is an experimental Codex feature. Security reviews are triggered when:

  • You comment "@codex security review"
  • A regular code review gets triggered (for example, "@codex review" or when a PR is opened), and you’re opted in so security review runs alongside code review

Once complete, Codex will leave suggestions, or a comment if no findings are found.

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Nice work!

Reviewed commit: 2c2d301fb9

ℹ️ 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".

---

_Generated with `xum` • Model: `unavailable` • Thinking: `unavailable` • Cost: `$unavailable`_

<!-- mux-attribution: model=unavailable thinking=unavailable costs=unavailable -->
@ThomasK33
ThomasK33 force-pushed the codex/effect-history-replacement-rows branch from 2c2d301 to beec25b Compare September 11, 2026 19:25
@ThomasK33

Copy link
Copy Markdown
Member Author

@codex review

Please review updated head beec25b7bf0bb4ba6b4089cb4f8c2fc9fcfbece6 against its stack base. This layer’s own source delta is unchanged; it is rebased onto the merged CI-gate prerequisite #4223 and the reviewed ancestor fixes. Please review against its stack base.

Final integrated validation passes on #4191 052fc084517c1a6fd8cfbecdf08c05b635358a32 (4026 tests / 36442 assertions) and #4209 02651d365f63c9dc04d95744207f42f8065d6f35 (4095 tests / 36847 assertions), across 54 affected suites each. Full source/test TypeScript and make static-check pass on both exact commits.

All eight members remain held until current-head review approval and required CI are complete.


Generated with xum • Model: unavailable • Thinking: unavailable • Cost: $unavailable

@chatgpt-codex-connector

Copy link
Copy Markdown

🛡️ Codex Security Review · Automatically triggered

Security review completed. No security issues were found in this pull request.

Reviewed commit: beec25b7bf

View security finding report

Only the user who started this review can view the report in Codex.

ℹ️ About Codex security reviews in GitHub

This is an experimental Codex feature. Security reviews are triggered when:

  • You comment "@codex security review"
  • A regular code review gets triggered (for example, "@codex review" or when a PR is opened), and you’re opted in so security review runs alongside code review

Once complete, Codex will leave suggestions, or a comment if no findings are found.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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".

Comment on lines +62 to +63
return await scanHistoryRows(
file,

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge 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 👍 / 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant