Skip to content

Add per-memory storage expiry (end_epoch/expires_at) - #543

Open
harrymove-ctrl wants to merge 18 commits into
feat/memory-read-apifrom
feat/memory-expiry-api
Open

Add per-memory storage expiry (end_epoch/expires_at)#543
harrymove-ctrl wants to merge 18 commits into
feat/memory-read-apifrom
feat/memory-expiry-api

Conversation

@harrymove-ctrl

Copy link
Copy Markdown
Collaborator

Summary

WALM-296: adds end_epoch/expires_at to every item in GET /v1/owners/{owner}/memories (built by WALM-295, PR #537, not yet merged — this PR targets feat/memory-read-api directly rather than dev, so the diff here is scoped to WALM-296 only).

  • New vector_entries columns: end_epoch, expires_at, expiry_synced_at (migration 014), plus an index on expiry_synced_at (migration 015).
  • end_epoch is populated at write time on the 2 write paths that have it for free (primary upload + seal-store); the other 3 (metadata-transfer recovery, finalize-recovery, restore) start NULL.
  • A new periodic background sweep (main.rs, mirrors the existing evict_expired_delegate_keys/fail_stale_remember_jobs pattern) queries the Walrus chain for storage-lease end epochs and converts them to expires_at timestamps, batched by owner, never touching the hot listing path.
  • Epoch→timestamp conversion (expires_at_from_epoch) is anchored on the current epoch, not Walrus genesis — expires_at = now + (end_epoch - current_epoch) * epoch_duration. This self-corrects every sweep cycle regardless of whether epoch_duration ever changes, and avoids extrapolating from a fixed point over potentially hundreds of epochs.
  • status on MemoryItem changes from a hardcoded "active" to a real derivation: "expired" if expires_at is in the past, "active" otherwise (including when not yet synced — an intentional optimistic default).

Migration notes

  • 014: single-transaction, metadata-only (3 nullable ADD COLUMNs, no rewrite).
  • 015: CREATE INDEX CONCURRENTLY, its own file/transaction (required — cannot run inside a transaction).

Test plan

  • Full suite green: 440 passed, 0 failed, 30 ignored (cargo test --bins, against real local Postgres).
  • Every task individually implemented + reviewed (fresh implementer + independent reviewer per task).
  • Final whole-branch review (opus) across the full diff — found and fixed: a sweep failure-ordering bug (schedule-fetch failure could mark rows synced with nothing written, invisible for 24h), a wrong index (NULLS LAST when the query needs NULLS FIRST — verified via EXPLAIN, cost dropped from 163 to 5.96), and an arithmetic overflow panic in the epoch-delta computation (function runs in an unsupervised background task with no restart wrapper).
  • Live sweep boot test against real chain data confirmed correct end-to-end behavior (74 rows resolved correctly on first tick).

Follow-ups (non-blocking, deferred)

  • The sweep's per-owner on-chain blob lookup has no query limit, so an owner with a very large blob collection could time out before resolving (sidecar-side fix needed).
  • The sidecar's nonexistentBlobIds verification (for blobs no longer on-chain) is fetched and parsed but not yet consumed — those rows stay NULL/"active" indefinitely rather than reflecting a more accurate state.
  • One malformed row can currently fail an entire owner's sweep batch (safe — retried in 24h — but not per-row isolated).
  • Sweep throughput (100 rows/300s) has a ceiling of ~28.8k rows/day against a 24h resync target; will need prioritization (e.g. end_epoch IS NULL first) once the table grows past that.
  • A second SuiClient/gRPC client instance is now constructed (separate quota from security_delete_sui) — small, accepted duplication, not unified.
  • docs/api/memory-read-api.md documents the final shape.

Scope note

This targets feat/memory-read-api (PR #537), not dev — WALM-296 fundamentally extends WALM-295's MemoryItem/query_owner_memories, so it's stacked on top rather than independent.

@railway-app
railway-app Bot temporarily deployed to Walrus Memory / dev August 6, 2026 07:30 Inactive
/api/remember returns {"job_id": ..., "status": ...}. The 64KB and
large-size test functions asserted "id" in result, so they always
reported failure even when the underlying job completed successfully.
@railway-app
railway-app Bot temporarily deployed to Walrus Memory / dev August 6, 2026 08:20 Inactive
@ducnmm

ducnmm commented Aug 6, 2026

Copy link
Copy Markdown
Collaborator

Architecture question after confirming the response contract with the Console team:

  • memory_id, namespace_id, blob_id, created_at, size, and status are required/non-null.
  • end_epoch and expires_at must remain nullable.
  • agent_id and package_id are nullable.

Since the existing V1 rows have already been backfilled and V1 leases cannot be renewed, can we simplify WALM-296 to a DB-only indexed flow?

  1. GET /v1/owners/{owner}/memories reads expiry only from vector_entries and never performs per-memory chain verification.
  2. Existing rows are handled by the one-time backfill.
  3. Every new write path, including metadata-transfer recovery, finalize, restore, and manual paths, persists end_epoch and expires_at when the memory is written.
  4. No periodic full reconciliation sweep is required for Phase 1.

This keeps reads cheap and avoids repeatedly scanning owner collections or checking each blob only to determine whether it has expired. It also removes the sweep retry/throughput/multi-replica concerns. The main implementation requirement becomes ensuring that all write and recovery paths preserve expiry metadata; nullable expiry remains available for genuinely unresolved/error cases.

If continuous reconciliation of external deletions is a Phase 1 requirement, could we document that requirement and its freshness/SLA explicitly? Otherwise I would prefer treating it as a later reconciliation feature rather than coupling it to this read-API change.

@jessiemongeon1

Copy link
Copy Markdown
Collaborator

Style Guide Audit

Audited 2 file(s) against the Sui Documentation Style Guide.

1 violation(s) found. All must be fixed before merge.

docs/api/memory-read-api.md (1 violation(s))

1 violation(s) (1 regex, 0 claude)

  • Line 166 — No em dashes in prose
    • Current:
    • Fix: Rewrite with comma, parentheses, or split sentence

Automated audit using the Sui Documentation Style Guide.

@jessiemongeon1

Copy link
Copy Markdown
Collaborator

Style Guide Audit

Audited 2 file(s) against the Sui Documentation Style Guide.

26 violation(s) found. All must be fixed before merge.

docs/api/memory-read-api.md (26 violation(s))

26 violation(s) (26 regex, 0 claude)

  • Line 1 — H1 only in frontmatter
    • Current: # Memory Read API (WALM-295)
    • Fix: Use ## or lower for section headings
  • Line 14 — No em dashes in prose
    • Current:
    • Fix: Rewrite with comma, parentheses, or split sentence
  • Line 28 — Capitalize testnet in prose
    • Current: testnet
    • Fix: Testnet
  • Line 28 — Use present tense, not future
    • Current: will
    • Fix: Use present tense verb
  • Line 39 — No em dashes in prose
    • Current:
    • Fix: Rewrite with comma, parentheses, or split sentence
  • Line 40 — No Latin abbreviations
    • Current: e.g.
    • Fix: for example
  • Line 40 — No em dashes in prose
    • Current:
    • Fix: Rewrite with comma, parentheses, or split sentence
  • Line 41 — No em dashes in prose
    • Current:
    • Fix: Rewrite with comma, parentheses, or split sentence
  • Line 42 — No Latin abbreviations
    • Current: e.g.
    • Fix: for example
  • Line 44 — No em dashes in prose
    • Current:
    • Fix: Rewrite with comma, parentheses, or split sentence
  • Line 46 — No em dashes in prose
    • Current:
    • Fix: Rewrite with comma, parentheses, or split sentence
  • Line 47 — No em dashes in prose
    • Current:
    • Fix: Rewrite with comma, parentheses, or split sentence
  • Line 48 — Use present tense, not future
    • Current: will
    • Fix: Use present tense verb
  • Line 53 — "on-chain" is one word → "onchain"
    • Current: on-chain
    • Fix: onchain
  • Line 55 — "on-chain" is one word → "onchain"
    • Current: on-chain
    • Fix: onchain
  • Line 68 — No em dashes in prose
    • Current:
    • Fix: Rewrite with comma, parentheses, or split sentence
  • Line 70 — "on-chain" is one word → "onchain"
    • Current: on-chain
    • Fix: onchain
  • Line 74 — No em dashes in prose
    • Current:
    • Fix: Rewrite with comma, parentheses, or split sentence
  • Line 91 — "on-chain" is one word → "onchain"
    • Current: on-chain
    • Fix: onchain
  • Line 112 — No em dashes in prose
    • Current:
    • Fix: Rewrite with comma, parentheses, or split sentence
  • Line 118 — No em dashes in prose
    • Current:
    • Fix: Rewrite with comma, parentheses, or split sentence
  • Line 122 — No em dashes in prose
    • Current:
    • Fix: Rewrite with comma, parentheses, or split sentence
  • Line 166 — No em dashes in prose
    • Current:
    • Fix: Rewrite with comma, parentheses, or split sentence
  • Line 172 — "on-chain" is one word → "onchain"
    • Current: on-chain
    • Fix: onchain
  • Line 189 — No em dashes in prose
    • Current:
    • Fix: Rewrite with comma, parentheses, or split sentence
  • Line 199 — "on-chain" is one word → "onchain"
    • Current: on-chain
    • Fix: onchain

Automated audit using the Sui Documentation Style Guide.

@harrymove-ctrl

Copy link
Copy Markdown
Collaborator Author

Status check — WALM-296

Following the Linear thread: the updated_at-bump-on-every-reverify fix and the dangling migration-011 reference are addressed (2026-08-11). Two things before this is ready for a full re-review:

  1. This PR currently shows mergeable: CONFLICTING against its base (feat/memory-read-api, i.e. WALM-295: Owner-scoped memory read API (namespaces/memories/agents) #537's branch) — likely just needs a rebase now that WALM-295: Owner-scoped memory read API (namespaces/memories/agents) #537 has moved, but please confirm and push before I do the deep pass.
  2. Style-guide-audit bot still shows 1 outstanding violation on docs/api/memory-read-api.md:166 (em dash).

Once rebased and CI is clean I'll do the full read-through (Nikola's item #2, the sweep's 24h-predicate/backlog concern, is filed as WALM-323 already so not blocking here). Also flagging: this PR isn't linked in the Notion "GitHub PR" field for WALM-296 — worth adding so it's discoverable from the board.

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.

4 participants