log_cache: skip unparseable entries instead of aborting fold#1
Open
Justin3546 wants to merge 1 commit intoevanpurkhiser:mainfrom
Open
log_cache: skip unparseable entries instead of aborting fold#1Justin3546 wants to merge 1 commit intoevanpurkhiser:mainfrom
Justin3546 wants to merge 1 commit intoevanpurkhiser:mainfrom
Conversation
The append-log can contain legacy entries that don't deserialize into the current WireItem schema — for example, historical ACTIONGROUP-* IDs, entries with fields that have since been narrowed from optional to required, or variants that no longer match. These are all best-effort replay data; a single entry the parser doesn't understand shouldn't abort the whole command. Previously fold_state_from_append_log returned on the first deserialization error, which made the CLI unusable against accounts with any old-format history in their log. Now we log a warning to stderr with the error location and skip the entry, advancing safe_offset so the state cache still moves forward. Fold continues, and the remaining entries apply normally. Added a test that seeds a log with a known-bad entry between two good ones and verifies both good entries end up in state.
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
1 participant
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.
Summary
fold_state_from_append_logcurrently bails out on the first line ofthings.logthat doesn't deserialize intoWireItem. On accounts with long Things Cloud history, the log can include legacy entries that the current schema doesn't recognize (historicalACTIONGROUP-*IDs, fields that are now required but used to benull, older variants ofOneOrMany, etc.), and any one of them kills every command the CLI runs — including--no-sync.On my account, the log has 21,733 entries and 9 of them trip this. The result is that the CLI exits with just
Corrupt log entry at …/things.logand never produces output.This PR changes the fold loop to:
safe_offsetpast the bad lineConceptually: the append-log is a best-effort replay log, and one unknown entry shouldn't be fatal. The behavior for partial trailing lines (the existing
fold_state_ignores_trailing_partial_linetest) is unchanged — those are still stopped-at, not skipped, since they may complete on the next sync.Test plan
fold_state_skips_unparseable_line_and_continueswhich seeds a log withgood / corrupt / goodand asserts both good entries end up in state and the state-cache offset advances to EOF.fold_state_ignores_trailing_partial_linestill passes.things3 today/inbox/projectsrender correctly.