Skip to content

Develop - #37

Merged
Bosheng0422 merged 33 commits into
mainfrom
develop
Nov 4, 2026
Merged

Bosheng0422 merged 33 commits into
mainfrom
develop

Conversation

@Bosheng0422

Copy link
Copy Markdown
Collaborator

Summary

Clarification and acceptance

Target branch

  • Base is develop (feature / fix — default)
  • Base is main (release/* or hotfix/* only)

Type of change

  • Bug fix
  • New feature
  • Breaking change
  • Documentation
  • Refactor / chore
  • Release / hotfix

Test plan

  • make all passes locally
  • Added/updated tests

Checklist

  • Updated CHANGELOG.md (if user-facing)
  • README / docs / HANDOFF updated (if needed)

fengyue-xve and others added 30 commits October 10, 2026 10:52
The Hermes adapter's _bridge_config() only forwarded profile, mode,
recall, capture and privacy, so an llm/extraction block in
octopmemory.json never reached MemoryRuntimeConfig: the runtime always
fell back to NoopLLMClient and model-assisted extraction was silently
unavailable (no candidate/atom, no error). Forward both blocks with the
same semantics as the bridge's own _parse_bridge_config.
Both previously documented routes were unusable: `octop-memory-hermes`
is not published to PyPI, and no standalone plugin repository exists.
Document the verified `owner/repo/path/to/plugin` subdirectory install
(Hermes rejects short SHAs, so the ref is a full 40-character SHA) and
`hermes --print-runtime-command` as the way to find the gateway's real
interpreter when dependencies fail to import.
CandidateExtractor.extract() serialized the entire session event list into a
single prompt, and the "already extracted" cursor lived only in an in-memory
LRU. Together they made a long session's backlog unbounded: the first call
overflowed the context ("length limit reached" / timeout), and every restart
reset the cursor so the whole backlog was re-fed.

1. Batch events by count (100) and estimated prompt size (20,000 chars),
   mirroring operations/retention.py, and merge per-batch results. A single
   event larger than the budget still gets its own batch; a failed batch stops
   the loop and surfaces failure_reason so events are retried.
2. Persist each session's extracted-id set in {ns}_meta and restore it on a
   cache miss, so a restart resumes incrementally. Persistence is best-effort:
   a write failure logs and keeps the in-memory cursor moving.

Adds regression tests for count/char splitting, cross-batch accumulation,
fail-fast, and cursor survival across a simulated restart.

Closes #25
Saving a fact compared the atom backlog with USER.md and MEMORY.md
before the tool could return, so one remember request blocked on
several model calls and retried the same batch after a bad response.
Store the fact immediately and let the host drain one bounded batch
in the background.

Co-authored-by: Cursor <cursoragent@cursor.com>
fix(memory): keep document review off the save path
 review)

P1 reproduced by @Bosheng0422: the cursor was persisted before candidates were
written, so a failed candidate INSERT still advanced the cursor -- those events
were skipped permanently and a plain retry could not recover them.

Now both writes share one backend transaction, the cursor write raises inside it
(strict=True instead of being swallowed), and the in-memory seen set advances
only after the commit.
…ared

The previous version recorded the hash before the match check, so three
different cases all counted as audited: (1) uncertain — intended, (2) no
match at all because the LLM failed or the batch broke — this would mark
facts as audited after a single transient failure and persist it in
`checked`, (3) stale/deprecated atoms that still need review. Split them:
advance only when a comparison actually happened.
fix(memory): expose background manual document review API
fix(hermes): forward llm/extraction config blocks into the runtime
A comparison that returns confidence="uncertain" still advances the audit
cursor -- that part is right, otherwise the same atoms are re-sent to the
model on every pass. What was missing is that the state itself has to
survive: the next pass has nothing left to compare, so it reported
"complete" for atoms that were never verified.

Record unresolved atoms against their assertion hash, report them as
`uncertain` in the audit result, and keep the persisted status from
flipping back to "complete" while any of them is still current. A new
document revision re-compares the atoms and can clear the state.

Verified on SQLite (tests/test_manual_documents.py, 27 passed); the
PostgreSQL half of the parity suite is skipped locally (no dedicated
server configured).
fix(host-files): reject rooted Windows include patterns
docs(hermes): document the working subdirectory install path
fix(page): reject stale regeneration writebacks
A comparison that returns confidence="uncertain" is recorded per atom id so that
a later pass -- which has nothing left to compare -- cannot report "complete" for
a fact nobody verified. The stored ids were only checked against the historical
`checked` map, so an atom the user disabled (or deleted, or reworded) stayed in
the set forever: every later pass reported status="uncertain" with zero live
facts pending, and only editing the documents could clear it.

Both paths now intersect the stored set with the current live atoms and their
assertion hashes -- the reconcile write path and manual_document_review_status.
`remaining` is computed over live atoms as well, and a persisted "uncertain"
downgrades to partial/complete once nothing unresolved is left. Model failures
and unverified comparisons still hold the audit, and a document edit that leaves
the facts untouched still leaves them audited.

Verified locally: tests/test_manual_documents.py 32 passed / 1 skipped (SQLite;
the PostgreSQL half skips without TEST_POSTGRES_DSN, so it is unverified here).
Full suite and strict mypy keep the same failure sets as the unpatched tree.
The incremental extraction path commits candidate writes and the cursor write
in ONE transaction, so a failed candidate INSERT or a failed (strict) cursor
write rolls back the whole batch -- the events stay un-extracted and a later
`extract` call retries them instead of being silently skipped. These guarantees
were verified by hand on temp SQLite / PostgreSQL 16 during review but never
entered the suite; this adds them as real dual-backend tests.

tests/test_extract_cursor_failure_retry.py covers:
- 1st / 2nd candidate INSERT failure -> batch rolls back, same-process retry
  recovers and extracts each event exactly once (no duplicate candidate);
- strict cursor-write failure rolls back the candidate writes too (#25 P1) --
  the events are NOT silently marked extracted;
- normal commit then reopen: the persisted cursor resumes, no re-extraction;
- failed extract then reopen: the events are retried, not skipped forever.

Also: ruff-format `_persist_seen_ids` (strict kwarg on its own line) and correct
CHANGELOG / docs/agent/HANDOFF wording -- the incremental strict cursor write
now rolls back the transaction (candidates included), so the old "persistence
failure only warns, never affects extraction" line no longer matches the code.

Verified locally: the new file passes on SQLite (PostgreSQL half skips without
TEST_POSTGRES_DSN, unverified here); the 14+ assertions mirror the reviewer's
hand-run set. Same-process retry paths are exercised on both backends; reopen
paths are SQLite-only (the persistence mechanism is identical).
@Bosheng0422
Bosheng0422 merged commit 790cb79 into main Nov 4, 2026
5 checks passed
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.

5 participants