fix: aggregate pre-migration history items under their migrated identifiers

[chrishenzie]

The symptom

On an account with history going back to 2018, memory.State reports tasks that should not be there. Old items that were long since completed, deleted, or edited into something else show up alongside the current ones, and some current tasks come back empty (no title, default status) even though the app shows them correctly. Task counts come out well above what Things itself displays. Accounts created recently look fine, which is why this has gone unnoticed: it only appears once a history is old enough to span the identifier change described below.

Why it happens

Things Cloud has used two generations of object identifier over the life of the service. Older history items (Task3, Task4, Area2, Tag3, ChecklistItem, ChecklistItem2, Tombstone) key objects by identifier strings, usually an uppercase UUID. Newer items (Task6, Area3, Tag4, ChecklistItem3, Tombstone2) key the same logical objects by a 22-character Base58 identifier.

When Things migrated, it did not write any mapping record into the history, and it did not rewrite the old items. Both generations are still in the log, and a client replaying that log has to know that the two identifier forms can refer to the same object. Things derives the new identifier from the old one by hashing it:

current = Base58(SHA1(legacy_identifier_text)[:16])

Recurring-task instances use a composite identifier of the form <uuid>-YYYYMMDD and hash in two steps: the UUID prefix first, then that raw 16-byte digest followed by the -YYYYMMDD text.

The SDK replayed both generations under their own raw keys, so one object became two entries in state. The old entry kept whatever the object looked like at migration time and was never touched again, which is the stale task that surfaces. The new entry started empty and only received whatever changes happened after the migration, which is why post-migration tasks with no recent edits appear blank. Deletes have the same split: a legacy delete or a Tombstone removes the legacy key and leaves the current one behind.

The derivation is the only link between the two generations. There is no mapping commit to read instead. I checked: the large commits around area deletions look like migration tables but contain only {"ar":[]} payload edits, and the kinds the SDK skips (Settings4/Settings5, Command) carry no cross-references either.

The fix

EncodeLegacyIdentifier implements the derivation, and state/memory runs every pre-migration item through it during replay: the envelope key, the identifier-valued reference fields on those items (task ar, pr, agr, tg, rt, dl; checklist ts; tag pn), and the deleted-object identifier on legacy tombstones. Items of current kinds are untouched, and no arbitrary string is ever rewritten, so an account with no pre-migration history replays exactly as before.

things-cli also needed a change. It caches aggregated state as JSON and resumes incremental sync on top of it, so a cache written by the old logic keeps its wrongly-keyed objects forever: later sync only appends newer history and never revisits them. The cache now carries a version, and any other version (including its absence, i.e. every existing cache) is treated as no cache, which forces one full replay through the corrected logic.

Validation

Unit tests cover the derivation against exact vectors, including the recurrence form and near-misses of it that must hash as ordinary text, plus replay tests for each pre-migration kind.

Beyond that, I replayed a full 239,199-item history through this branch and diffed the resulting state against the main.sqlite that Things.app maintains on the Mac for the same account, comparing task identity, title, type, status, trash state, and area/project/heading relationships, plus area and tag identity and title:

    sqlite: 13167 tasks, 6 areas, 10 tags
    cloud:  13167 tasks, 6 areas, 10 tags
    missing: 0   extra: 0   field mismatches: 0

Before the fix the same history produced 1,458 extra pre-migration tasks. Each one's derived identifier is unique and accounts for a current object: 1,326 map to a task that still exists in sqlite, 132 map to an identifier whose last history event is a delete, and none are left unexplained. That mapping was established from identifiers and timestamps alone, without matching on titles.

The comparison does not yet cover dates, notes, task tags, recurrence, delegates, or checklist items, so treat the zero as validating the identifier handling rather than every field the SDK exposes.

Known gap

The sync package keys the same pre-migration kinds by their raw identifiers and has the same bug. I left it alone deliberately: fixing it changes what gets written to a user's existing database, so it needs a decision about whether those databases are rebuilt or migrated in place, and that does not belong in this change. If you would rather see the shared pieces (the kind predicate and the payload rewriting) lifted out of state/memory into the root package now so sync can reuse them later, say so and I will restructure.

