Add Natural/Boundary retention (v1.1.11) and silent Hermes provider - #1
Conversation
Process* default is now Natural (force_boundary=false, force_write=false) so episode algorithms decide; Durable keeps force turn + force write; Boundary forces the edge only; Ephemeral stays no-store query. Expose retention on C options and Python/JS/Go bindings (v1.1.11). Add augmem.hermes silent memory provider with live cold-start e2e and a macOS flock fix for sqlite-objstore.
There was a problem hiding this comment.
Pull request overview
This PR expands Cortext ingress retention semantics (defaulting Process* calls to Natural with no forced boundary/write), plumbs the new retention policy through the C API and language bindings, bumps the project to v1.1.11, and introduces a new Hermes Agent integration that provides an “invisible” (no tools / no system prompt branding) memory provider with optional live end-to-end verification.
Changes:
- Add
Retention::{Natural, Durable, Boundary, Ephemeral}behavior and update Process* defaults + C API options/bindings to pass retention explicitly. - Introduce
integrations/hermesprovider/plugin + CLI + tests, including optional cold-start live e2e verification. - Patch build/docs/versioning updates, plus a macOS feature-macro fix for sqlite-objstore
flock()visibility.
Reviewed changes
Copilot reviewed 33 out of 35 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| third_party/sqlite-objstore/src/backend_file.c | Adjust feature-test macros to avoid hiding flock() on macOS. |
| tests/cortext.test.cpp | Add retention behavior tests for Natural/Durable/Boundary/Ephemeral. |
| src/operations/accumulator.cpp | Change accumulator tracking to exclude only Ephemeral signals. |
| src/cortext.cpp | Update force-boundary logic and default retention for cached ProcessText path. |
| src/capi.cpp | Add retention parsing from cortext_process_json_options and thread through JSON APIs. |
| README.md | Document v1.1.11 retention policies and updated Process* defaults. |
| integrations/hermes/uv.lock | Add Hermes integration lockfile for reproducible Python env. |
| integrations/hermes/tests/test_provider.py | Unit tests for Hermes provider behavior (invisibility, seams, provenance, media/tool ingest). |
| integrations/hermes/tests/test_live_e2e.py | Optional live-model cold-start e2e test (skipped by default). |
| integrations/hermes/tests/test_cli.py | Basic CLI tests for install/status helpers. |
| integrations/hermes/src/augmem/hermes/provider.py | Implement silent Hermes MemoryProvider with 3 seam hooks + durable ingest and ephemeral prefetch. |
| integrations/hermes/src/augmem/hermes/plugin/plugin.yaml | Define Hermes plugin metadata and hook registration. |
| integrations/hermes/src/augmem/hermes/plugin/init.py | Plugin package entry exporting provider/register. |
| integrations/hermes/src/augmem/hermes/media.py | Multimodal extraction + decoding and engine dispatch helpers. |
| integrations/hermes/src/augmem/hermes/live_e2e.py | Live e2e harness exercising cold-start recall + control/treatment checks. |
| integrations/hermes/src/augmem/hermes/cli.py | augmem-hermes CLI: install/status/live-e2e. |
| integrations/hermes/src/augmem/hermes/init.py | Hermes integration package exports + version. |
| integrations/hermes/README.md | Hermes integration documentation (seams, provenance, config, live e2e). |
| integrations/hermes/pyproject.toml | Hermes integration packaging, deps, scripts, pytest config. |
| integrations/hermes/.gitignore | Ignore venv/test/build artifacts for Hermes integration. |
| include/cortext/signal.hpp | Change default Signal.retention to Natural and update comment. |
| include/cortext/retention.hpp | Expand/describe retention policies and add Natural/Boundary. |
| include/cortext/internal/replay_ingress.hpp | Update replay ingress default retention to Natural. |
| include/cortext/cortext.hpp | Update public API docs + default retention arguments to Natural. |
| include/cortext/capi.h | Add cortext_retention enum and cortext_process_json_options.retention. |
| docs/paper/sections/9_experimental.qmd | Update experiment log narrative for retention expansion. |
| docs/paper/sections/8_interrupt_gate.qmd | Update algorithm/pseudocode notes for retention policies. |
| CMakeLists.txt | Bump project version to 1.1.11. |
| bindings/python/README.md | Document new retention behavior and recommended usage. |
| bindings/python/pyproject.toml | Bump Python binding version to 1.1.11. |
| bindings/python/augmem/cortext/init.py | Add Python Retention enum and thread retention into process options. |
| bindings/javascript/src/addon.cpp | Parse retention from JS options and pass to native C API. |
| bindings/javascript/package.json | Bump JS package version to 1.1.11. |
| bindings/javascript/index.d.ts | Add Retention type and ProcessOptions.retention. |
| bindings/go/cortext.go | Add Go Retention type and thread it into native process options. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Pin Retention enum values, rename storable-signal tracking, harden JS retention parsing, expose boundary_type for force diagnostics, queue Hermes ingest without join stalls, and disable remote media fetch by default.
|
Addressed all Copilot review threads in f3815b5:
Verified: |
ubuntu-aist was failing because download_aist_model.py no longer accepts --models-dir after the asset packaging cleanup.
Call sites that assert storage now pass Retention::Durable (or C API CORTEXT_RETENTION_DURABLE) instead of relying on the old Durable default.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: be84033139
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 66fe28f433
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 64b057de05
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
@codex Please review the latest fixes. |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
@codex Please review the CI fix in the latest commit. |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
gabewillen
left a comment
gabewillen
left a comment
There was a problem hiding this comment.
Summary
This PR introduces a four-value retention model (Natural/Durable/Boundary/Ephemeral), folds the old force_boundary/force_write flags into retention helpers, defers objstore payload puts until write acceptance, and adds a silent Hermes Agent memory provider. The core policy mapping looks intentional and is mostly well-covered for the happy paths (Durable force write, Ephemeral no-store, ABI-safe options expansion with reserved). Dominant risks are the public default flip from Durable→Natural (incomplete call-site migration and C simple-API surface), and flush-on-boundary interactions that can discard pending Natural units when Ephemeral/Boundary share a source_id.
Issue counts by severity
- bugs: 2
- suggestions: 5
- nits: 3
Issues outside the diff
These findings reference lines that are not present in the diff and could not be posted as inline comments:
- [nit] docs/paper/sections/9_experimental.qmd:138 -- The v1.1.11 retention expansion note still points smoke-eval output paths named
release_1_1_10_smoke_..., which is confusing for release traceability.- Suggestion: Rename the logged run directory to a 1.1.11-tagged path (or note that the smoke reuses the 1.1.10 artifact id).
|
@codex Please review the latest retention hardening fixes. |
gabewillen
left a comment
gabewillen
left a comment
There was a problem hiding this comment.
Summary
Prior review items are largely addressed in head 0009fbfa: Natural cutover is intentional and documented; classic C process APIs hard-force Durable; Ephemeral no longer joins or resets open same-source units; Hermes fake engine, retention validation, docs, and prefetch worker reuse look fixed. One material correctness gap remains: same-source Ephemeral isolation stops at UpdateAccumulator / ResetAccumulatorAfterFlush, so later ops still contaminate open Natural accumulator scores (and related drift/pacing state). That residual path can skew later write-gate decisions and persisted signal scores; fix before merge if Natural+same-source recall is an expected production pattern.
Issue counts by severity
- bugs: 1
- suggestions: 1
- nits: 1
Issues outside the diff
These findings reference lines that are not present in the diff and could not be posted as inline comments:
- [bug] src/operations/accumulator_scores.cpp:28 -- Same-source
Ephemeralisolation is incomplete.UpdateAccumulatorcorrectly early-returns andResetAccumulatorAfterFlushcorrectly skips reset, butUpdateAccumulatorScoresstill runs for an open Natural unit (n_signals > 0). It adds the probe composite score intoacc.s_sum, may raises_max/ rewritee_peak, and overwritesacc.signals.back().scorewith the query score. Because write-gateS_windowis derived froms_sum/s_max, a same-source recall/probe can change whether a later Natural/Boundary close commits the pending unit, and can persist a corrupted last-signal score. The regression test only assertsn_signals == 3and does not catch score contamination.- Suggestion: Skip score updates (and ideally all storable-unit mutations) when
signal.retention == Retention::Ephemeral. Extend the same-source regression test to seed knowns_sum/s_max/last signal score and assert they are unchanged after an Ephemeral probe through the full CoreStage path (or at leastUpdateAccumulatorScores+ reset).
- Suggestion: Skip score updates (and ideally all storable-unit mutations) when
- [suggestion] src/operations/drift_accumulation.cpp:61 -- Related residual same-source interference: on Ephemeral probes,
UpdateDriftAccumulationstill mutates the open unit’sdrift_accum/prev_x, andCheckStreamingPacingstill mutatesdrift_acc_pacing/x_last_check(and may insert an empty accumulator for a brand-new source). This is weaker than score contamination but still violates the “observe without becoming part of the open Natural unit” intent established for Ephemeral.- Suggestion: Gate these ops (or their accumulator mutations) on non-Ephemeral retention, matching
UpdateAccumulator. Prefer a single early “Ephemeral does not touch source accumulator state” policy so future ops cannot reintroduce the hole.
- Suggestion: Gate these ops (or their accumulator mutations) on non-Ephemeral retention, matching
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0009fbfa67
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
gabewillen
left a comment
gabewillen
left a comment
There was a problem hiding this comment.
Summary
Head d16e141 keeps the core Ephemeral isolation fixes from the prior re-review (UpdateAccumulator early-return, Coherence skip, ResetAccumulatorAfterFlush skip, write-gate no-store, Hermes Durable/Ephemeral split docs). The previously open scores bug is still unfixed: UpdateAccumulatorScores still mutates open same-source Natural unit s_sum/s_max/last-signal score during Ephemeral probes, and that still feeds write-gate S_window and stored memory metrics. Drift/pacing open-unit mutation is also still present (UpdateDriftAccumulation, CheckStreamingPacing). Not merge-ready for same-source Natural+Ephemeral isolation until the scores path (at minimum) is gated and covered.
Issue counts by severity
- bugs: 1
- suggestions: 1
- nits: 0
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d16e141063
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
gabewillen
left a comment
gabewillen
left a comment
There was a problem hiding this comment.
Summary
Prior critical findings are fixed on head f0f2709: UpdateAccumulatorScores, UpdateDriftAccumulation, and CheckStreamingPacing all early-return for Retention::Ephemeral, and the same-source isolation test now asserts score fields (s_sum/s_max/last signal score) plus drift/pacing state, not only n_signals. CoreStage open-unit mutators (accumulate, scores, coherence, boundary explicit path, write gate, reset-after-flush) look isolated. One residual same-source hole remains in the interrupt gate, which can still advance open-unit refractory state when a probe allows an interrupt. Not a merge blocker relative to the fixed write-score pollution path, but worth closing for complete Ephemeral isolation.
Issue counts by severity
- bugs: 0
- suggestions: 1
- nits: 0
gabewillen
left a comment
gabewillen
left a comment
There was a problem hiding this comment.
Summary
The critical residual from head e5b1f7cb is fixed: ComputeMniGateDecision no longer inserts a source accumulator for Ephemeral, uses a read-only fallback when no entry exists, and does not advance drift_at_last_interrupt or last_interrupt_tick when an interrupt is allowed. Prior CoreStage isolation (accumulate/scores/drift/pacing/reset/coherence) remains intact, and regression tests cover scores + drift/pacing + interrupt refractory. One remaining same-source lifecycle hole: forced Ephemeral boundary score still drives capacity finalize through CheckSpikeBypass without write or accumulator reset. Merge is close; that cascade should be gated before treating isolation as complete.
Issue counts by severity
- bugs: 1
- suggestions: 0
- nits: 0
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 69efa571f9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Add Natural/Boundary retention (v1.1.11) and silent Hermes provider
Summary
Retentionto Natural (default), Durable, Boundary, Ephemeralforce_boundary=false,force_write=false— episode algorithms decidecortext_process_json_optionsand Python / JS / Go bindingsaugmem.hermesmemory provider (no tools / no system-prompt branding) with cold-start live e2eflockfix for sqlite-objstore under_POSIX_C_SOURCETest plan
./build/debug-test/tests/cortext_tests "[retention]"— 3 cases / 45 assertionscd integrations/hermes && uv run pytest— 16 passed, 1 skippedCORTEXT_HERMES_LIVE=1 OPENAI_API_KEY=... OPENAI_MODEL=gpt-5.4-mini uv run augmem-hermes live-e2e --forceBreaking note
Process* default retention is now Natural (not Durable force-turn). Callers that need the old “every message is a committed turn” behavior must pass
Retention::Durableexplicitly (CLI remember / Hermes stores already do).