Skip to content

feat(sources): persist immutable evidence declarations - #1717

Open
jiannnnyyyyy wants to merge 42 commits into
oceanbase:masterfrom
jiannnnyyyyy:rfc/p0-source-evidence-lifecycle
Open

jiannnnyyyyy wants to merge 42 commits into
oceanbase:masterfrom
jiannnnyyyyy:rfc/p0-source-evidence-lifecycle

Conversation

@jiannnnyyyyy

@jiannnnyyyyy jiannnnyyyyy commented Sep 22, 2026 •

Copy link
Copy Markdown
Contributor

Which issue or RFC does this PR close?

Refs #1652 and #1711. Implements the Source evidence declaration prerequisite; Memory quality and lifecycle policy remain follow-up work.

Rationale for this change

Captured Sources need an immutable declaration of evidence authority, verification state, and declaration version. The registered Definition owns that declaration. Persistence, remote ingestion, replay, and historical reads must preserve it without allowing captured content or a worker observation to override it.

What changes are included in this PR?

  • Add MemoryEvidenceDeclaration to Source Definitions, persisted Sources, manifests, and remote observations. Definitions without a declaration use the neutral untrusted/unknown default.
  • Stamp new native captures consistently, including record-backed capture and fresh Artifact lineage Sources. Readback preserves the stored declaration instead of applying the current registry.
  • Exclude declaration metadata from captured input schemas and transport payloads, including nested and recursive Sources, adapter serializers, and aliases. Preserve ordinary captured fields that share metadata names.
  • Preserve neutral Definition identity and observation replay across upgrades. Reject native encodings that lose their authoritative declaration before accepting a write.
  • Add frozen worker contracts and public HTTP registration/replay regressions. Preserve existing Atomic Memory persistence, search, rebuild, scope isolation, and migration coverage.
  • Document Definition identity conflicts in both locales and synchronize generated API contracts.

Are there any user-facing changes?

Existing Source Definition registration and Source Observation contracts gain an optional memory_evidence field. Python Source APIs gain the declaration types and fields. No HTTP routes or MCP tools are added.

No database tables or columns are added. Source declarations are serialized in the existing pc_sources.payload; legacy Memory schemas and their migration behavior remain unchanged.

Atomic Memory continues to own revisions, lineage, forget/restore, merge, retirement, and projection rebuild. This PR adds no per-memory evidence snapshot model, lifecycle projection, ranking policy, or filtering policy.

How was this change tested?

  • Focused Source APIs, persisted readback/retry, public HTTP registration/replay, restored Atomic Memory persistence/search/scope tests, and SQLite migration/rebuild scenarios: 99 passed, 78 skipped. Skipped cases require opt-in real OceanBase or SeekDB acceptance.
  • make check: passed, including the lockfile, pre-commit hooks, full type checks, and integration manifest tests.
  • make contract-test: passed, including Python/JavaScript generation checks and API/Desktop contract tests.
  • node desktop/scripts/generate.mjs --check: passed with Node 24.14.1.
  • Local backend execution used SQLite. OceanBase and SeekDB acceptance remains covered by the dedicated CI jobs; it is not claimed as local execution.

AI usage statement

Codex (GPT-6) assisted with implementation, regression tests, review, documentation, and validation.

@jiannnnyyyyy
jiannnnyyyyy force-pushed the rfc/p0-source-evidence-lifecycle branch 2 times, most recently from 7c1c986 to a2f8238 Compare September 22, 2026 14:11

@PsiACE PsiACE left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Please fix the manifest registration/readback failure. The versioning and write-cost comments are design notes.