Assisted by Claude Code + Codex

[arthursoares]

This is excellent work — careful reverse-engineering, honestly scoped, and the write-up is a model PR description. Before reviewing I verified independently:

• Clean-room check of the derivation: I re-implemented Base58(SHA1(legacyID)[:16]) and the two-step recurrence variant from scratch using only your prose description (not the code), and both committed test vectors (LXmxn9gakySzcEjKj1DtgD, H8Xu72gj7fooPuYoBMZ5TK) reproduce exactly — the tests genuinely pin the documented algorithm.
• No self-inflicted rehashing: things-cli and example/ write only current kinds, so the remapping never touches identifiers this SDK creates.
• Cache invalidation path: version mismatch → nil cache → full replay from index 0; every existing user cache gets rebuilt once through the corrected logic. Confirmed the caller handles it correctly.
• Build, all tests, and go vet pass; splitLegacyRecurrenceIdentifier's fixed 45-byte length correctly excludes the raw/URN/braced UUID forms uuid.Parse would otherwise accept.

The substance is approvable as-is. Requesting changes only for the mechanics below.

Required

1. Rebase onto develop once #23 lands, and run gofmt. I've retargeted the base to develop (this repo integrates there; main is for tagged releases). Note the interaction we confirmed: this branch and #23 merge with no textual conflict, but the merged types.go is gofmt-dirty (the tag-constant block keeps pre-ItemKindTombstonePlain alignment), which fails the lint gate #23 introduces. types.go on this branch is also not gofmt-clean on its own.

Asks (non-blocking, would love to have)

2. Real test vectors, if you're comfortable. The current vectors are synthetic and (as verified above) pin the documented algorithm — but nothing in-repo can falsify the algorithm against Things itself; that evidence lives entirely in your one-time 239k-item sqlite diff. Two or three anonymized real legacy→current pairs from your history (an ordinary object, a recurrence instance, and one reference-field pair) would make the suite independently falsifiable. Identifiers are random, so the privacy exposure is minimal — but it's your data, your call.
3. Non-empty identifiers in the adjusted tests. The memory_test.go changes assert on EncodeLegacyIdentifier(""). The empty-id collapse is behavior-equivalent to main (everything empty previously collapsed onto the "" key, now onto hash("")), so no regression — but blessing it in tests reads as intended behavior. Realistic ids would document intent better.
4. One-line comment on the Area2 tg field. Legacy area payloads carry tg (tag IDs) which isn't rewritten. Harmless today — updateArea ignores TagIDs entirely — but it's a landmine if area→tag handling is ever added. A comment at the area case in Update would defuse it. (Also noted in #24.)

Questions (genuinely asking, not blocking)

5. Did your validation data give any signal on mixed-generation events — a legacy-kind item carrying an already-Base58 reference (would be double-hashed) or a current-kind item carrying a legacy reference (never normalized)? Your zero-extras result suggests neither occurs, but if you saw explicit evidence either way it'd be worth a sentence in the encodeLegacyIDs comment.
6. The recurrence split accepts calendar-invalid suffixes like -20230230. Presumably Things' own rule is shape-based too — did you see anything confirming that, or is it inferred from the matching?

On your restructuring offer

Keep this PR scoped as-is. The sync/ gap and lifting the shared pieces (kind predicate + payload rewriting) into the root package are now tracked in #24 — the offer is gratefully accepted there if you want to take it on, and includes deciding rebuild-vs-migrate for existing sync databases.

(Review assisted by Claude Code + an adversarial Codex pass; all findings above were verified empirically before posting.)

[arthursoares]

#23 is merged into develop — you're clear to rebase this branch and run gofmt (or just make lint, which now exists at the repo root and runs the same pinned golangci-lint the CI gate uses). As a heads-up from our earlier check: the rebase itself will be conflict-free, but types.go will need reformatting afterward (the tag-constant alignment shifts once ItemKindTombstonePlain widens the column), so the lint gate will catch it if gofmt is skipped.

The non-blocking asks and questions from the review stand as written — happy to merge once the rebase is up.