Skip to content

feat(memory): add bounded Memory entry pagination - #1767

Open
wutongyuonce wants to merge 12 commits into
oceanbase:masterfrom
wutongyuonce:feat/memory-entry-pagination-1656
Open

wutongyuonce wants to merge 12 commits into
oceanbase:masterfrom
wutongyuonce:feat/memory-entry-pagination-1656

Conversation

@wutongyuonce

@wutongyuonce wutongyuonce commented Sep 28, 2026 •

Copy link
Copy Markdown
Contributor

Which issue or RFC does this PR close?

Closes #1656.

Detailed design: RFC 1656: bounded Memory entry pagination.

Rationale for this change

The existing Memory entry list resolves the complete manifest and loads all selected entry bodies. Slicing its HTTP response would not bound persistence or response-assembly work. Clients need a stable, authorized directory page before choosing exact entries to inspect.

What changes are included in this PR?

  • Add the additive POST /v1/memory/entries/query operation, generated SDK support, Server mapping, and MCP exposure. The legacy full-result operation is unchanged.
  • Return compact exact identities and metadata in entry_id ASC order, with a default limit of 50, maximum of 100, signed continuation, and a 4 MiB encoded item budget.
  • Pin each traversal to one immutable Memory revision. Tag-filtered cursors additionally bind a durable tag generation and require restart after an effective tag change.
  • Maintain a rebuildable revision-valid directory projection atomically with Memory commits, while preserving unchanged search projections and exact authoritative records. Upstream compaction removes current membership without changing a traversal pinned to an earlier revision.
  • Add a feature-scoped offline plan / revision-batched apply / verify migration. Until verification succeeds, only the new query returns 503 memory_query_index_unavailable; it never falls back to the unbounded path.
  • Add operator documentation, MySQL/OceanBase restore ordering, integration capability metadata, and a reproducible SQLite scale measurement for 200, 1,000, and 5,000 entries.

Are there any user-facing changes?

Yes, but the API change is additive. Existing list, exact-detail, search, and write behavior is unchanged.

Existing databases must complete the documented offline Memory query-index migration before using the new operation. The response is intentionally a compact directory and omits entry text/source/artifact references; callers use the existing exact-detail operation for those fields.

How was this change tested?

  • make check
  • make docs-test — built and verified 859 public pages
  • make build — source distribution and wheel built
  • make contract-test — 50 passed
  • make js-test — 266 DSH unit tests and 9 e2e tests passed; the committed bundle matches a fresh build
  • Focused persistence, migration, capacity/compaction, HTTP/SDK, and MCP/access suite — 116 passed, 17 skipped because a live OceanBase service was not configured; includes pinned traversal across compaction
  • uv run python scripts/measure_memory_directory.py — SQLite 3.50.4 at 200/1,000/5,000 entries; each page returned 100 and materialized 101 compact rows, used the directory and entry-version indexes, and a one-entry revision added/closed one directory row with no head-row count change

The full local Python suite is not claimed green: the native-code parser worker failed before test logic with code_parser_failed on this macOS/Python 3.13 environment in the pre-merge run, and the full suite was not rerun locally after integrating master. Fresh hosted CI validates the updated head separately. The live OceanBase migration/capacity tests are present but skipped locally because POWERCONTEXT_TEST_OCEANBASE_URL is not configured.

AI usage statement

OpenAI Codex GPT-6 Sol was used for repository analysis, implementation, tests, documentation, and validation. The contributor reviewed the design boundaries and generated changes.

Preserve pagination alongside upstream capacity and MCP additions. Keep revision-valid directory updates in the same commit transaction and verify pinned traversal across compaction. Regenerate the checked-in DSH bundle to satisfy the package freshness check.
@wutongyuonce

wutongyuonce commented Sep 28, 2026 •

Copy link
Copy Markdown
Contributor Author

@Teingi CI update: the previous dsh-package failure was caused by a stale checked-in bundle. This is fixed in 07375594, and the full DSH job now passes, including the real runtime checks. The conflicts with current master are also resolved.

23 checks are now passing, including Python 3.11–3.14 and OceanBase acceptance. The only remaining failure is Acceptance (sqlite). Its uploaded exception evidence shows that the acceptance, LoCoMo, and fail-fast batches all stopped during agent dependency installation, before scenario execution:

Failed to fetch: https://pypi.org/simple/inquirer-textual/
HTTP status server error (503 Service Unavailable)

The download failed after three retries. This remaining failure is an external dependency-download error, not a pagination assertion failure; it does not establish a SQLite acceptance pass yet.

Could a maintainer please rerun the failed SQLite acceptance job? My attempt to rerun it was rejected with HTTP 403 due to repository permissions.

@Teingi Teingi added the 1.3.0 label Sep 28, 2026
Preserve both the Memory directory cursor secret and the runtime model usage recorder in scoped service wiring.
@Zxf-xufeng