Comment thread src/powercontext/sources/observations.py Outdated
Comment thread src/powercontext/sources/observations.py Outdated
# Lifecycle rows are keyed by entry, not by revision, so the artifact's rows
# are replaced wholesale: lifecycle_projections reads them back by revision.
await connection.execute(
delete(MEMORY_ENTRY_LIFECYCLE_PROJECTIONS_TABLE).where(

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Design note (non-blocking): appending one entry to 5,000 deletes 5,000 lifecycle rows and inserts 5,001, even with neutral policy and no Source evidence. Repeated appends produce quadratic cumulative writes. Consider incremental updates, or justify full replacement against the expected scale. This is a performance risk; no incorrect results or latency-budget violation were established.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Write cost — implemented incremental updates. Lifecycle rows now follow the same per-entry maintenance rule as the active-head and vector rows in this revision: _commit diffs each manifest entry's (entry_version_id, state) against the base revision and rewrites only the changed entries, deleting rows only for entries that left the manifest. Appending one entry to an N-entry Memory now writes one lifecycle row instead of N deletes plus N+1 inserts. The read no longer filters on head_revision == memory.revision — that filter was the only reason every row had to be re-stamped; it now selects the artifact's rows and proves completeness against the authoritative manifest by entry identity, exactly as the active-head projection does, and a stale ref is still rejected. head_revision keeps its meaning as the revision that last wrote a row, consistent with pc_memory_entry_heads and pc_memory_vector_entries. rebuild_projections remains a deliberate full replacement as the repair path. Correctness is pinned against full derivation: tests/builtin/persistence/test_memory.py::test_lifecycle_projection_rows_are_rewritten_only_for_changed_entries asserts that unchanged rows keep their writing revision across an append, that forget re-stamps only the forgotten entry and flips its validity, and that the incrementally maintained rows equal rebuild_projections() output. Remaining append-write amplification for the projection set as a whole (storage layout, compaction) still belongs to #1321.

@AlexStocks AlexStocks 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.

Review summary — 2 blockers; the rest of the change is technically sound

Reviewed 599ff4ca against a three-tree harness (baseline 1e90376a, previous revision a2f82381, HEAD 599ff4ca). Verdict: REQUEST_CHANGES, for the two blockers below. Both are the same class of problem: a default value silently changes an existing invariant.

Verified as working — please do not re-litigate these:

  • The four fixes described in the commits are genuine guards, not "passes on the new tree only". Each fails on a2f82381 and passes on 599ff4ca: the legacy-manifest round trip, the bind-variable limit (reproduced at 10,921 entries → too many SQL variables), the null projection value (422 before, accepted after), and lifecycle incrementality (51 rows written before, 1 after).
  • The generated API artefacts do not drift when merged with current master (24 commits ahead): regenerating from the merged openapi/powercontext.yaml reproduces _generated/* byte-for-byte, and the negative control flags a tampered file correctly.
  • Migration and rollback are clean. On an offline copy of a real SQLite database, upgrade creates the two new tables (pc_ 108 → 110) with no ProcessingSchemaNotReadyError and no change to existing rows; rolling back leaves the new tables unused rather than broken.
  • All 22 CI checks are green.

🔴 Blockers

  1. sources/observations.py — the neutral declaration is folded into the manifest fingerprint, so upgrading a deployed worker makes every re-registration conflict (HTTP 409 on every connector run).
  2. builtin/runtime/relational.py — the declaration consistency gate is keyed on __pydantic_fields_set__ and the server never stamps the declaration from the registered manifest: omitting the field bypasses the check, while explicitly sending the same neutral value is rejected.

🟠 / 🟡

  1. persistence/memory.py — the bind-budget fix closed one query but left the two sibling IN queries introduced by this PR unbounded.
  2. artifacts/memory/protocols.py — an entire lifecycle channel (table + write path + read path + protocol method + model field) has zero production consumers, and its docstrings advertise availability to the Context Pack.
  3. sources/definitions.py — a new required protocol attribute is a breaking change for out-of-tree adapters.
  4. persistence/tables.py — FK semantics: RESTRICT is latent today, but the INSERT-time check surfaces as an unmapped IntegrityError → HTTP 500.
  5. docs/en/docs/operate/troubleshoot.md — the upgrade failure modes are not covered by any troubleshooting entry (suggested text included inline).

Not anchorable to a line — but please address before merge

A. Three claims in the description need correcting. The "no breaking changes to existing Memory data" section is accurate for Memory, but the description currently reads as if nothing user-visible changes. Three things do:

  • the manifest fingerprint changes for every definition (see the observations.py comment) — this is the one operators will hit on upgrade;
  • SourceDefinition now has a required attribute, so out-of-tree adapters built directly on the protocol fail registration;
  • the new pc_memory_entry_evidence → pc_sources foreign key adds a constraint on deleting pc_sources rows, which no retention/erasure implementation can bypass later.
    A short "user-facing changes" paragraph covering these three (with the version-bump guidance) would make the description match the code.

B. Rebase and re-generate before the next review. master is 24 commits ahead and has touched the same four generated artefacts (openapi/powercontext.yaml, _generated/models.py, _generated/schema.py, http/__init__.py). The merge is textually clean and semantically self-consistent, but the generated files must be regenerated, not hand-merged, or the --check gate will fail: run make api-generate and then make contract-test after the rebase.

C. Two branches have no test coverage at all, and both are the kind that fails silently:

  • the declaration mismatch rejection (relational.py:1931) — grep -rn "does not match the registered manifest" tests/ returns nothing, so the security boundary this PR introduces is untested;
  • the pc_sources foreign key — no test asserts that a referenced Source cannot be deleted, nor that an observation referencing an unregistered Source is rejected.
    Suggested assertions (1) InvalidSourceObservationError with field == "memory_evidence" when the observation contradicts the manifest; (2) IntegrityError when deleting a referenced pc_sources row, plus the positive path after the evidence rows are removed.

D. Housekeeping in the new bind-budget test. The docstring states the Windows SQLite limit is 250,000; it is 32,766 on this machine (and that is why 10,921 entries reproduces the original failure). Also, the assertion currently holds only because SELECTION_BATCH_SIZE % 3 == 2 (500 → 499 bound values ≤ 500); if the constant is ever set to a multiple of 3 the assertion fails by itself. Assert the invariant (3 * (B // 3) + 1 <= limit) instead of relying on the current modulus.


This review was produced with AI-assisted analysis. Every claim above was reproduced by running the probes against the actual source trees; the scripts are available on request.

"version": version,
"source_schema": source_schema,
"projections": [projection.model_dump(mode="json", by_alias=True) for projection in projections],
**({} if memory_evidence is None else {"memory_evidence": memory_evidence.model_dump(mode="json")}),

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.

🔴 Blocker: the neutral declaration is folded into the manifest fingerprint, so upgrading breaks every existing Source registration.

The fingerprint now depends on whether memory_evidence was supplied rather than on what it means, and manifest_for_definition always supplies it (lines 212/216). Every definition therefore gets a different fingerprint than it had before this PR — even though the value is the neutral default.

Measured with _source_definition_fingerprint on the two definitions in the tree:

definition before (no declaration) after (neutral)
note/1 sha256:ce55ca82b4fd…cedc853 sha256:10c565a260ec…9500114
content/1 sha256:f3b6e9e4… sha256:0faa91ad…

SourceDefinitionManifestRepository.register rejects a payload that differs for the same (name, version) (persistence/source_definitions.py:46-50, wrapped into SourceConflictError → HTTP 409). Remote workers re-register on every run — docs/en/docs/workflows/ingest-text-files-with-opendal.md:47 says "On every run, the worker idempotently registers the text-file-snapshot Definition manifest", and integrations/opendal/.../source.py:94 still uses version = "1". So any deployment that already registered a definition starts failing as soon as either side is upgraded:

SourceConflictError("duplicate Source definition-manifest: ('note', '1')")   # → HTTP 409 source_conflict

The legacy fallback from 6f5d546 does not cover this: it keeps a historical payload readable, but it does not make a newly emitted payload match an old registration.

Suggestion — treat the neutral declaration as absent for fingerprinting, so a neutral declaration keeps the manifest's historical identity and only a non-neutral adapter declaration changes it:

neutral = MemoryEvidenceDeclaration()
**({} if memory_evidence is None or memory_evidence == neutral else
   {"memory_evidence": memory_evidence.model_dump(mode="json")}),

Please also consider whether register should accept a re-registration that differs only by the neutral declaration, and update the "user-facing changes" section of the description — this is the failure operators will hit first.

raise InvalidSourceObservationError("definition", "does not match the registered manifest identity")
if source.definition_fingerprint != manifest.fingerprint:
raise InvalidSourceObservationError("fingerprint", "does not match the registered manifest")
if "memory_evidence" in source.__pydantic_fields_set__ and source.memory_evidence != manifest.memory_evidence:

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.

🔴 Blocker: the consistency gate is keyed on a serialization side-effect, and the server never stamps the declaration from the registered manifest.

This check only fires when the field is present in __pydantic_fields_set__ — that is, it inspects whether the client mentioned the field, not what it said. Combined with the fact that the observation is never overwritten from manifest.memory_evidence before it is persisted, the same intent gets opposite verdicts:

client behaviour manifest declares user_asserted declaration reaching storage
omits memory_evidence accepted neutral (untrusted / unknown)
explicitly sends the neutral default rejected — InvalidSourceObservationError("memory_evidence", "does not match the registered manifest") —

So the declaration is never guaranteed to equal the Definition-owned value. Today the impact is contained because nothing in src/ consumes authority / verification yet, but the invariant this code is meant to enforce — that content, SourceRef, and the model cannot manufacture a stronger declaration than the Definition owns — does not actually hold; it is enforced by convention only. As soon as stage 2 uses authority for ranking or protection, an omitted field silently downgrades attestation.

Suggestion — make the server authoritative rather than advisory, for example:

source = source.model_copy(update={"memory_evidence": manifest.memory_evidence})

(equivalently, drop the field from the transport contract). If explicit client declarations must be preserved, keep the equality check but compare values after defaulting, so omitting the field cannot bypass it.

Also: this branch has no test coverage — grep -rn "does not match the registered manifest" tests/ returns nothing. A test asserting InvalidSourceObservationError with field == "memory_evidence" would lock the boundary in place.

select(MEMORY_ENTRY_VERSIONS_TABLE).where(
MEMORY_ENTRY_VERSIONS_TABLE.c.scope_id == self._scope_id,
MEMORY_ENTRY_VERSIONS_TABLE.c.memory_artifact_id == memory.artifact_id,
MEMORY_ENTRY_VERSIONS_TABLE.c.entry_version_id.in_(version_ids),

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.

🟠 The bind-budget fix is incomplete: the sibling IN query added by this same PR is still unbounded.

_hydrate_source_evidence was correctly batched (line 458, _EVIDENCE_SELECTION_BATCH_SIZE), but this query — new in this PR, in the very path the fix targets — still binds every manifest entry in one statement (3 * N + 1 values). The same pattern appears in the read path at line 236.

The threshold is the one that motivated 599ff4c: with SQLite's SQLITE_LIMIT_VARIABLE_NUMBER = 32766 (measured locally on this machine; the 250,000 figure in the new test docstring is not what this platform reports), a Memory whose manifest exceeds 32,766 entries raises too many SQL variables. Unlike the evidence query, this one is reachable from rebuild_projections for every Memory.

This is an established pattern rather than something invented here (the pre-existing code had it at the old lines 192 and 398), but since the commit explicitly sets out to fix exactly this failure mode, the two new call sites should be batched as well. Consider factoring the batching into one helper so _evidence_values, _lifecycle_for_entries, and lifecycle_projections cannot drift apart again.


...

async def lifecycle_projections(self, memory: ArtifactRef, /) -> tuple[MemoryLifecycleProjection, ...]:

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.

🟠 This method has no production caller, and neither does the rest of the lifecycle channel it belongs to.

Grepping src/ for lifecycle_projections finds only this declaration and the implementation in persistence/memory.py:222 — the facade and the HTTP layer never expose it. The same is true for the rest of the channel added by this PR:

  • MemoryProjection.lifecycle (line 57) is a field that no construction site assigns — all five (artifacts/memory/service.py, persistence/memory.py, sqlite/memory_index.py, oceanbase/memory_index.py) leave it at None;
  • MemoryContextAnnotation.context_annotation is read only from tests/;
  • _lifecycle_projection can only produce current / inactive and always sets validity_reason / successor_entry_id to None, so two of the four declared validity values are unreachable.

Measured cost: the two new tables and their paths account for roughly 270 of the 317 added lines in memory.py, rebuild_projections now derives lifecycle rows for every Memory head (twice, since _authoritative_projections runs for both the baseline and the verification pass), and _hydrate_source_evidence is now invoked on the ordinary read paths — entries() (line 212), projections() (line 288), and expand() (line 434) — to populate a field with no reader.

The docstrings are also stronger than the code: models.py:172/:186 say the annotation is "available to Context Pack assembly" and "available to later Context Pack rendering". Nothing consumes it yet.

Two acceptable resolutions — pick one explicitly: (a) defer the materialized table and derive lifecycle data on demand until a consumer exists (the existing tests already prove the derivation is reproducible), or (b) keep it but state in the description and docstrings that this is infrastructure reserved for a later stage and is not yet consumed. Please also fix the "available to Context Pack" wording so the documentation does not promise a wiring that does not exist.


version: str
projections: tuple[SourceProjection[SourceT], ...]
memory_evidence: MemoryEvidenceDeclaration

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.

🟠 A new required attribute on the SourceDefinition protocol is a breaking change for out-of-tree adapters.

In-tree this is consistent: AdapterSourceDefinition supplies a default (line 73), and the one class-style implementation, integrations/opendal/.../source.py:96, was updated in this PR. But any third-party adapter that implements the protocol directly now fails at registration with InvalidSourceDefinitionError(..., "memory_evidence", "must be a MemoryEvidenceDeclaration") (lines 235-241), because _validate_definition has no compatibility branch — unlike the legacy fallbacks this same PR adds for manifests and observations, which do tolerate a missing declaration.

RFC 1652 already flags the risk ("a Source authority/verification declaration is a new adapter-contract responsibility and must be migrated carefully"), but the description does not.

Suggestion: either treat a missing attribute as the neutral declaration inside _validate_definition, or declare this as a breaking adapter-contract change with a migration note.

ForeignKeyConstraint(
("scope_id", "source_type", "source_id"),
("pc_sources.scope_id", "pc_sources.source_type", "pc_sources.source_id"),
ondelete="RESTRICT",

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.

🟡 Two foreign-key consequences worth pinning down before merge.

1. RESTRICT here is latent today, not an active breakage — but it is a hard constraint on future maintenance paths. No code path deletes pc_sources rows: SourceRepository only inserts and updates, and forget() sets a state flag rather than removing rows. The same constraint already exists on pc_artifact_lineage_sources (tables.py:256-260), so nothing regresses. It does mean any future retention/erasure implementation must delete the child rows first, which should be stated as a known limitation rather than left implicit.

2. The immediately observable change is on the INSERT side. This FK is validated when evidence rows are written, so an entry whose source_evidence points at a Source that was never registered in pc_sources fails with IntegrityError. That exception is not mapped by _map_domain_error — grep -n IntegrityError src/powercontext/server/app.py returns nothing, so it falls through to the generic handler at app.py:5480 and the client receives HTTP 500 internal_error rather than a 4xx.

Existing tests always register the Source before remembering entries, so the happy path is covered, but a real ingestion pipeline that skips registration would surface as an opaque 500 with no actionable code. Suggest mapping the FK violation to 409/422 and adding a test — nothing in tests/ currently asserts anything about this constraint.

Comment thread docs/en/docs/operate/troubleshoot.md Outdated
```bash
obloader <connection-options> -D <new-database> --csv \
--table 'pc_artifact_candidate_heads,pc_topic_memory_active_topics,pc_topic_memory_active_chunks,pc_memory_entry_heads,pc_artifact_tags,pc_recurrence_match,pc_recurrence_observation' \
--table 'pc_artifact_candidate_heads,pc_topic_memory_active_topics,pc_topic_memory_active_chunks,pc_memory_entry_evidence,pc_memory_entry_heads,pc_memory_entry_lifecycle_projections,pc_artifact_tags,pc_recurrence_match,pc_recurrence_observation' \

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.

🟠 The migration list is correct, but the failure modes this PR introduces are not covered by any troubleshooting entry.

Placing both tables in layer 3 is right: pc_memory_entry_evidence references layers 1 and 2 only, and pc_memory_entry_lifecycle_projections references layers 1 and 2 only, so neither depends on a table in its own layer.

What is missing is an entry for what operators will actually hit after upgrading, i.e. the HTTP 409 / 422 pair described in the sources/observations.py comment. Suggested addition (please mirror it in docs/zh/docs/operate/troubleshoot.md:279):

A remote Source Definition stops registering

A Source Definition manifest is immutable and keyed by name and version; its fingerprint covers exactly the fields the manifest was emitted with. After upgrading a worker or the Server across a change to those fields, an existing registration no longer matches:

  • Registering the Definition returns HTTP 409 source_conflict.
  • Submitting an observation whose definition_fingerprint predates the change returns HTTP 422 invalid_source_ingestion.

Register the changed Definition under a new version, then restart the worker so it submits observations against the new manifest. Do not edit a stored manifest in place, and do not reuse a version with different content.

"validity IN ('current', 'inactive', 'superseded', 'unresolved_conflict')",
name="ck_pc_memory_lifecycle_projection_validity",
),
CheckConstraint("quality_policy = 'neutral'", name="ck_pc_memory_lifecycle_projection_quality_neutral"),

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.

🟡 This constraint pins quality_policy to a single legal value, so the stage-2 opt-in cannot ship without a DDL migration.

Combined with Literal["neutral"] on the model side (artifacts/memory/models.py:168/:194), "neutral by default" is currently "neutral forever": any non-neutral quality policy requires a schema change plus a model change. The same applies to the validity, authority, and verification CHECK constraints above, which RFC 1652 explicitly says may need finer labels later.

The companion version fields do not help either — declaration_version and rule_version are stored and returned but never consulted (grep finds no branch on either), so they cannot be used to distinguish old rows from new ones after a rule change.

This is consistent with the RFC's "ranking remains disabled" for stage 1, so it is not a blocker. But if stage 2 is intended to ship without a migration window, consider widening these constraints to a controlled set of values (or add an allow-list of supported versions validated on read) now, while the tables are new and empty.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@AlexStocks All items addressed. Both blockers and A–D are in place; two were verified against your suggestions and differ, with reasons noted.

🔴 Blockers

1. observations.py — neutral declaration folded into the fingerprint

Fixed. Added _declared_evidence(), which treats "absent" and "the neutral default" as the same thing, shared by validate_manifest, the serializer, and the fingerprint computation. A neutral declaration now yields the identical fingerprint a Definition had before the field existed, so re-registration after an upgrade no longer conflicts; a declaration that asserts authority/verification still changes it. A regression test covers both repeat registration and an explicit neutral declaration.

2. relational.py — gate keyed on __pydantic_fields_set__, server never stamped

Fixed. _validate_source_observation now returns the stamped observation and the call site persists it: omitting the field is stamped from the registered manifest (it no longer lands as the neutral value), while an explicit contradictory declaration is rejected (issue == "memory_evidence"). __pydantic_fields_set__ is kept only to detect that the client supplied the field — its actual purpose; the bypass path is closed by stamping. The test asserts the persisted value and fails on the previous revision.

🟠 / 🟡

3. memory.py — bind budget

Fixed. There were five call sites, not two (the rebase added entries and tagged_entry_ids). Extracted a shared _batched() and _hydrate_entry_versions(), with the missing-version completeness check in one place.

Verification changed the reserve: the tag predicate binds about 2 × labels, and TagFilter admits 16. Chunking at 500 measured 530 bound values at 16 labels — still over budget. With _ENTRY_BIND_RESERVE = 40 it now measures 498 at 16 labels and 499 for the evidence query.

4. protocols.py — lifecycle channel has no consumer

Took option (b): kept the structure (the derivation is deterministic and already proven reproducible) and stated in the docstrings that it is infrastructure reserved for stage 2, currently unconsumed, and not wired into Context Pack assembly. The "available to Context Pack" wording is corrected.

5. definitions.py — new required protocol attribute

As suggested: a missing attribute resolves to the neutral declaration via definition_memory_evidence(), and only a wrong type raises. This mirrors the tolerance this PR already added for manifests and observations.

6. tables.py — FK semantics

Agreed that RESTRICT is latent rather than active.

  • The INSERT-side mapping is unreachable, so I did not add it. Evidence rows can only be written for a registered Source: plan_remember resolves every reference through _canonical_operation_evidence (service.py:1223), and the resolver (relational.py:1482) goes through require_for_generation, which returns only stored rows and raises SourceNotFoundError (404) otherwise. pc_sources also has no delete statement anywhere in the repository — only insert and update — so there is no "deleted between resolution and write" race either. Mapping an unreachable branch would be untestable dead code.
  • Boundary: this holds for Python code paths only; raw SQL or an external tool writing evidence rows would still surface an unmapped IntegrityError.
  • The DELETE side is now pinned by a test: test_registered_source_cannot_be_deleted_while_memory_evidence_cites_it. That is the constraint you identified; forget() only sets a state flag, so nothing is affected today.

7. troubleshoot.md — upgrade failure modes

Added your suggested entry, mirrored into the Chinese page, plus a note that a neutral declaration does not contribute to the fingerprint, so upgrading across the introduction of that field changes no Definition's identity.

The two unreachable validity values and validity_reason/successor_entry_id are covered by the item 4 docstring note.

8. tables.py — CHECK constraint pins the value

Documented at the table definition: the constraints express the stage-1 vocabulary, so quality_policy admits only neutral; stage 2 has to widen them in one DDL migration alongside the model literals, and the column widths make that a constraint swap rather than a rebuild. I did not widen them early, which would let the database accept values the model cannot produce.

Additional items

  • A. Description updated with the three user-visible changes: a non-neutral declaration changing the fingerprint (with version-bump guidance), the SourceDefinition declaration attribute, and the pc_memory_entry_evidence → pc_sources foreign key's effect on deletion.
  • B. Rebased onto current upstream/master and regenerated; both --check gates exit 0 with no drift, and the contract tests pass (49).
  • C. Added test_definition_without_a_declaration_attribute_still_registers plus tests covering the bound budget of every read path.
  • D. Dropped the 250,000 figure, which is not what this platform reports. The assertion now checks the invariant limit = max(3*E+ER, B+R) instead of relying on the current constants. Widening the monitor from the evidence table to every SELECT is what surfaced the 16-label overflow in item 3. test_tag_filter_label_ceiling_matches_the_entry_bind_reserve ties the label ceiling to the reserve so the pair cannot drift apart.

Validation: 766 passing in the affected modules; ruff check clean apart from one pre-existing C901 in this repository; ruff format --check clean; ty check reports nothing in the changed files.

The fingerprint covers exactly the fields a manifest was emitted with, so an absent memory_evidence resolves to the neutral declaration while keeping its historical content-addressed identity. Serialization re-introduced the model default, so a legacy manifest validated on the way in, was stored with the field, and then failed its own fingerprint check on readback (InvalidStoredPayloadError). Serialize the field set the manifest actually validated instead, keeping registration, readback, repeat registration and observation submission consistent.
Lifecycle rows now follow the same per-entry rule as the active-head and vector rows: a commit diffs each manifest entry's identity and state against its base revision and rewrites only what changed, so appending one entry to an N-entry Memory writes one row instead of N deletes plus N+1 inserts. The read no longer filters rows by head_revision, which was the only reason every row had to be re-stamped; it selects the artifact's rows and proves completeness against the authoritative manifest by entry identity. head_revision keeps its meaning as the revision that last wrote a row.
exclude_none was used to keep an absent evidence declaration out of the runtime model, but it also dropped required-but-nullable values: a projection whose value is JSON null became projections.0.value Field required, so a valid observation was rejected with 422. Drop only the absent declaration and keep every other value as sent.
The evidence lookup bound three identity columns per entry plus the scope, so a manifest beyond roughly a third of SQLite's variable ceiling could no longer be read or appended to, even when no entry carried sources. Chunk the lookup at the shared SELECTION_BATCH_SIZE bind budget, deduplicating identities so batches stay disjoint, and pin the budget with a regression test that measures the bound values of each statement.
…plexity budget

Comparing the worker's manifest against the registration response compared two
encodings of the same Definition: the transport model carries the evidence
declaration as an optional value, so the response states it explicitly while the
worker's manifest omits a neutral declaration. Normalize both sides before
comparing, so a neutral declaration does not read as a mismatch. The emitted
manifest also carries a neutral declaration by omission, matching the form its
fingerprint covers and the form restored from storage.

Extracting the lifecycle write and the evidence insert keeps _commit within its
complexity budget; the lifecycle block this PR introduced had pushed it over.
@jiannnnyyyyy
jiannnnyyyyy force-pushed the rfc/p0-source-evidence-lifecycle branch from 3436b2d to cc290a5 Compare September 30, 2026 03:54
… the requested revision

Adding the evidence declaration to the Source base class also changed every
Source class's schema, and the manifest fingerprint covers that schema, so a
Definition registered before the field existed no longer matched the manifest a
worker emits. The emitted input contract now excludes the declaration and the
definitions only it referenced.

A lifecycle read also returned the state of whatever revision last wrote a row:
deactivating and reactivating an entry keeps its version identity, so an
identity-only check accepted a later row under a historical reference. Lifecycle
state is now derived from the requested revision's manifest.

Version parts are validated at the length the transport and identity columns
allow, so a declaration version cannot register locally and then fail conversion.
Indexing the schema keeps the assertion readable to the type checker, which
cannot narrow the union a JsonValue mapping returns from a defaulted lookup.
…storical reads

The declaration is Definition-owned metadata, so it must not travel as captured
content: a Source class that forbids extra properties rejected its own generated
observation, and an unchanged Source gained a payload field, so replaying an
observation captured before the upgrade conflicted.

Stripping it from the input contract now covers the whole schema rather than the
root properties, because a recursive or nested Source has its inherited fields
under a local definition, not at the root.

A lifecycle read cannot reuse a persisted row's columns either: revising an entry
to cite a Source keeps the row while changing what it describes, so a historical
reference reported the latest revision's evidence count, or failed outright. The
read derives every field from the requested revision.
…ts name

Removing every property called memory_evidence also stripped a field an ordinary
captured model declares for itself, which changed that model's contract: the
generated payload satisfied the schema no longer, and a Definition registered
before the change no longer matched.

The removal now matches the schema a Source inherits, so declaration metadata is
recognized by ownership and a captured field of the same name keeps its place.

@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.

Reviewed fd2efa17. Two reproducible P2 Source contract failures remain; please address them before merging.

The configured real OceanBase, generation model, and embedding service passed a small synthetic-data extraction/retrieval journey and historical evidence checks through projection rebuild and runtime reconnection. The inline failures were reproduced through the HTTP routes using ASGITransport backed by OceanBase; the upgrade/replay counterexamples were additionally checked against base and head on SQLite. The isolated test database was removed afterward.

Comment on lines +320 to +322
payload = _json_object({
key: value for key, value in source.model_dump(mode="json").items() if key not in DECLARATION_OWNED_FIELDS
})

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.

[P2] Strip inherited declaration metadata from nested Source payloads

For BundleSource.child: ChildSource with extra="forbid", the generated schema excludes child.memory_evidence, but this root-only filter retains it in the generated payload. On real OceanBase through the HTTP routes, registration returns 200 and submission returns 422 invalid_source_ingestion. Removing only that nested payload field restores 202 and idempotent replay at journal position 1.

Separately, SQLite base-to-head testing with permissive nested models returns 409 source_conflict when replaying unchanged input. Please apply consistent recursive Source-ownership rules to schema generation and payload serialization, while preserving ordinary captured fields.

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.

Rechecked at ba977912ee6da91454402f3edd3e3c8aab9fd211: this P2 is still reproducible. On real OceanBase, the strict nested Source registers with 200 but its generated observation returns 422 invalid_source_ingestion; removing only payload.child.memory_evidence gives 202; replaying that accepted payload remains at journal position 1. Current-base-to-head SQLite testing also still returns 409 for an unchanged permissive nested observation. The filter at current lines 338–340 still removes only root fields.

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.

Rechecked at b7f8c2e0bf8743d4d4226f25d3075a11c973c1ac: the original direct/list nested Source examples now pass. A P2 remains for other previously working payload shapes.

RootModel[ChildSource], RootModel[list[ChildSource]], dataclass wrappers, and set/frozenset collections retain nested memory_evidence. The schema omits that field, so registration succeeds with 200 but the generated observation fails with 422. The RootModel exclusion incorrectly adds a root level; dataclasses and sets fall through the traversal. Current-base-to-head SQLite tests reproduce all six shapes. On real OceanBase, RootModel, dataclass and set cases likewise reject the generated payload; deleting only the leaked field replays the base-generated input with 202 and the original receipt.

Please make exclusion follow the serialization shape for these supported values. Current code: payload traversal.

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.

Re-tested at 23d8dcd4. Actual RootModel[ChildSource], RootModel[list[ChildSource]], and both standard/Pydantic dataclass wrappers now replay successfully. The set/frozenset case remains P2: a Source containing set[ChildSource] or frozenset[ChildSource] with a frozen, extra="forbid" child still serializes each child's memory_evidence, although the manifest excludes it. The indexed exclusions at observations.py:261-262 do not remove it during serialization.

On both SQLite and real OceanBase, the unchanged manifest registers with HTTP 200 and the exact-base observation is accepted with 202, but the head-generated observation gets 422 invalid_source_ingestion. Removing only the leaked child metadata restores 202 and the original receipt. Please make exclusion effective for these accepted container types, or reject unsupported definitions before returning a usable manifest.

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.

Re-tested at df3157e1 on SQLite and real OceanBase. The set/frozenset transport output now excludes the declaration when projecting an already-constructed adapter result, but normal registry resolution hits the separate new validation failure described in the current review.

The transport issue remains P2 in other accepted shapes. Switching to _strip_payload_metadata at observations.py:363 regresses both standard and Pydantic dataclass wrappers: the walk has no dataclass branch. It also misses dict[int, ChildSource] because JSON converts its keys to strings, and a nested model using serialize_by_alias=True because encoded field names differ from model field names. Each strict ChildSource retains memory_evidence, although its unchanged manifest excludes it. Registration returns 200; the generated observation returns 422 invalid_source_ingestion. Removing only the leaked metadata restores 202 and the original receipt in all four cases.

Please preserve Source-field ownership across these serialization boundaries so observations generated for accepted definitions satisfy their own manifests.

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.

Re-tested at e39cca09 on SQLite and real OceanBase. The previous dataclass, integer-key dictionary, and nested BaseModel alias cases now pass the complete registry.resolve → transport → HTTP submission path. Native set/frozenset resolution, persistence, readback, and replay also pass.

One P2 remains in the same traversal: it does not consistently follow the serialized field/key mapping (observations.py:276-279,284-288). Three accepted shapes still retain nested memory_evidence:

  • dict[bool, ChildSource]: {True: child} becomes JSON key "true", while the traversal looks for "True".
  • dict[KeyEnum, ChildSource]: a plain Enum member with value "primary" becomes "primary", while str(key) produces "KeyEnum.PRIMARY".
  • A Pydantic dataclass configured with serialize_by_alias=True, containing child: ChildSource = Field(alias="item"): the traversal searches child instead of item.

With a frozen, extra="forbid" ChildSource, every current-base/head manifest is identical and registers with HTTP 200. The current-base observation receives 202; the head-generated observation receives 422 invalid_source_ingestion. Removing only the leaked declaration restores 202 and the exact original receipt in all three cases, on both databases. Please preserve Source ownership through the actual serialized names so accepted definitions generate payloads satisfying their own manifests.

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.

Re-tested at 9963fd5a: the boolean/Enum dictionary keys and aliased Pydantic dataclass cases now replay successfully. One P2 remains in the same ownership traversal (observations.py:286-294) when accepted models use custom serializers:

  • Typed dictionary keys: dict[Annotated[str, PlainSerializer(lambda value: value.lower(), return_type=str, when_used="json")], ChildSource] with {"PRIMARY": child} serializes the key as "primary". The new TypeAdapter(dict[Any, Any]) lookup discards that declared serializer and searches "PRIMARY", leaving the child's memory_evidence in the payload.
  • Reordered lists: a field_serializer("items") returning sorted(values, key=lambda item: item.name) changes [ChildSource(name="z-child"), CapturedReport(name="a-report", memory_evidence="Captured report evidence.")] into report-first order. The walk still pairs original values with encoded indices: it deletes the report's legitimate string field and leaves the actual Source's declaration behind.

Both reproduce through registry.resolve → transport projection → public HTTP on SQLite and real OceanBase. Current-base/head manifests are identical and register with 200; the current-base observation gets 202, while the generated head observation gets 422 invalid_source_ingestion. Correcting only field ownership restores 202 and the exact original receipt: remove the leaked Source declaration, and for the reordered case also restore the captured report field.

Please preserve Source-field ownership through the declared serialization, including key transformations and reordered elements, so generated observations satisfy their accepted manifests without deleting captured data.

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.

Re-tested at e8720629: the custom dictionary-key and reordered-list examples are fixed, including preservation of the captured report field. One P2 remains at the new Source serializer boundary (sources/models.py:121-128):

  • A Source subclass's own serializer replaces the exclusion hook. A strict nested Source with an existing @model_serializer(mode="wrap") that calls handler(self), trims its text, and returns the result retains memory_evidence. Pydantic uses that subclass serializer instead of the new inherited serializer, so the transport context does not remove the declaration.
  • Configured aliases are missed when by_alias=None. With serialize_by_alias=True and an alias generator mapping memory_evidence to evidence, the handler emits evidence. Line 127 checks only info.by_alias, which is None for the transport call, and attempts to remove memory_evidence instead.

Both reproduce through full registry resolution, transport projection, and public HTTP on SQLite and real OceanBase. The base/head manifests are identical and register with 200; the base observations receive 202, but generated head observations receive 422 invalid_source_ingestion. Removing only the leaked memory_evidence or evidence field restores the exact original receipt in each case. The same Source models also pass native capture/read/replay controls on both revisions using SQLite.

Please keep transport declaration exclusion effective for accepted Source subclasses with their own serializers, and honor the effective model alias configuration when no explicit by_alias override is supplied.

Comment on lines +239 to +240
if isinstance(properties, dict) and properties.get("memory_evidence") == signature:
del properties["memory_evidence"]

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.

[P2] Identify declaration fields by actual model ownership

Schema equality produces both false positives and false negatives. An ordinary CapturedAudit(BaseModel) with memory_evidence: MemoryEvidenceDeclaration = MemoryEvidenceDeclaration() and extra="forbid" loses its captured field from the schema. On real OceanBase through the HTTP routes, registration returns 200 but submission returns 422; preserving that field in the schema accepts the identical payload with 202.

Conversely, an adapter model also named MemoryEvidenceDeclaration makes Pydantic qualify the library declaration's $ref, so this comparison misses the inherited field. SQLite base-to-head testing then shows an unchanged neutral adapter registering/submitting with 200/202 before upgrade, but 409/422 afterward because its fingerprint changed. Please identify inherited Source metadata by actual field ownership and preserve ordinary captured data and neutral Definition identity.

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.

Rechecked at ba977912ee6da91454402f3edd3e3c8aab9fd211. The original ordinary typed CapturedAudit case is fixed (200/202), but the ownership and neutral-identity issue is not fully resolved:

  • The new title match at lines 235–240 deletes captured fields when an ordinary model and a Source share a JSON Schema title. A captured model with ConfigDict(title="AuditSource", extra="forbid") and an ordinary memory_evidence: str, embedded in AuditSource(Source), registers/submits with 200/202 on current base and 200/422 on head. Preserving that captured field restores 202. A typed-field variant also reproduces against real OceanBase.
  • The traversal at lines 262–265 misses list[ChildSource], leaving its inherited declaration in the schema. Reopening the base database at head changes the neutral fingerprint and produces registration 409 / submission 422.
  • The original custom class named MemoryEvidenceDeclaration still produces 409/422 after upgrade. The inherited property is now removed, but Pydantic's renamed surviving $defs key and $ref still alter the fingerprint; restoring only those names makes the schema identical to base.

Please use actual Source field ownership throughout the model graph and preserve neutral schema identity, including reference names. Matching display titles does not establish ownership.

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.

Rechecked at b7f8c2e0bf8743d4d4226f25d3075a11c973c1ac: the title collision, list traversal and same-name $ref examples now pass. Neutral identity still changes when a field has a concrete Source default, for example child: ChildSource = ChildSource(...).

The generator removes the declaration from the child model's fields, but source_schema.properties.child.default still contains the new memory_evidence. The generated payload is identical to base, yet the fingerprint changes from c7fdebb5... to 09b86012.... Current-base-to-head SQLite registration/submission return 409/422. On real OceanBase, registering and submitting the actual base-generated contract succeeds with 200/202; the head-generated contract then produces the same 409/422 pair. Removing only the declaration from the schema default restores the exact base schema and fingerprint.

Please exclude Definition-owned metadata when encoding Source-valued schema defaults as well. Current code: Source schema generation.

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.

Re-tested at 23d8dcd4: a direct concrete Source default now retains the exact base schema/fingerprint. Container defaults still reproduce P2. For child: list[ChildSource] = [ChildSource(...)], encode_default at observations.py:229-232 skips normalization because the default itself is a list. The neutral declaration appears at source_schema.properties.child.default[0].memory_evidence, changing the fingerprint even though the definition version and observation payload are unchanged.

After registering/ingesting the exact-base definition, SQLite and real OceanBase both reject the head-generated manifest with HTTP 409 source_conflict and its observation with 422. Removing just that default metadata restores the complete base schema and fingerprint; replaying with the base fingerprint returns the original receipt. Please normalize Source-owned metadata inside container defaults as well.

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.

Verified fixed at df3157e1 on SQLite and real OceanBase: both the direct concrete Source default and list[ChildSource] default now produce the exact base manifest and fingerprint. Re-registering the unchanged definition returns 200, and observation replay returns 202 with the original receipt. The list-default 409/422 counterexample no longer reproduces.

@AlexStocks AlexStocks added 1.4.0 v1.4.0 release tracking and removed 1.3.0 labels Oct 11, 2026
@hidb4ai

hidb4ai commented Oct 12, 2026

Copy link
Copy Markdown
Contributor

please update & resolve conflicts

@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.

Re-reviewed df3157e1 against base ee50b36d, including actual OceanBase and the configured Qwen generation/embedding services. Extraction, search, lifecycle changes, rebuild and readback pass. Ordinary root wrappers, list defaults, Subject creation and rejection of incompatible stamped declarations now pass their counterexamples.

Three P2 issues remain: metadata leakage through serialized nested shapes, new validation rejecting valid aliased/container Sources, and remaining fresh writers skipping Definition declarations. Existing threads are updated; the new validation regression is inline. Please address these before merge.

The unit matrix also fails because the Subject rollback test's fail_second replacement does not accept apply_definition. An external control with only keyword forwarding updated passes all unchanged rollback assertions; the repository test still needs that update.

Comment thread src/powercontext/sources/definitions.py Outdated
definition = self.definition_for_source(source)
stamped = source.model_copy(update={"memory_evidence": definition_memory_evidence(definition)})
try:
type(stamped).model_validate(stamped.model_dump(mode="python"))

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.

[P2] Validate stamped Sources without losing their accepted input representation

A valid Source with content: str = Field(alias="body") now fails both context.sources.resolve(...) and direct context.sources.add(...): this dump emits content, then validation requires body. Current-head SQLite and real OceanBase both raise InvalidSourceResultError before writing; the identical previous-head SQLite probe successfully writes and reads both paths. Native persistence already encodes these fields with by_alias=True. The same new boundary rejects set/frozenset fields containing frozen ChildSources because Python dumping attempts to put serialized dictionaries into a set and raises TypeError. Validate the authoritative stamped value using a representation compatible with the accepted Source model, while retaining pre-write rejection of genuinely invalid declarations.

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.

Verified fixed at e39cca09 on SQLite and real OceanBase. An alias-only Source (content: str = Field(alias="body")) now succeeds through both resolve-then-add and direct add, followed by persisted get/read. Frozen ChildSources inside set/frozenset fields also resolve, persist, read back with their container types preserved, and replay idempotently.

The invalid-declaration safeguard remains intact: stamping a neutral declaration onto a model that requires verified evidence raises InvalidSourceResultError before any Source journal row is committed; the compatible verified Definition control writes and reads successfully.

@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.

Reviewed e39cca0943ef6b568cdf85a954c563762e5df5a3 against current base ba739963192fc1b69b84231e9445d426f1b88630.

One P2 remains in the existing nested Source transport thread: boolean/Enum dictionary keys and aliased Pydantic dataclasses still generate observations rejected with HTTP 422. All three were reproduced on SQLite and real OceanBase, with metadata-only corrections restoring the original receipts. The native validation and fresh-lineage declaration findings are fixed; verification details are in their threads.

Validation: 135 focused local tests passed; Python and JavaScript contract generation checks passed. Using the configured real Qwen generation/embedding services and OceanBase, Source capture/replay, Atomic extraction, text/vector/hybrid retrieval, forget/restore, rebuild, and runtime close/reopen all passed. Both isolated test databases were removed and their absence verified.

HTTP probes used the real application lifespan through ASGITransport; runtime reopen was within the same process. Local live-backend testing covered OceanBase. The current CI logs separately confirm all 47 clean-migration cases passing on each of OceanBase and SeekDB.

@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.

Reviewed 9963fd5ad1f64b1599255be761ab2399fa883cf8 against current base 666b043a6947caabbbd471ef1601695238270eee.

One P2 remains in the existing Source transport thread: custom key serialization and list reordering break declaration ownership, producing HTTP 422; the latter also deletes a captured report field. Both are reproduced on real OceanBase with exact-receipt correction controls. All 18 previously exercised transport/default cases now pass, including the last three reported examples. Native persistence and lifecycle regressions also pass.

Validation: 139 focused local tests and both contract-generation checks passed. Real Qwen generation/embedding with OceanBase passed capture/replay, extraction, text/vector/hybrid search, forget/restore, rebuild, and runtime close/reopen. Both isolated test databases were removed and verified absent. HTTP tests used the actual application lifespan through ASGITransport; reopen was within one process.

CI is not fully green: the Windows Kimi host test completed memory write/read and emitted its success marker, then Node exited with 3221226505. That log alone does not establish a Source regression.

@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.

Reviewed e87206296b0108c61887a2205e60db1c67ca0a91 against current base a1ed8553149ced3e15756642905a8ea315a3a4a8.

One P2 remains in the existing Source transport thread: a Source subclass's own serializer and configured declaration aliases can bypass metadata removal, producing HTTP 422. Both were reproduced on real OceanBase with metadata-only corrections restoring the original receipts. All 20 previously exercised transport/default cases now pass, including custom keys and reordered lists.

142 focused local tests and both contract-generation checks passed. Real Qwen generation/embedding with OceanBase passed capture/replay, extraction, text/vector/hybrid search, forget/restore, rebuild, and runtime close/reopen; native lifecycle probes also passed. Both isolated databases were removed and verified absent. HTTP tests used the actual app lifespan through ASGITransport, and runtime reopen was within one process.

Current CI has 37 successful checks and one skipped check, including successful Windows Kimi and OceanBase/SeekDB migration jobs. Those results do not cover the two failing configurations above.

@hireview001 hireview001 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

  • Resolve merge conflicts with the target branch.

Comment thread src/powercontext/sources/definitions.py Outdated
definition = self.definition_for_source(source)
stamped = source.model_copy(update={"memory_evidence": definition_memory_evidence(definition)})
try:
type(stamped).model_validate_json(stamped.model_dump_json(by_alias=True))

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[P2] Reject lossy declaration serialization before persistence

With a non-neutral Definition and a Source serializer that emits only the pre-existing fields, this validation succeeds by reconstructing the neutral default, but its result is discarded. The persisted Source then reads back with a different declaration and identical retries conflict; verify that serialization preserves the stamped declaration or reject the encoding before writing.

def _normalize_default(value: Any) -> Any:
"""Serialize defaults while removing Source-owned declaration fields."""
if isinstance(value, BaseModel):
return value.model_dump(mode="json", context={"source_transport_payload": True})

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[P2] Preserve aliases when normalizing schema defaults

For a default nested model whose text field has alias body, this normalization emits {"text":"hello"} where the base emitted {"body":"hello"}. That changes an unchanged neutral Definition's fingerprint and causes re-registration to conflict with its persisted manifest; preserve the schema generator's alias policy when serializing defaults.

# Definition-owned metadata is not part of it. Carrying it here would make a
# generated observation fail its own schema, and would change the payload of an
# unchanged Source, so replaying a pre-upgrade observation would conflict.
payload = _json_object(source.model_dump(mode="json", context={"source_transport_payload": True}))

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[P2] Preserve transport exclusion with custom Source serializers

A Source subclass with extra="forbid" and its own @model_serializer(mode="wrap") overrides Source.serialize_source, so even a serializer returning only handler(self) retains memory_evidence while the generated manifest excludes that property. Previously valid observations are consequently rejected by schema validation; ensure declaration exclusion also applies when Sources define custom serializers.

Wrap Source serialization outside adapter model serializers and honor the
effective alias policy for transport payloads and schema defaults. Reject
encodings that lose the Definition-owned declaration before persistence.

Add frozen neutral-upgrade HTTP replay cases and native persistence rejection
and readback coverage. Merge current master, regenerate API/Desktop contracts,
and register the standalone continuity fixture path for type checking.
Keep immutable evidence declarations in Source payloads and use existing
Atomic Memory lineage. Remove unused legacy entry snapshot and lifecycle
types, the evidence table, and its maintenance and fixture additions.

Preserve the existing legacy schemas and migration behavior. Restore the
Atomic Memory persistence, search, rebuild, and scope isolation tests.
@hidb4ai hidb4ai added 1.5.0 v1.5.0 release tracking and removed 1.4.0 v1.4.0 release tracking labels Oct 19, 2026
Merge master and retain both Definition-owned Source declarations and
Artifact rollback metadata. Regenerate Desktop contract fingerprints.

Cover Profile rollback and declaration persistence after reopening with a
neutral Source registry.
Include the configured Authorization header fix from master.

@hireview001 hireview001 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

  • Resolve merge conflicts with the target branch.
  • CI failed: Atomic Memory clean migration (OceanBase) (failure).
  • CI failed: Atomic Memory clean migration (SeekDB) (failure).
  • CI failed: Main (failure). Failed steps: Atomic Memory clean migration (SeekDB) / Run clean migration evidence, grants and vectors without skips.
  • CI failed: E2E harness (failure). Failed steps: Atomic Memory clean migration (OceanBase) / Run clean migration evidence, grants and vectors without skips.

fingerprint: Annotated[StrictStr, Field(pattern="^sha256:[0-9a-f]{64}$")]
source_schema: dict[str, Any]
projections: Annotated[list[SourceProjectionManifest], Field(max_length=16)]
memory_evidence: MemoryEvidenceDeclaration | None = None

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[P1] Omit absent declarations from registration responses

For a neutral Definition, the runtime omits the declaration, but the transport model fills this default and the registration route emits "memory_evidence": null, violating the non-nullable OpenAPI schema and causing pre-declaration Python clients with extra="forbid" to reject the response. Omit absent declarations from HTTP registration responses so existing workers can continue registering after a Server upgrade.

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.

Confirmed at 57a821f2 through the public registration endpoint with SQLite. A neutral manifest registers with HTTP 200, but the response includes "memory_evidence": null. Validation against this head's OpenAPI schema rejects it (None is not of type 'object'). The base Python response model also rejects the response with extra_forbidden at memory_evidence, so an existing worker fails when only the Server is upgraded.

Please omit the undeclared field at the registration response boundary and preserve registration compatibility with the old client. Avoid globally dropping nulls, since captured data and projection values can legitimately be null.

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.

Re-tested at 5d765ebd. This P1 is still reproducible after 2cd004f8: source_definition_manifest_response() removes the field before model_validate(), but the generated HTTP model restores memory_evidence=None, and the route's default response serialization emits it again.

The public registration endpoint still returns HTTP 200 with "memory_evidence": null. The current OpenAPI schema rejects that value, and the Python response model from current base c0c38dea rejects the response with extra_forbidden. Please omit the undeclared field at the actual response serialization boundary and verify that the old client can read the resulting response. Clearing the input dictionary before constructing the response model does not preserve the wire shape.


def _serialize_source(source: Source, handler: SerializerFunctionWrapHandler, info: SerializationInfo) -> Any:
value = handler(source)
if isinstance(value, dict) and isinstance(info.context, dict) and info.context.get("source_transport_payload"):

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[P2] Preserve declaration filtering through nested field serializers

A nested Source field serializer that returns child.model_dump(mode="json") loses the outer transport context, leaving memory_evidence in the child payload while _SourceInputSchema excludes it. This previously valid adapter now fails observation validation when the child forbids extras; preserve declaration exclusion through this serialization path.

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.

Confirmed at 57a821f2 through registry.resolve -> transport projection -> public HTTP submission. A parent Source with @field_serializer("child") returning child.model_dump(mode="json") leaves the child's memory_evidence in the payload because that dump does not inherit the transport context. With extra="forbid" on the child, the generated observation fails its own advertised schema.

The base and head manifest fingerprints are identical. The base observation receives HTTP 202; the head observation receives 422 invalid_source_ingestion. Removing only the leaked child declaration restores 202 and idempotent replay. Please keep declaration exclusion effective through this accepted serialization path.

The earlier subclass wrap-serializer and configured-alias examples now pass at this head.

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.

Re-tested at 5d765ebd against current base c0c38dea: this P2 remains. Through registry.resolve -> transport projection -> public HTTP submission, a nested @field_serializer("child") returning child.model_dump(mode="json") still loses the outer transport context and leaks the child's memory_evidence into the payload.

The base and head manifest fingerprints are identical. The base observation returns 202, while the head observation returns 422 invalid_source_ingestion with a strict child Source. Removing only the leaked declaration restores 202 and idempotent replay. Accepted Source serializers must generate observations that satisfy their advertised schemas. These reproductions used SQLite and ASGITransport.

@AlexStocks

Copy link
Copy Markdown
Contributor

@jiannnnyyyyy Please fix the file confliction.

@AlexStocks AlexStocks added the 1.4.1 v1.4.1 release tracking label Oct 20, 2026

async with open_builtin_contexts(config) as contexts:
context = await contexts.get(scope.scope_id)
assert await contexts.records.get_artifact(scope.scope_id, "profile", "profile") == restored

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.

[P2] 新用例在该 head 上让两个后端 job 变红:整条 ArtifactRecord 相等断言无法跨后端成立

tests/e2e/test_source_declaration_contract.py:261 在重开上下文后断言 await contexts.records.get_artifact(...) == restored。该对象包含 created_at,而这一列在这条链路上不可移植,两个 MySQL 系后端都以不同方式失败:

  • Atomic Memory clean migration (OceanBase)(job 118381565778,2026-10-20T00:50:15Z,test_artifact_rollback_preserves_source_declaration_and_revision_metadata[oceanbase]):读回 created_at=2026-10-20 00:50:10+00:00,写入侧是 00:50:09.931190。
  • Atomic Memory clean migration (SeekDB)(job 118381539652,2026-10-20T00:40:36Z,同用例 [seekdb]):读回 .476426,写入侧 .478797。

机制(按 head 57a821f2 的源码核对,不是猜测):

  1. 列类型来自 tables.py 的 Column("created_at", DateTime(timezone=True), nullable=True),OceanBase 侧由 revision_metadata_schema.py::ensure_artifact_revision_metadata_schema 用 CreateColumn 生成 DATETIME,无小数秒;00:50:09.931190 落库被取整到 00:50:10,回读必然与写入值不等。
  2. seekdb 上差异方向相反(读回更早),说明 replace_artifact() 返回对象的 created_at 与其写入行的值并非同一个采样,回读后自然不相等。

两侧 revision、restored_from_revision、reason、created_by、sources、content、content_digest 完全一致,差异只在 created_at,因此这不是声明持久化本身的问题,但当前 head 上这两个 job 都是红的(该 head 38 项 check 中只有这两项非 success),需要先让用例与后端精度对齐。

建议:改为逐字段断言身份与声明(revision / restored_from_revision / reason / created_by / content / sources[].memory_evidence),created_at 按秒级归一后比较;若认为写入返回值应与回读一致,则需另开改动让 replace_artifact() 复用落库时间戳,而不是在本用例里断言两者相等。

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.

Confirmed from both failed CI jobs at 57a821f2: the new whole-record equality assertion at line 261 fails on created_at after reopening. The raw logs show:

  • OceanBase: read 00:50:10, write 00:50:09.931190.
  • SeekDB: read 00:40:31, write 00:40:31.478797.

The content, lineage, and rollback metadata agree; these failures do not demonstrate a Source declaration persistence defect. Please assert the intended identity, content, rollback metadata, and declaration fields directly, handling timestamps according to backend precision if needed. The current assertion makes both migration jobs fail. These backend results were inspected in CI; this review's local execution used SQLite.

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.

Verified fixed at 5d765ebd. The reopened record is now checked field by field for identity, content, lineage, digest, actor, and rollback metadata, and the persisted Source declaration is still asserted. The test no longer assumes exact created_at equality across backend storage precision.

The current OceanBase and SeekDB CI logs both explicitly show test_artifact_rollback_preserves_source_declaration_and_revision_metadata passing on their respective real backends. This finding is no longer a blocker. These backend results were verified from CI; local execution used SQLite.

@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.

Reviewed 57a821f2f348d566261ae791d1f4032c21de5f6f against merge base 683e2db0; current master is 1eb7c2ed.

Changes are still needed for the three existing findings: neutral registration responses break old clients, nested field serializers leak declaration metadata, and the new rollback assertion fails both backend CI jobs. Reproduction details are in those threads. The previous subclass serializer and alias cases are fixed.

Local HTTP reproductions used SQLite and ASGITransport. The migration failures were verified from CI logs. Merge conflicts also remain.

@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.

Re-reviewed 5d765ebd7551e7fa85018aa3e39e273403347c06 against current base c0c38dea5178679c6bca95381a1008e8df1826e2.

Two existing findings still require changes: P1: neutral registration responses break old clients and P2: nested field serializers leak declaration metadata. Both were reproduced on this head; details are in their threads.

The rollback timestamp assertion is fixed, its scenario passes in both real-backend CI logs, and merge conflicts are resolved. Current CI is green apart from one skipped check. Local counterexamples used SQLite and ASGITransport; OceanBase and SeekDB results were inspected in CI.

@hidb4ai hidb4ai removed the 1.4.1 v1.4.1 release tracking label Oct 23, 2026

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

1.5.0 v1.5.0 release tracking

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants