Skip to content

fix(*): give bulk memory writes the extraction budget and halve the import batch - #563

Closed
gloryfromca wants to merge 3 commits into
mainfrom
fix/import_bulk_store_budget
Closed

gloryfromca wants to merge 3 commits into
mainfrom
fix/import_bulk_store_budget

Conversation

@gloryfromca

Copy link
Copy Markdown
Member

Summary

Every real cold-start import source failed against a live EverOS, and the failure was on the client side: the importer's non-final appends ran under the per-turn store budget (10s plus 0.5s per message on main), while EverOS extracts on the add itself. Measured against a real service, a 15-message batch took 12s, a 52-message batch 24s, and a batch of 100 ran past the six-minute extraction budget. On a real machine 14 of 18 memory-file sources failed at the client while the server kept writing, so a retry would have duplicated what had already landed.

Two changes, one per layer:

  • The importer marks its appends with metadata["bulk"], and the EverOS backend gives a bulk write the extraction budget (_MEMORIZE_TIMEOUT_S) outright instead of the per-message estimate. Nothing waits on an import write, so the turn budget's reason does not apply. Chat turns carry no flag and keep the short budget. The hosted backends (Mem0, Zep, MemOS) do not read the incoming metadata, so the key is invisible to them.
  • _BATCH_MSG_LIMIT drops from 100 to 50, the top of the range where EverOS extraction cost is still linear in the message count. A pin test records the measurement so the ceiling is not raised back casually.

Not in this PR: the web wizard's sync step still hardcodes the full tier and shows no progress; the CLI tier labels (minutes / hours) predate these measurements. Both are follow-ups once the numbers below are agreed.

Type

  • Fix
  • Feature
  • Docs
  • CI / tooling
  • Refactor
  • Other

Verification

Unit tests (the four files that exercise the importer and the EverOS backend):

uv run pytest tests/test_importer_orchestrator.py tests/test_everos_backend.py tests/test_cli_import_commands.py tests/test_cli_onboard_commands.py -q
585 passed

Reverse check, with the two source files reverted and the tests kept: TestBatching::test_msg_count_limit, TestBatching::test_a_batch_stays_inside_the_zone_everos_extracts_linearly, TestMetadata::test_metadata_marks_the_write_bulk_and_names_no_owner and TestWriteBudgetFollowsTheCaller::test_a_bulk_write_gets_the_extraction_budget_however_small fail; restored, 585 pass again.

Lint and gates:

uv run ruff check <4 files>            All checks passed
uv run ruff format --check <4 files>   4 files already formatted
uv run python scripts/check_source_language.py github/main..HEAD   exit 0
make check-large-files                 exit 0

End to end, against a real EverOS 1.2.3 with real LLM and embedding credentials, in an isolated RAVEN_HOME with an empty EverOS root on port 18893 (the normal install untouched). The source is a real Claude Code project memory directory, 36 files / 91 KB / 249 messages, driven through raven.cli.import_commands._build_and_run, the same path raven import run takes:

  • main (before): the first append (100 messages) timed out at the client after 10s; store returned False, the source was marked failed, and the server kept extracting for another three minutes.

  • this branch (after): the same source completed, submitted=1 failed=0, in 348.6s. Five bulk appends of 50 messages took 41s, 58s, 43s, 97s and 58s, the final flush 47s; the server logged 11 episodes, 9 atomic-fact batches and 5 user-profile updates for it. One episode_extract_retry (EverOS failing to parse its own model's JSON) appeared and was absorbed by the budget; that retry is EverOS-side and not touched here.

  • Relevant tests pass locally

  • Relevant lint / type checks pass locally

  • User-facing docs or screenshots are updated when needed (no user-facing copy changes; the CLI tier labels are a named follow-up)

Risk

User-visible change: cold-start imports that used to fail on every real source now complete; each import write may hold the EverOS client for up to the extraction budget (360s) instead of the per-message estimate, which only affects raven import and the web wizard's sync step, never a chat turn. Batch boundaries feed EverOS message ids, so content imported under the old limit is not deduplicated against a re-import; no install had completed an import at either limit, so there is nothing to migrate.

Rollback: revert the commit. The bulk key is additive and ignored by every other backend.

  • Security impact considered (no new inputs, credentials or endpoints)
  • Backward compatibility considered (see the message-id note above)
  • Rollback path is clear for risky changes

Related Issues

N/A

gloryfromca and others added 2 commits September 20, 2026 22:42
…mport batch

Every real cold-start import source failed against a live EverOS: the
importer's non-final appends ran under the per-turn store budget (10s plus
0.5s per message on main), while EverOS extracts on the add itself. Measured
against a real service, a 15-message batch took 12s, a 52-message batch 24s,
and a batch of 100 ran past the six-minute extraction budget. Of 18
memory-file sources on a real machine, 14 failed at the client while the
server kept writing, so a retry would have duplicated what had landed.