Copy link
Copy Markdown
Member

Minimize additional tables and justify their responsibilities.
This PR introduces pc_memory_entry_directory, pc_memory_tag_generations, and pc_memory_query_index_schema. Please evaluate whether extending existing tables can reduce this footprint. For example, the tag generation could potentially be stored on the owning Artifact head, while migration state should be reviewed alongside the unified migration design.
A separate directory projection may still be justified for efficient historical pagination. Please document why existing tables cannot support it through appropriate extensions. Any alternative must preserve revision-pinned traversal, deactivation/reactivation history, compaction behavior, and bounded reads. Moving an unbounded history into a JSON field would require performance evidence before being considered equivalent.

@Zxf-xufeng

Copy link
Copy Markdown
Member

Align schema changes and historical backfills with PR #1771.
#1771 proposes a unified database migration process. This PR currently adds startup table creation and marker writes, plus a separate memory-query-migrate command that independently creates tables, backfills data, and publishes readiness. Its standalone verify also updates the database.
Please separate schema migration, projection backfill, and readiness verification according to that proposed process:

  • Deliver schema changes through versioned migrations, with historical projection reconstruction as an explicit, idempotent data task.
  • Keep ordinary startup and inspection commands read-only for existing databases; publish projection readiness during explicit migration execution.
  • Use the unified execution boundary for maintenance, locking across batches, backup choices, and final verification.
  • Distinguish schema readiness from projection readiness. Legacy APIs may remain available during an optional backfill only after their required schema has been migrated and validated.
  • Cover interrupted/resumed backfills and verification failures, including a previously complete projection that subsequently fails verification.
    docs(rfc): define unified versioned database migrations #1771 is still a proposal, so this does not necessarily require waiting for the framework to ship. If this PR lands before framework enablement, please document its baseline adoption path. If it remains unmerged when the framework is enabled, it should follow the unified migration process rather than retain an independent migration authority.

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

Reviewed 6eb0eb3. Three reproducible issues are detailed inline: unbounded authorization work per page, oversized cursors for valid tag filters, and directory reads ignoring an explicitly bound transaction. The third issue is limited to explicit backend composition in the reproduction.

_COLLECTION_CONTENT_OPERATIONS = frozenset({
"search_memory",
"list_memory_entries",
"query_memory_entries",

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.

[P2] Keep owner-readiness checks bounded for directory pages

Adding this operation to the collection preflight makes every page materialize all retained identities in the Scope and perform a sequential owner lookup for each. With enforced authorization and real SQLite HTTP requests, limit=1 read 200/1,000 identities and issued 200/1,000 owner SELECTs respectively; continuation pages repeated the same work, taking approximately 0.57/2.52 seconds to return one item. The bounded directory query therefore still has whole-collection work ahead of it. Please preserve owner-readiness enforcement without a whole-Scope application scan and per-entry database round trips, and include the enforced-auth HTTP path in the scale checks.

"scope_id": scope_id,
"memory_artifact_id": artifact_id,
"include_inactive": query.include_inactive,
"tags": [] if query.tag_filter is None else list(query.tag_filter.keys),

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.

[P2] Keep cursors within the API limit for every legal tag filter

Embedding the complete normalized tags can exceed the 4096-character limit on both cursor and next_cursor. I tagged two entries with 16 distinct, valid 64-character Chinese labels and queried with that filter and limit=1: the runtime generated a 4592-character cursor, and response mapping returned HTTP 422 with an error on next_cursor. The same filter with limit=100 returned both entries successfully because no cursor was needed; submitting the generated cursor also returned 422. Please bind the filter with a bounded representation, or reconcile the cursor limits so every accepted filter can complete pagination.

expected_tag_generation: int | None,
after_entry_id: str,
) -> tuple[int, int | None, list[Mapping[Any, Any]]] | None:
async with self._database.transaction() as connection:

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.

[P2] Reuse the bound connection for directory reads

When a RelationalMemoryBackend is constructed with connection=connection inside an open file-SQLite transaction, this method starts a separate transaction and cannot see the caller's pending writes. I reproduced remember() followed by entries() and head() on the same bound service returning one entry at revision 1, while query_directory() returned memory_ref=None and no items; it became correct after the outer commit. Please use the supplied connection consistently with the other backend reads. This reproduction covers explicit backend transaction composition; the default HTTP query path currently uses an unbound service.

@PsiACE

PsiACE commented Oct 1, 2026

Copy link
Copy Markdown
Member

@wutongyuonce Thanks for the implementation, tests, and scale validation in this PR. We are reviewing unified database migrations (#1771) and the Memory redesign (#1809, dependent on #1803). These designs would affect the directory projection, revision cursors, and migration approach used here.

We therefore plan to close this PR for now and keep the pagination requirement in #1656 open until those designs are settled. We'd welcome your continued involvement, and the work you've done provides a useful reference for the next implementation.

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

4 participants