Repository navigation
fix(blame): order a session's own edits by seq, not by timestamp - #172
Open
thegoodengineer wants to merge 1 commit into
Open
thegoodengineer wants to merge 1 commit into
thegoodengineer wants to merge 1 commit into
Conversation
SPEC §2 says seq is the ordering and readers must not rely on ts, but blameFile sorted every step by ts first, even two edits from the same session. When the clock stepped back between two edits, blame replayed them out of order: with originalFile it reported the pre-edit content as verified, without it a false divergence. Each step now sorts by the latest ts its session has reached on that path, so a session's edits follow seq while sessions still interleave by time. Logs whose timestamps never go back sort exactly as before.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
blameFilesorts every step by timestamp first, including two edits from the same session. When the recording clock steps back between two edits to one file, blame replays them in the wrong order and still marks the result verified. SPEC §2 rules that out:forkwalks seq, so the two verbs disagree about the same log. Here is a synthetic session in the same record shape as the demo fixture: a Write ofa b c, then an Edit ofbtoB, with the clock a minute behind by the time of the Edit (the same records the second test builds):Blame replays seq 9 first. It seeds from the Edit's recorded
originalFile, which hash-checks, and the create at seq 5 then lands on top and wins. So blame reports, and vouches for, the file as it was before the edit. WithoutoriginalFile(Codex, OpenClaw) the same reordering goes wrong the other way: the later edit has nothing to apply to, and blame reports a divergence, which the CLI prints as "proof the file changed outside structured edits".After:
Cause
seqonly breaks ties here. Each step now also carries a running clock: the latesttsits session's steps on this path have reached. The sort uses that clock in place oftsand keepsseqas the tie-break. Inside a session the clock never goes back and seq always goes up, so a session's own steps follow seq. Across sessions, steps still interleave by time. The docstring said "in timestamp order"; it now says which order applies where.When a session's timestamps never go back, the clock is just
ts, so the order is exactly what it was. To check that, I blamed every path in every text fixture and golden log with both sorts, with each session alone and with all of them together (13 sessions, 16 paths, 224 calls). Every result was byte-identical.Tests
test/blame-clock-skew.test.tshas five tests. The first three cover the symptoms and the last two pin the ordering between sessions:verified: false(divergence at seq 2).B, from the modify event. On main it fails: line 2 isbandverifiedistrue.file.deletestamped before the create it follows. The file must end up deleted. On main it fails: the delete replays first, and blame shows the created lines.The existing
blame,no-trailing-newline,atif-adapterandcline-sdk-adaptersuites pass unchanged.Not changed
fixtures/openclaw/edits.jsonlandfixtures/codex/edits.jsonlalready stamp file events a second earlier than the event just before them. That is ordinary adapter output, and a clock taken over every event would push those edits later relative to other sessions. SPEC settles the order inside a session. Across sessions a stepped-back timestamp can only be guessed at, however it is handled, so I left that part alone.seq, as before.Full suite 876 green; typecheck, lint and prettier clean on Windows.