Skip to content

fix(memory): enforce authoritative revision integrity - #1303

Closed
knqiufan wants to merge 2 commits into
oceanbase:masterfrom
knqiufan:fix/issue-1297-memory-integrity
Closed

knqiufan wants to merge 2 commits into
oceanbase:masterfrom
knqiufan:fix/issue-1297-memory-integrity

Conversation

@knqiufan

@knqiufan knqiufan commented Aug 21, 2026 •

Copy link
Copy Markdown
Collaborator

Which issue or RFC does this PR close?

Closes #1297. Retains the Memory compaction contract in RFC #1718.

Rationale for this change

MemoryUnitOfWork.commit() is the final persistence trust boundary. A malformed SPI writer could persist illegal predecessor links or body/hash mismatches, and search could return text that authoritative entry reads rejected. The boundary must validate complete revisions while accepting supported service transitions, including audited inactive compaction.

What changes are included in this PR?

  • Validate the stored base, reachable entry history and complete base-to-revision transition inside the commit transaction.
  • Require manifest entries, audit changes, new entry versions, direct predecessors and active projections to agree exactly. Recompute content hashes and validate canonical searchable text and embedding bindings before writes.
  • Enforce scope-global entry_version_id identity with a database unique index, startup migration and legacy duplicate detection for SQLite/OceanBase. Translate insert races into MemoryBackendConfigurationError with full rollback.
  • Reject semantic revisions of inactive entries while retaining organize(normalize) when canonical content bytes are unchanged.
  • Accept compact only when it removes an existing inactive pointer, names its exact predecessor, has no new target version and records every removal. Preserve historical revisions/bodies, current tag-conflict locking and active projections; explicitly reject unknown operations.
  • Validate dense embeddings by dimension, finite values and unit-norm tolerance without an exact second floating-point normalization comparison.
  • Revalidate entries, expand, projection reads and every FTS/vector candidate against authoritative revisions, manifests, entry rows and current projection pointers.
  • Audit an actual two-writer OceanBase identity collision after disposing the writer engine. A fresh engine/connection verifies that pc_artifacts, pc_artifact_heads, pc_memory_entry_versions and pc_memory_entry_heads contain exactly the winner's records and no loser residue.
  • Add a dedicated disposable OceanBase CI service. Require the collision rollback, hydrated-vector reuse and inactive-compaction regressions to execute; fail the job if any is skipped.
  • Add malformed compact SPI regressions for active deletion, wrong predecessor, retained compacted pointer and unmentioned deletion, with unchanged head/entries and no leaked revision.
  • Prepare bytecode for the isolated Desktop CLI qualification wheel before native probes begin; retain native executable/version/digest verification, command deadlines and process containment.

Are there any user-facing changes?

Malformed public Memory SPI commits fail with MemoryBackendConfigurationError rather than becoming durable state. Existing SQLite/OceanBase schemas receive the scope-global entry-version unique index during startup; legacy duplicates require repair before startup succeeds.

Valid service workflows, inactive compaction, historical readback and documented no-vector degradation remain supported. No HTTP API changes or automatic physical history deletion are added.

How was this change tested?

Validated head: 288b86b, based on master 77cf867.

  • Desktop Windows CI passed at this head, including native CLI diagnostics, fixture-qualified installer creation and installed WebView2 acceptance.

  • Real OceanBase integrity CI passed: 3 passed, zero skips, covering the two-writer identity race/rollback, hydrated dense-embedding reuse and inactive compaction.

  • Windows CPython 3.12 Memory service, persistence, integrity, backend profiles, capacity, concurrency, vector and usage-recorder regressions — 151 passed, 20 skipped. HTTP observability regressions — 18 passed. The Memory/runtime source and regression-test trees are unchanged by the Desktop fixture preparation.

  • Desktop qualification regressions — 5 passed.

  • Genuine Rust 1.95.0 MSVC native CLI probe against the isolated built wheel passed for fixed version, service-status and integrations commands. The original fixture reproduced the native service projection Timeout. The report records the byte-compiled fixture; native command deadlines, executable pinning, isolated user directories and system-only PATH remain enforced.

  • Local cold-fixture measurements: the first version call took 24.533 seconds; wheel bytecode preparation took 4.323 seconds and the prepared version call took 8.173 seconds. These measurements characterize the disposable fixture on this host, not installed-release startup performance.

  • Whole-project Linux-target types, changed-file Windows/Linux types, Ruff, formatting, all non-type pre-commit hooks, lock and diff checks passed. The type hook was checked separately with the platform targets above.

    uv run --locked python -X utf8 -m pytest -q tests/builtin/artifacts/memory tests/builtin/persistence/test_memory.py tests/builtin/persistence/test_memory_integrity.py tests/builtin/persistence/test_sqlite_profile.py tests/builtin/persistence/test_oceanbase_profile.py tests/e2e/test_memory_search_concurrency.py tests/e2e/test_memory_vector_round_trip.py tests/e2e/test_memory_capacity.py tests/builtin/runtime/test_model_usage_recorder.py --tb=short
    uv run --locked python -X utf8 -m pytest -q tests/e2e/test_observability.py --tb=short
    uv run --no-sync python -X utf8 -m pytest -q tests/test_desktop_fixture_qualification.py --tb=short
    cargo build --locked --manifest-path desktop/src-tauri/Cargo.toml --example diagnostic_cli_probe
    uv build --wheel --out-dir desktop/.artifacts/server-wheel
    uv run --no-sync python -X utf8 desktop/tests/real_cli.py
    uv run --locked ty check --python-platform linux