Two changes, one per layer:

- The importer marks its appends with metadata["bulk"]; the EverOS backend
  gives a bulk write the extraction budget outright instead of the
  per-message estimate. Nothing waits on an import write, so the turn
  budget's reason does not apply to it. Chat turns carry no flag and keep
  the short budget; the hosted backends (Mem0, Zep, MemOS) do not read the
  incoming metadata, so the key is invisible to them.
- _BATCH_MSG_LIMIT drops from 100 to 50, the top of the range where EverOS
  extraction cost is still linear in the message count. A pin test records
  the measurement so the ceiling is not raised back casually.

Batch boundaries feed EverOS message ids, so content imported under the old
limit is not deduplicated against a re-import; no install had completed an
import at either limit.

Co-authored-by: Claude (claude-fable-5-1) <noreply@anthropic.com>
The measurement behind the batch ceiling now lives in one place, the
orchestrator constant and its pin test; the backend comment only says what
the flag does.

Co-authored-by: Claude (claude-fable-5-1) <noreply@anthropic.com>

@gloryfromca gloryfromca left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Blocking: update the import integration test to match the new 50-message batching contract.

I reviewed the full diff, the MemoryBackend metadata contract, importer and EverOS call paths, affected callers and integration coverage, both branch commits and surrounding history, backward compatibility, test changes, and the repository rules in AGENTS.md plus CONTEXT-MAP.md/CONTEXT.md. The production behavior is internally consistent: ordinary turn writes retain the short size-based timeout, import writes get the extraction timeout, final-flush semantics remain unchanged, and unknown metadata remains optional for other backends. I found one blocking test-suite regression inline.

Verification:

  • uv run --extra dev pytest tests/test_everos_backend.py -x: 171 passed.
  • uv run --extra dev pytest tests/test_importer_orchestrator.py tests/integration/test_import_e2e.py: 28 passed, 1 failed (test_batching_large_conversation).
  • git diff --check github/main...HEAD: passed.

No tests were weakened to conceal behavior, and I found no dependency, asset, source-language, test-naming, or architecture-boundary violation beyond the stale integration expectation called out inline.

Comment thread raven/importer/orchestrator.py
…ssage contract

The integration test still asserted two batches of 100 and 60 for a
160-message conversation; under the new ceiling the run makes four calls of
50, 50, 50 and 10. The total-message and final-batch coverage stays, and the
test now also checks that every import write carries the bulk flag.

Co-authored-by: Claude (claude-fable-5-1) <noreply@anthropic.com>
@gloryfromca

Copy link
Copy Markdown
Member Author

Addressed the one blocker in 7bde3b5 (integration batching expectation moved to the 50-message contract). Verification on the branch head: the two files the review reproduced with plus the four unit files that exercise the importer and the EverOS backend all pass locally; ruff, commitlint, check_commit_messages and check-source-language pass on github/main..HEAD. No production code changed in this commit.

@gloryfromca gloryfromca left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

No blockers; this can merge as far as I am concerned.

The new commit updates only tests/integration/test_import_e2e.py and directly fixes the prior failure without weakening coverage: it asserts the exact 50, 50, 50, 10 batch sequence, the final flag only on the last batch, the bulk flag on every batch, and the unchanged 160-message total. I rechecked the full diff and the earlier review areas: repository rules and context vocabulary, importer and EverOS callers, commit history, compatibility of optional metadata with other backends, architecture boundaries, and test integrity. No new issue emerged.

Verification: uv run pytest tests/test_importer_orchestrator.py tests/integration/test_import_e2e.py tests/test_everos_backend.py -q -> 200 passed in 9.22s; git diff --check github/main...HEAD passed. The blocker thread was replied to and resolved.

@0xKT

0xKT commented Sep 20, 2026

Copy link
Copy Markdown
Member

Not a blocker. Nothing here holds the PR; two of the three points are about
sentences the code now contradicts, and the third is a note for whoever touches
this next. Verified by reading this head (7bde3b5) against base 0cb8b79.

1. bulk makes raven import stop up to 6x slower to be honoured, and the comment above it says the opposite

backend.py:1246 reads "A bulk write is neither a turn nor a flush: nothing waits
on it". Something does. run_import reads the cancel file at
orchestrator.py:83 (top of the per-source loop) and at :169 (after it), and
nowhere else -- _feed_session (:198) and _flush (:205) never look at it. So
raven import stop, which only touches that file (import_commands.py:543-547),
cannot be honoured until the source currently being fed has finished every one
of its batches.

What one wedged non-final store can hold for:

base  limit 100, no bulk : _store_budget(100) = 10.0 + 0.5*100 =  60.0s
head  limit  50, bulk    : _MEMORIZE_TIMEOUT_S               = 360.0s

and for a whole wedged source, with every batch burning its full budget:

messages    base     head    factor
    100     360s     720s     2.0x
    250     480s    1800s     3.8x
    500     600s    3600s     6.0x
   1000     900s    7200s     8.0x

Ctrl-C on a foreground raven import run is unaffected, so there is always a
way out; it is import stop -- the documented cancel, and the only one
available from another terminal -- that degrades. That is why this is not a
blocker rather than one.

2. The two halves of the fix overlap, and the redundant half is the one carrying that cost

On the comment's own measurements, the per-message estimate was not failing the
sizes it cites: 15 messages cost 12s against _store_budget(15) = 17.5s, and 52
cost 24s against _store_budget(52) = 36.0s -- 1.46x and 1.50x headroom, the
same headroom at both points. Only n=100 failed.

Halving alone already resolves that: the non-final batch becomes 50 messages,
where the cited cost is 24s and the unchanged estimate gives 35.0s. Whether
bulk alone would also have been enough depends on which reading of the comment
is right, and neither reading needs it:

  • if a 100-message add really "ran past the six-minute extraction budget"
    (orchestrator.py:21-23), then 360s does not cover n=100 either, and the
    halving is doing the whole job;
  • if instead it expired at the 60s per-message estimate -- which is what the
    base text at backend.py:147-151 describes, "read as a dead service and failed
    every source behind it" -- then either half alone would have fixed it.

If bulk is there for variance headroom rather than for these numbers, the
comment is the place to say so. Otherwise: a smaller non-final bulk budget, or a
cancel-file check between batches in _feed_session, keeps stop responsive
without giving up anything the fix is for.

3. A constant the same file calls part of the message id moved, and nothing in the PR records that

orchestrator.py:16-19 is unchanged by this PR and says the batch boundary is
part of EverOS's message_id: "two messages sharing a millisecond collide, and
one is dropped, if they land at the same index in different batches." The new
comment and the new tests discuss only latency, and test_import_e2e.py now pins
[50, 50, 50, 10] without asserting anything about identity.

This is not a regression claim, and I am not filing it as one -- the direction
is not one-way. The flush condition at :221 is len(batch) >= _BATCH_MSG_LIMIT or batch_chars + msg_chars > _BATCH_CHAR_LIMIT, so with the char limit binding
first the effective batch at a limit of 100 is not 100, and halving can remove
collisions as easily as add them. The point is only that a second constant with
an id-shaped consequence moved silently, and the next reader of :16-19 has
nothing pointing at it.

@gloryfromca

Copy link
Copy Markdown
Member Author

No blockers; suggestions only, and they are marked inline.

The cancellation-latency calculation holds: stop_cmd promises that the running item will complete, and the cancel file is checked only between sources, so raising every non-final bulk wait to 360 seconds can materially delay that documented boundary. Foreground Ctrl-C remains a working escape. This is therefore a named follow-up, not a merge blocker; a between-batch cancellation check or an evidence-backed bulk-specific ceiling would address it.

The 15- and 52-message samples show that _store_budget covered those individual observations, but they do not establish the runtime tail, while the reported live run failed 14 of 18 sources. The arithmetic does not prove that bulk is redundant, though it makes a shorter measured ceiling worth considering.

The identity point is already recorded in the original fix commit body: it states that batch boundaries feed EverOS message IDs, re-import under the old limit will not deduplicate, and no install had completed an import at either limit. The source comment also preserves the boundary/identity coupling, so no additional merge condition follows from that point.

Verification on the unchanged head: uv run pytest tests/test_importer_orchestrator.py tests/integration/test_import_e2e.py tests/test_everos_backend.py -q -> 200 passed in 8.88s. The previously opened review thread remains resolved.

@gloryfromca

Copy link
Copy Markdown
Member Author

Closed in favour of #570 at the maintainer's request: the same three commits are folded onto the web import branch so the whole import work lands on refactor/ui_web_architecture in one PR, and nothing goes to main.

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.

2 participants