Skip to content

fix(migration): skip unenveloped Sources in the residual Memory check - #1935

Merged
hidb4ai merged 2 commits into
oceanbase:masterfrom
happy-v587:codex/fix-1934-unenveloped-source-residual-check
Oct 12, 2026
Merged

hidb4ai merged 2 commits into
oceanbase:masterfrom
happy-v587:codex/fix-1934-unenveloped-source-residual-check

Conversation

@happy-v587

@happy-v587 happy-v587 commented Oct 11, 2026 •

Copy link
Copy Markdown
Contributor

Which issue or RFC does this PR close?

Closes #1934

Rationale for this change

atomic-memory-migrate --action apply aborted with KeyError: 'value' before it could remove any collection, so an upgraded Server could not start at all. Three places in the offline reference conversion read a stored Source payload's value field without checking that the row actually uses the stored envelope:

  • residual_issues() narrows candidate content Sources with a free-text match and then read payload["value"]. Sources written before the payload envelope existed keep the source value at the top level, so a captured prompt merely containing the substring memory crashed the final acceptance check.
  • _work_sources() and _dream_inputs_change() read the same field, so a legacy structured Work Source crashed the conversion itself, or made an unfinished Dream run look unaffected.

The runtime read path already tolerates both layouts through _decode_legacy_payload(), so the offline checks are the only places that assumed the envelope unconditionally.

What changes are included in this PR?

  • _stored_work_payload() reads one stored Source payload in either layout and returns its typed Work payload, the value and the stored root. _work_sources(), _dream_inputs_change() and residual_issues() all use it.
  • A legacy row is rewritten in the layout it was written with; the conversion never wraps a pre-envelope row in an envelope.
  • A legacy structured Work Source is no longer skipped. Its citations are converted, and a whole-collection relationship is archived before the collection is removed, exactly as for an enveloped row. residual_issues() now judges those rows instead of passing them silently, so ready: true cannot be reported while a public Source still cites a removed collection.
  • Regression tests:
    • test_unenveloped_work_source_citations_convert_and_keep_their_layout — a pre-envelope task-outcome holding one exact entry citation and one whole-collection reference. Both are converted, the collection relationship is archived, the row keeps its own layout and stays readable, and verify returns ready: true. Fails on the previous commit with migration-evidence/content/legacy-layout-outcome: 'value'.
    • test_unenveloped_source_mentioning_memory_migrates_unchanged — a pre-envelope prompt capture whose free text mentions memory stays byte-identical and does not fail the migration.

No public API, persisted format or documentation changes.

Are there any user-facing changes?

No user-facing behavior change. A deployment whose database contains pre-envelope Sources can now complete the Atomic Memory migration instead of failing with a KeyError, and a legacy structured Work Source is converted rather than skipped.

How was this change tested?

  • pytest tests/e2e/test_atomic_memory_migration_evidence.py -k unenveloped — both tests fail before the fix ('value') and pass after it.
  • pytest tests/e2e/test_atomic_memory_migration_evidence.py tests/e2e/test_atomic_memory_grant_migration.py tests/e2e/test_atomic_memory_migration_vector.py tests/builtin/persistence/test_atomic_memory_startup_legacy_schema.py tests/test_atomic_memory_migration_cli.py — 71 passed, 84 skipped.
  • make check — prek hooks and ty check pass.
  • Manual validation on a real deployment: a 63.7 MB SQLite database written by an earlier release, with 12 containers, 116 collection revisions and 101 entries. Before the fix, apply failed at the pre-removal check; after it, apply reported ready: true with removed_collections: 12, verify reported ready: true with verified_entries: 101, and powercontext server run --env-file .env completed startup. That database was also scanned directly: no parsed Source content still cites a removed collection, and no legacy memory artifact or lineage row remains.

AI usage statement

Built with OpenCode (opencode/space-bunny-free). The agent reproduced the failure on the user's deployment, located the unconditional envelope reads, wrote the fix and the regression tests, ran the validation commands above, and drafted the issue and this PR description for review. Reviewer feedback on the first commit was verified independently before being acted on: the incomplete-fix finding was reproduced and fixed, and one conclusion this PR originally stated about the conversion query was wrong and has been corrected. The root-cause analysis, the decision to fix rather than work around, and the final wording were reviewed by the user.

The Atomic Memory residual check narrows content Sources with a free-text
match on the substring "memory" and then read the stored envelope
unconditionally. Sources captured before the payload envelope existed keep
the source value itself, so any prompt mentioning memory aborted apply and
verify with KeyError: 'value' before collections could be removed.

Judge the parsed payload instead: without a Work payload there is no
legacy citation to report, the same outcome the existing model lookup
gives for unrelated enveloped Sources.
@AlexStocks AlexStocks added the 1.4.0 v1.4.0 release tracking label Oct 11, 2026
Comment thread src/powercontext/builtin/persistence/migrations/atomic_memory_references.py Outdated
Comment thread src/powercontext/builtin/persistence/migrations/atomic_memory_references.py Outdated
The residual fix skipped every payload without the stored envelope, which
silently accepted a legacy row that still cited a removed collection, and
the conversion path and the Dream input check still read the envelope
unconditionally, so the same row aborted apply with KeyError: 'value'.

Read one layout in all three places through a shared helper, and rewrite a
legacy row in the layout it was written with instead of wrapping it.
@happy-v587

Copy link
Copy Markdown
Contributor Author

@AlexStocks 两条意见已修复,commit 57a12019,两个 discussion 已 resolve。

改动摘要:

验证:make check 通过;pytest tests/e2e/test_atomic_memory_migration_evidence.py tests/e2e/test_atomic_memory_grant_migration.py tests/e2e/test_atomic_memory_migration_vector.py tests/builtin/persistence/test_atomic_memory_startup_legacy_schema.py tests/test_atomic_memory_migration_cli.py → 71 passed, 84 skipped。

我没有在 oceanbase/powercontext 上的 review 请求权限(账号只有 pull),无法用 API 重新指派审查,麻烦你手动再看一遍。CI 正在跑。

@hidb4ai hidb4ai left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@hidb4ai
hidb4ai merged commit 4a2441f into oceanbase:master Oct 12, 2026
35 of 36 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

1.4.0 v1.4.0 release tracking

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug(memory-migration): residual Source check crashes on payloads stored before the source envelope

3 participants