OpenClaw backfill reads message fields one level too high, projecting 0 rows - #552
Conversation
A real OpenClaw v3 `type: "message"` record states only `id`, `parentId`, `timestamp`, and `type` on the record line: `role`, `content`, `model`, `provider`, `api`, `stopReason`, and `usage` are all nested under `message`. The LLP 0158 reader read them off the record line, so every field came back absent, every record resolved to `provider: unknown`, the backfill allowlist excluded all of them, and `hyp status` reported `backfill @hypaware/openclaw [done] (0 rows)` for a session it had failed to read. The settlement enricher was broken the same way one seam later (`record.role`/`record.content`), so a real session settled nothing either. The reader now owns the envelope address: fields are read from the nested `message` object, falling back to the record line for a record that nests none, and `role`/`content` are normalized fields rather than something each consumer picks out of the raw record. Both consumers read them off the normalized message, so neither can drift a level again. Fixtures across the three OpenClaw suites now write the real two-level shape through one helper each; the old flat fixtures asserted an envelope OpenClaw never writes, which is why the suite stayed green through the bug. LLP 0158 records the verified record shape, the envelope read rule, and the path-faithful-fixture consequence. Co-Authored-By: Claude <noreply@anthropic.com>
… is the line's Follow-up to the #543 envelope fix, from review of PR #552. - `messageField` fell back on key *absence*, not value *usability*, so a present-but-unusable nested value permanently masked a good record-line one. A nested `provider: " "` beside a line-level `provider: "anthropic"` resolved the record to `unknown` and the allowlist excluded it fail-closed; a nested `timestamp` that did not parse dropped `message_created_at`, which re-dates the row to session start, defeats the `--since` window (a timestamp-less item is kept unconditionally) and puts the settlement ordinal match outside every window so the turn never dedupes. Rule 3's present-value test now runs at both levels before the fallback decides. - `id` is now read line-first, envelope-fallback. LLP 0158 verified message identity on the record line; envelope-first meant a future OpenClaw that copied the provider's own id into the nested message would silently repoint every `message_id` and `part_id`, so committed rows would stop deduping against new ones and the history would double with nothing raised. - The OPENCLAW_HOME relocation fixture still wrote the invented flat shape, bypassing `messageLine`, so it passed with the envelope read reverted. It now goes through the helper, and the shape pin carries `idempotencyKey` so it matches the live key list the same file documents. - Say which LEVEL `record` is: `parentId` is on the line, `idempotencyKey` and `toolCallId` are at `record.message`. The old wording invited the very read #543 was. - LLP 0158 gains rules 6 and 7 for the two behaviors above, and the stale "no live OpenClaw install was reachable" note on `usageAttributes` is reconciled with the spelling this work verified. Co-Authored-By: Claude <noreply@anthropic.com>
Neutral review round: PR #552 @
|
| record | result |
|---|---|
line provider:"anthropic", nested provider:" " |
provider: undefined |
line timestamp:"2026-07-30T00:00:00.000Z", nested timestamp:"not-a-date" |
timestampMs: undefined |
line usage:{input:10,output:20}, nested usage:null |
usage: undefined |
That value is absent (rule 3) and load-bearing at the same time. Downstream, each of those is a silent drop of the same family as #543:
- lost
providerresolves the record tounknown, and the allowlist excludes it fail-closed - lost
timestampMsomitsmessage_created_at(backfill.js:518-520), so the materializer substitutesconversation_started_atand the row lands in the wrongdatepartition;filterByWindow(src/core/backfill/scan_util.js:68) keeps timestamp-less items unconditionally, so--sinceimports records the window should exclude; andsettle.js:381setsInfinity, which is outside every window, so the ordinal fallback can never match,part_iddedupe never collapses the pair, and the turn double-counts, which is exactly what R11 exists to prevent
Fixed (session_file.js:363-366): the field's own present-value test now runs at both levels before the fallback decides. content and usage got named present-value tests (statedValue, plainObject) so every field is guarded the same way.
2. Medium: id was read envelope-first, contradicting LLP 0158's own statement of where identity lives
session_file.js:275 (pre-fix) read id through the same envelope-first path as content fields, while the PR's own LLP text says the record line states "what identifies and positions the message, ['id','message','parentId','timestamp','type']". Verified: a record with id at both levels resolved to the nested one.
No upside (the verified shape never nests id) and an unrecoverable downside. The nested envelope is OpenClaw's normalization of a provider response and already carries idempotencyKey; a version that also copied the provider's msg_01... id there would repoint every backfilled message_id (backfill.js:517) and every settled one (settle.js:471), and therefore every part_id. Committed rows would stop deduping against new ones and the history would double, with nothing raised anywhere.
Fixed (session_file.js:287): id is line-first, envelope-fallback, so a record that states identity only in the envelope still resolves one. LLP 0158 gains rule 7 stating the exception and why.
3. Medium: the PR's own fixture-fidelity guarantee was already false
The LLP addition claims "Every test that writes a session file writes the real two-level record shape, through one helper per suite, so no fixture can quietly re-invent a flat envelope." test/plugins/openclaw-backfill.test.js:682 bypassed messageLine() and wrote JSON.stringify({ type: 'message', ...ASSISTANT_RECORD }) by hand. Measured: with the envelope read reverted, that test still passed while 15 others failed, so the sole test of the relocated-OPENCLAW_HOME path was not a regression guard at all.
Separately, the new shape pin at :236 asserted nested keys without idempotencyKey, contradicting the live key list stated 190 lines above it and in LLP 0158 Context. The one test whose job is "this is the shape OpenClaw appends" pinned a shape OpenClaw does not append.
Fixed: the relocation fixture routes through messageLine, and ASSISTANT_RECORD carries idempotencyKey so the pin matches the documented key list. Re-verified by mutation: with openclawMessageEnvelope regressed to return row, the relocation test now fails (it did not before).
4. Medium: record was documented at the wrong level, inviting the #543 read
session_file.js:253-254 and types.d.ts:26 told the next caller that parentId, idempotencyKey and toolCallId are "reachable through record, the untouched record line". Per LLP 0158's verified key list idempotencyKey is nested, and match_key.js:175 states toolCallId/toolName "live on the message". So record.idempotencyKey is undefined, and the doc written to prevent one-level-too-high reads was itself instructing one.
Fixed: both sites now name the level. parentId is on the line; a message-level field this reader does not normalize is at record.message.
5. Low: timestamp precedence flipped versus master, unpinned
Master always used row.timestamp; the PR prefers the nested one, and a real record states both. I could not construct a failure from the precedence itself (the two differ only by append latency, inside every tolerance), but no test pinned it: both fixture helpers write the identical timestamp to both levels, so the precedence is unobservable in the suite, and only the envelope-absent direction was covered.
Fixed by pinning rather than changing: a new reader test states different values at the two levels and asserts envelope-wins for timestamp and line-wins for id in one place. Four more reader tests cover the blank/wrong-typed nested field, the blank-nested-with-no-line-value case, and a message key that is not an object.
Not changed, worth a human's eye
- No hermetic smoke writes an OpenClaw session file, in any shape. There is no
backfill_openclaw_fixtureanalog to the Codex and Claude flows, so tier 2 has never touched a real-shape OpenClaw session file and the only gate that would have caught OpenClaw backfill projects 0 rows from real session files: message fields are read at the top level but OpenClaw nests them under 'message' #543 is the manualdocs/ACCEPTANCE.md:437. Given CLAUDE.md's three-tier model, that gap is the reason this bug shipped green, and it outlives this PR. - A projectable record that yields no message is counted nowhere (
backfill.js:441,if (!message) continue), soscan_completecan under-report againstmessages_projected + records_excluded. Pre-existing, not introduced here. - LLP 0158 Decision still says "The invariant is tested once in core", which contradicts the same document's decision to keep the reader plugin-local. Pre-existing; this PR added a bullet beneath it without touching it.
Gates
npm test 3281 tests / 3280 pass / 0 fail / 1 skipped, npm run typecheck clean, at df7f119. Each fix was positively verified by mutation, not by a green suite: reverting the guard-order fix fails only a blank or wrong-typed nested field reads as absent..., reverting the id direction fails only the envelope wins for timestamp, the record line wins for id, and reverting openclawMessageEnvelope now fails the relocation test it previously passed.
House style checked: no em dash (U+2014) anywhere in the change, no statement-terminating semicolons, no @typedef and no inline import('...') types, and the three added @refs resolve to LLP 0158's ## Decision with correct attachment.
…xception Three follow-ups to the round-1 fixes, all in the same family the round-1 findings were: a new behavior that no test pins, and docs that state a rule the code does not follow. - `statedValue`, the `content` present-value test the guard-order fix introduced, was entirely unpinned: replacing it with the identity function left the whole suite green while a nulled-out nested `content` suppressed a usable record-line value and landed `content: null` on the message. Pinned in both directions (line supplies it; absent when neither level does). - `types.d.ts` still stated the blanket envelope-first rule over `id`, which the same commit made line-first (LLP 0158 rule 7), and over `content`, whose test refuses only `null`. The published declaration is what a package consumer reads, so it now names both exceptions. - LLP 0158 rule 6 claimed a blank or wrong-typed nested value can never suppress the line. True of the string fields, false of `content`: a nested `content: " "` or `content: 42` does suppress it. The rule now says what "reads as absent" is per field. Also extends the non-object-`message` test with the `null` case, the one input where the plain-object guard is the difference between reading the record line and throwing out of the whole file read. Co-Authored-By: Claude <noreply@anthropic.com>
Neutral review round 2: PR #552 @
|
| record | result |
|---|---|
line provider:"anthropic", nested provider:" " |
provider: "anthropic" |
line timestamp:"2026-07-30T00:00:00.000Z", nested timestamp:"nope" |
line's value |
line usage:{input:1}, nested usage:null |
{input:1} |
line content:"from-line", nested content:null |
"from-line" |
nested-only {id:" ", timestamp:"", model:42, stopReason:null} |
all four absent, nothing substituted |
The resurrection direction holds too: on the verified real shape the record line carries only ['id','message','parentId','timestamp','type'], so a user turn whose envelope legitimately omits model/provider/usage has nothing to resurrect them from. I confirmed the per-field fallback does pick up a same-named line field when one exists (a synthetic line-level provider:'openai' reaches a user turn), but that is rule 6 working as documented, it is unchanged from before the round-1 fix, and it is unreachable on the shape OpenClaw writes.
Mutation: reverting messageField to the absence-based version fails exactly one test, a blank or wrong-typed nested field reads as absent... (61/62 pass).
2. The id direction flip is safe downstream. Line-first restores master's behavior exactly (master read nonBlankString(row.id)), so nothing regressed. Both part_id producers consume the same reader field: backfill.js:517 (projected.message_id = message.id, the gateway derives part_id from it) and settle.js:471-476 (settled.message_id = match.id, settled.part_id = match.id + '#' + partIndex). The settlement content match key does not depend on id at all: buildOpenclawSessionIndex keys on sessionMatchKey(rawRole, message.content) and uses id only as the value to upgrade to. match_key.js is byte-identical to master. Mutation: reverting to envelope-first fails exactly one test, the envelope wins for timestamp, the record line wins for id.
3. The fixture fix is real and was load-bearing. With openclawMessageEnvelope regressed to return row, 31 of 62 OpenClaw tests fail including a relocated install is found through OPENCLAW_HOME (#17). I then reverted only the fixture line back to JSON.stringify({ type: 'message', ...ASSISTANT_RECORD }) and re-applied the same regression: test #17 goes green again. That is direct proof the round-1 fixture change converted a test that passed for the wrong reason into a real guard. Dropping idempotencyKey from ASSISTANT_RECORD fails the shape pin, so the pin is honest against LLP 0158's live key list. Every settlement fixture routes through sessionFileLine (writeSessionFile, openclaw-settlement.test.js:171); no raw type:'message' literal bypasses a nesting helper in any OpenClaw suite.
4. Whole-PR sanity re-confirmed at df7f119. PROJECTABLE_PROVIDERS, effectiveProviders and partitionByBackend are byte-identical to master (only line numbers shift); claude-cli still excludes with covered_by: claude_transcript and unknown still fails closed. readOpenclawSessionMessages still has exactly two non-test callers, backfill.js:266 and settle.js:325, and no message.record.<field> read survives anywhere. match_key.js unchanged, so LLP 0159 match-key semantics are untouched. The PR's real diff against its merge-base (57e2ec9) is 8 files; master has since moved to be07015 (#554) but touched nothing under openclaw/, so there is no semantic conflict.
House style clean: no U+2014 anywhere in the diff, no statement-terminating semicolons, no @typedef, no inline import('...') types, and test/core/llp-ref-hygiene.test.js passes on the added @refs.
Findings
1. Low: statedValue, the new content present-value test, was pinned by nothing
session_file.js:378 (at df7f119)
function statedValue(value) {
return value === null ? undefined : value
}This function is new in the round-1 guard-order fix and is the only thing making a nulled-out nested content fall through to the record line. I replaced its body with return value (the pre-fix behavior) and ran the whole repo suite: 3281/3281 green. Nothing observed it. That is precisely the "a test that passes under both the fixed and the buggy reader is not pinning anything" case, and the consequence is live: with the identity version, a nested content: null both suppresses a usable line value and lands content: null on the message, which projectedMessageFromRecord drops (backfill.js:502-506) and which buildOpenclawSessionIndex hashes into a content key no live row can match (settle.js:388).
Fixed by adding a nulled-out nested content is unstated, so the record line still supplies it, which pins both halves (the line supplies it; with neither level stating it the key is absent, not null). Re-verified: the identity mutant now fails that test and only that test, 61/62.
2. Low: types.d.ts still stated the blanket envelope-first rule the same commit had carved two exceptions out of
types.d.ts:20-23 (at df7f119): "each is read off the nested message envelope, falling back to the record line, and is absent when the field is missing, non-string (for the string fields), or blank."
Both clauses are now false for a listed field. id is line-first as of the same commit (session_file.js:287, LLP 0158 rule 7), and content is neither string-tested nor blank-tested. This is the round-1 finding-4 defect in the one file it matters most in: types.d.ts is what npm run build:types publishes, so it is the description a package consumer reads, and it currently points them the way #543 was read.
Fixed: the interface doc now names both exceptions.
3. Low: LLP 0158 rule 6 overclaimed, and the code is the honest one
llp/0158-one-reader-for-openclaw-session-jsonl.decision.md:74-76 (at df7f119): "a nested value that reads as absent (blank, wrong-typed, null) cannot also be the value that suppresses the line."
True of every string field and of usage. False of content, whose test refuses only null. Verified at runtime: a nested content: " " and a nested content: 42 each suppress a line-level content: "from-line".
I deliberately did not "fix" this by tightening statedValue. The narrow test is the right call and is already argued at its own JSDoc: content has no single shape, and refusing a blank string would change message.content from '' to undefined, which changes the settlement content key for a turn whose stored match_key was computed at wire time. The overclaim is in the doc, so the doc moved: rule 6 now says "reads as absent" is each field's own answer, and states content's.
Also folded in (not a separate finding): the a message key that is not an object test (openclaw-session-file.test.js:374) asserted only outcomes that are identical with and without the isPlainObject guard, so it pinned nothing. It now covers message: null, the one input where the guard is the difference between reading the record line and throwing out of the whole file read.
Not fixed, and why
- No hermetic smoke writes an OpenClaw session file in any shape. Reported in round 1, out of scope here by instruction, and it outlives this PR.
- The per-field fallback can read a same-named field off the record line for a turn whose envelope deliberately omits it. Confirmed at runtime, but it is documented rule 6, it predates the round-1 fixes, and it is unreachable on the verified record shape. Not a defect; recorded so the next reader does not re-find it.
timestampremains envelope-first whileidis line-first, though LLP 0158 names both as line-level positioning fields. Round 1 pinned this rather than changing it; I could not construct a failure from the precedence either, both consumers read the same reader so they cannot disagree, and flipping it in a final review round would be a behavior change with no evidence behind it. Left as pinned.
Gates
At b60ca6a: npm test -> # tests 3282 # pass 3281 # fail 0 # skipped 1; npm run typecheck clean. (df7f119 baseline was 3281 / 3280 / 0 / 1.) Each fix positively verified against the committed blob: the new test name and the changed types.d.ts / LLP 0158 sentences are present in b60ca6a and absent in df7f119.
No open findings.
Neutral triage: PR #552 @
|
philcunliffe
added
the
neutral:approved
1 participant
Root cause
A real OpenClaw v3
type: "message"record is two levels deep. The record linestates only
['id', 'message', 'parentId', 'timestamp', 'type']; the messageitself (
role,content, and on an assistant turnapi,model,provider,stopReason,usage) is nested undermessage.parseOpenclawSessionMessage(session_file.js) readmodel,provider,api,stopReason, andusageoff the record line, so all of them came backundefined.effectiveProvidersthen had no stated provider to propagate,every record resolved to
unknown, andPROJECTABLE_PROVIDERSexcluded all ofthem fail-closed. That is why a real
~/.openclawscannedfiles_seen: 7, sessions_projected: 0, records_excluded: 18andhyp statusreportedbackfill @hypaware/openclaw [done] (0 rows): a parse miss and an intendedexclusion are indistinguishable at that seam.
The same defect sat one stage later at both consumers, so fixing
provideralone would only have moved the drop:
backfill.js projectedMessageFromRecordreadrecord.role/record.contentsettle.jsreadmessage.record.role(rawRole) andmessage.record.contentfor the LLP 0159 content match key, so a real session settled nothing either
The fix
One envelope-shape change in the LLP 0158 reader, not a per-field patch and not
a loosened exclusion filter:
openclawMessageEnvelope(row)states the address once: the nestedmessageobject is the message envelope, with the record line as the fallback for a
record that nests none. A field the envelope states is never overridden by a
same-named field on the line.
roleandcontentbecome normalized fields onOpenclawSessionMessage.Their address is the thing that was easy to get wrong, so they belong to the
one reader rather than to each consumer's own reach into
record.recordstays exposed for genuinely caller-specific fields (
parentId,toolCallId).again.
usageneeded no change:usageAttributesalready accepts OpenClaw'sinput/output/cacheRead/cacheWritespelling.Fixtures
All three OpenClaw suites now write the real two-level record shape through one
helper each (
messageLinein the backfill suite,sessionFileLinein thesettlement suite, literal nested records in the reader suite). The old fixtures
asserted a flat envelope OpenClaw never writes, which is exactly why CI stayed
green through this bug. A new test pins the on-disk bytes against the shape
verified on a live install, so a fixture cannot quietly re-invent the flat one.
LLP 0158 is updated in the same commit: the verified record shape, read rule 5
(the envelope address and why it fails quietly), the "a field two callers must
locate identically is the reader's" consequence, and the path-faithful-fixture
consequence.
Test evidence
The added regression test is
test/plugins/openclaw-backfill.test.js->a mixed real-shape session partially projects: anthropic turns land, claude-cli turns stay excluded,the mixed-session case from the issue: an
anthropicturn and aclaude-cliturn in one real-shape file must partially project.
Before (on the fixed fixtures, unfixed source):
After:
Whole-suite reproduction, on the path-faithful fixtures against the unfixed
reader (this is the bug's blast radius, not one test's):
After the fix, all three are green (21/21, 20/20, 16/16), as are the other six
OpenClaw suites.
Repo gates:
No pre-existing failures to disclose: the suite is fully green before and after.
Deliberately not in this PR
The issue's two adjacent decisions are design questions, not this defect, and
neither costs rows today (both parse to nothing rather than to something wrong):
*.trajectory.jsonlfiles should be skipped explicitly by the scannerrather than silently parsed-and-empty
probe-anthropic-*/probe-claude-cli-*attach-probe sessions shouldbe imported or skipped by session-id prefix
Both want their own short decision LLP rather than a silent filter here.
Fixes #543