OceanBase tests are opt-in locally because this host has no dedicated database URL. The dedicated disposable OceanBase CI job requires collision rollback, hydrated-vector reuse and inactive compaction to execute with zero skips. The Desktop native test validates CLI process and response handling; it does not qualify installed GUI behavior, accessibility or release startup budgets.

AI usage statement

OpenAI Codex assisted with implementation, review feedback analysis, regression design, validation and PR preparation. Open Code Review was used in delegation mode for deterministic file selection and rule resolution; Codex performed the review.

except (TypeError, ValueError) as error:
raise _InvalidMemoryCommitError("vector") from error
if canonical != projection.embedding or expected_hash != projection.embedding_content_hash:
raise _InvalidMemoryCommitError("vector")

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.

[P1] 不要用精确浮点相等判断单位向量已经规范化

MemoryService._embed_texts() 已先执行一次 canonical_embedding();这里再次归一化后再做 tuple 精确比较。单位归一化不是位级幂等的:公开 MemoryService.remember() 探针让模型返回 (0.2407121489724894, -0.9705965492093231),服务层得到 (0.24071214897248938, -0.9705965492093231),这里二次归一化又把首项改成 0.24071214897248935,于是合法的有限非零向量被抛为 _InvalidMemoryCommitError("vector"),整个 Memory 写入失败。现有向量测试只使用 (1, 0, 0) 这类轴向量,无法覆盖该舍入路径。请改为校验维度、有限性和带容差的单位范数(或确保只规范化一次并复用结果),并补一个非轴向量的公开 MemoryService.remember() 回归测试。

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

感谢指正。我使用你给出的非轴向量复现并确认了这个问题:二次归一化后的精确浮点比较确实会误拒绝合法写入。现已修复为校验维度、有限性和带容差的单位范数,同时保留 profile/content hash 校验,并补充了公开 MemoryService.remember() 稠密向量回归测试及非单位向量拒绝测试。

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

done. Please take a moment to review again to see if there are any other issues.

@knqiufan
knqiufan force-pushed the fix/issue-1297-memory-integrity branch 2 times, most recently from 6a633d2 to 0fb92d1 Compare September 4, 2026 02:59
knqiufan added a commit to knqiufan/powercontext that referenced this pull request Sep 5, 2026
knqiufan added a commit to knqiufan/powercontext that referenced this pull request Sep 20, 2026
@knqiufan
knqiufan force-pushed the fix/issue-1297-memory-integrity branch from 34ca39f to d258721 Compare October 9, 2026 14:44
@AlexStocks AlexStocks added 1.4.0 v1.4.0 release tracking and removed 1.3.0 labels Oct 11, 2026
@hidb4ai hidb4ai removed the 1.4.0 v1.4.0 release tracking label Oct 12, 2026

Copy link
Copy Markdown
Collaborator Author

Closing this PR because #1857 replaced the code it was meant to fix.

The original issue in #1297 was valid, but the old Memory service and backend have now been removed in favor of Atomic Memory. The current migration rejects broken version chains and content/hash mismatches, and Atomic Memory search checks results against authoritative records. These scenarios were verified on SQLite.

The core changes in this PR are therefore no longer needed on master. Any remaining integrity issues should be addressed separately in the current Atomic Memory or migration implementation.

@knqiufan knqiufan closed this Oct 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug: Memory commit accepts malformed revisions and search can return text that entries() rejects

3 participants