feat(frontend): implement baseline memories toggle and UI badges in mobile app#8728
Conversation
- Added `is_baseline` field to `MemoryDB` model to support memory pinning.
- Added `PATCH /v3/memories/{memory_id}/baseline` endpoint for toggling status.
- Updated memory injection logic to prioritize baseline memories in prompt context.
- Added and updated unit tests for baseline memory functionality.
- Add `isBaseline` property to `Memory` schema, including JSON parsing/serialization
- Add `updateMemoryBaselineServer` API call wrapper to toggle baseline status via PATCH /v3/memories/{id}/baseline
- Implement `toggleMemoryBaseline` method in `MemoriesProvider` to handle state updates
- Display a blue flag icon next to baseline memories in the memories list view (`MemoryItem`)
- Add a "Baseline Memory" badge and a togglable flag action inside the `MemoryEditSheet`
- Integrate localization tooltips for pinning/unpinning baseline memories
|
Thanks for putting this together — the baseline-memory idea is relevant to the memories surface, but I would keep this as draft / human-maintainer review before merge. A few things need attention before this can be considered:
No formal approval from me while this is draft, conflicting, and product-sensitive, but the direction is worth a maintainer look once it is rebased and covered with behavioral tests. |
…mories Resolves all 5 merge conflicts from syncing with upstream main: - backend/models/memories.py: keep is_baseline alongside new evidence/memory_tier/layer - backend/utils/llms/memory.py: merge canonical path support with baseline bucket logic - app/lib/backend/schema/memory.dart: keep isBaseline + new layer/captureDeviceIds fields - app/lib/pages/memories/widgets/memory_edit_sheet.dart: baseline UI + withValues(alpha:) - app/lib/pages/memories/widgets/memory_item.dart: use FaIconData icon (typed, from main) Addresses PR reviewer feedback (BasedHardware#8728): - router: replace _validate_memory with _validate_mutable_memory in baseline endpoint so canonical-path users are validated against the canonical store, not the legacy store - tests: replace regex/source-level checks with 9 behavioral tests that exercise actual MemoryDB instantiation, dict serialization, and get_prompt_data/get_prompt_memories logic via mocked legacy DB path (no Firebase required) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
kodjima33
left a comment
There was a problem hiding this comment.
frontend baseline-memories toggle + badges — approve-only (feature; needs rebase)
…resh Replace import-inside-test-body pattern with the project's sanctioned testing/import_isolation.py primitives so the tests run hermetically without installing stripe, anthropic, or other heavy transitive deps. - Add session-scoped mem_module fixture that stubs database.memories, database.auth, and utils.memory.memory_service (the two import chains that pull in stripe and anthropic) before loading utils.llms.memory fresh - Switch TestBaselineMemoryInjection tests to receive mem_module as a fixture and patch only resolve_memory_system, memories_db.get_memories, get_user_name - Replace .dict() calls with .model_dump() (removes PydanticDeprecatedSince20 warnings) All 12 tests pass: 5 model field tests + 7 prompt injection tests. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Thanks for the approval! The branch has been rebased and all merge conflicts resolved — it's now up to date with main.
@Git-on-my-level, Thank you for the detailed review - I've addressed each point: Rebase / conflicts - Branch is now rebased and all 5 conflicts resolved (models, router, llm utils, Flutter schema, Flutter widgets). Canonical memory path - The baseline endpoint now uses _validate_mutable_memory instead of _validate_memory, so canonical-path users are validated against the canonical store. get_prompt_data() also handles MemorySystem.CANONICAL via MemoryService.read() and the legacy path separately. Behavioral tests — Replaced all regex/source-level checks with 9 hermetic behavioral tests: MemoryDB model instantiation, dict() roundtrip, get_prompt_data() bucket routing (baseline / user_made / generated / locked), and get_prompt_memories() prompt-string formatting — all via mocked DB, no Firebase required. Downstream invalidation — Noted as a follow-up. The visibility endpoint triggers submit_with_context(postprocess_executor, update_personas_async, uid) after changing prompt-affecting metadata; the same hook could be added to the baseline toggle. Happy to add that if maintainers want it in this PR. Product/privacy sensitivity — Agreed this needs a maintainer call on UX scope. Keeping as draft until there's sign-off on the intended behavior. |
@kodjima33 thank you for the approval! The branch has been rebased and all merge conflicts resolved - it's now up to date with main. |
|
Thank you for this well-structured PR! The end-to-end implementation of baseline memories — schema, API, prompt injection, and mobile UI — is cohesive and nicely tested with the new behavioral test suite. A few items to address before this can move forward: 1. Breaking change to
2. Needs rebase The PR currently has merge conflicts with 3. Canonical-path consistency (worth verifying) The PATCH endpoint validates via 4. Prompt wording change The base prompt wording for regular memories changed from The new tests are thorough — bucket routing, precedence, prompt-label presence/absence, and lock exclusion all covered. This feature needs human product/architecture review for the baseline-memories concept and its interaction with the canonical memory system before merge. |
- fix test_lock_bypass_fixes.py:1356: update 3-tuple unpack of get_prompt_data() to 4-tuple (_, baseline, user_made, generated) and include baseline in all_mems; complete fixture dicts with required MemoryDB fields (uid, created_at, updated_at); mock resolve_memory_system to LEGACY for hermetic isolation - fix utils/llms/memory.py: restore "you already know" wording for non-baseline memories — "you also know" was an unintentional regression from the original prompt template Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
@Git-on-my-level , thanks again for the detailed review.
|
|
Thanks for the updates — the test fix, rebase, and prompt-wording restoration all look good. The new behavioral test suite is well-structured. Two items still need attention before this can move forward: 1. Canonical-path write inconsistency (blocking for canonical users) Your reasoning in the previous comment — that
The toggle would appear to succeed ( Compare with the visibility endpoint, which branches on 2. Missing downstream refresh / persona postprocessing Every other memory mutation endpoint in this router calls 3. CI No check runs have been recorded for this head SHA yet — worth confirming CI picks it up after the rebase. The feature direction (persistent baseline memories) is a coherent product concept and the end-to-end implementation is well-organized. This needs human product/architecture review for the baseline-memories concept and the canonical-path interaction before merge. Automated maintainer review — not a formal approval. Human maintainer review required before merge. by AI on behalf of David — if you need David’s attention urgently, please @Git-on-my-level and escalate with |
…nse model
baseline endpoint (PATCH /v3/memories/{memory_id}/baseline):
- add _canonical_write_enabled_or_fail_closed gate: canonical users now
receive an explicit 503 instead of silently writing to the legacy store
that the canonical read path (MemoryService) never consults
- add submit_with_context(postprocess_executor, update_personas_async, uid)
after the legacy write, matching the visibility endpoint pattern;
changing is_baseline affects prompt injection so persona context should refresh
- add response_model=MemoryMutationResponse on the decorator, consistent
with all other mutation endpoints (delete, review, edit, visibility)
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Thanks for the updates — all three items from the previous review are resolved on this head:
One minor observation worth awareness (not blocking): CI has not recorded any check runs for this head SHA yet (status is pending). Worth confirming CI picks it up. The implementation is cohesive and well-tested — bucket routing, precedence, prompt-label presence/absence, and lock exclusion are all covered. The persistent-baseline-memories concept is a coherent product direction. This still needs human product/architecture review for the feature concept and its interaction with the canonical memory system before merge. Automated maintainer review — not a formal approval. Human maintainer review required before merge. by AI on behalf of David — if you need David’s attention urgently, please @Git-on-my-level and escalate with |
kodjima33
left a comment
There was a problem hiding this comment.
Re-approve on new sha: frontend feature (baseline memories toggle + UI badges) — web area, approve only
|
Thanks for the thorough review - glad all three items landed well. On the safe_create_memory observation: noted. The MemoryDB construction requirement (id, uid, created_at, updated_at) is stricter than the legacy dict path, but those fields are always present in Firestore-stored memories. The try/except silently skips any malformed document rather than crashing the prompt build, which is the safer failure mode for a read path. On CI: the check runs haven't appeared yet — this is likely because the workflows need a maintainer to approve the run for fork-based PRs. Happy to re-push a no-op commit if that helps trigger it, otherwise waiting on workflow approval. |
Thanks for the re-approval! |
…e toggle The memories baseline toggle (BasedHardware#8728) merged without regenerating the artifacts derived from the app-client OpenAPI contract, so several committed files drifted from source: - backend specs: app-client (+PATCH /v3/memories/{memory_id}/baseline), integration-public (+is_baseline field) - app-client client types: Dart memories_wire, Swift OmiApi.generated, TS omiApi.generated (web app / admin / personas, windows renderer) All add the same is_baseline field / baseline route; method counts and every other model are unchanged. The openapi-contract and generated-type contract checks have been red on main and on every PR based off it, and prod-eligibility inherits the failure. Regenerated with the repo's own generators (no hand edits): export_openapi.py --surface {app-client,integration-public} --write ... generate_dart_models.py --all generate_swift_openapi_types.py generate_ts_openapi_types.py Verified all three OpenAPI surfaces report "up to date" and re-running every generator reproduces the committed bytes, via the .openapi-venv (Python 3.11.15). Stacked on BasedHardware#10481, which exempts Sources/Generated/ from the desktop changelog gate so the regenerated Swift needs no changelog fragment. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…e toggle The memories baseline toggle (BasedHardware#8728) merged without regenerating the artifacts derived from the app-client OpenAPI contract, so several committed files drifted from source: - backend specs: app-client (+PATCH /v3/memories/{memory_id}/baseline), integration-public (+is_baseline field) - app-client client types: Dart memories_wire, Swift OmiApi.generated, TS omiApi.generated (web app / admin / personas, windows renderer) All add the same is_baseline field / baseline route; method counts and every other model are unchanged. The openapi-contract and generated-type contract checks have been red on main and on every PR based off it, and prod-eligibility inherits the failure. Regenerated with the repo's own generators (no hand edits): export_openapi.py --surface {app-client,integration-public} --write ... generate_dart_models.py --all generate_swift_openapi_types.py generate_ts_openapi_types.py Verified all three OpenAPI surfaces report "up to date" and re-running every generator reproduces the committed bytes, via the .openapi-venv (Python 3.11.15). Stacked on BasedHardware#10481, which exempts Sources/Generated/ from the desktop changelog gate so the regenerated Swift needs no changelog fragment. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
## What `check-desktop-changelog.py` exempts `Backend-Rust/` and `tests/` from the "desktop change requires a changelog fragment" rule as *never user-facing app notes* — but it does **not** exempt `desktop/macos/Desktop/Sources/Generated/`. So regenerating `Sources/Generated/OmiApi.generated.swift` after a backend OpenAPI spec change demands a changelog fragment. Worse, exactly as the `tests/` exemption comment already documents (incident #10387): the **post-merge push run** of this gate would redden `main`, because the PR-only `no-changelog-needed` label can't be honored on the push lane. ## Fix Add `desktop/macos/Desktop/Sources/Generated/` to `EXEMPT_DESKTOP_PATH_PREFIXES`. Generated Swift is deterministically derived from the OpenAPI contract — the same directory the swift-format linter already skips. A test asserts the generated file is exempt while a hand-written `Sources/AppDelegate.swift` still requires a changelog (the exemption must not leak). ## Why now The memories baseline toggle (#8728) changed the app-client OpenAPI contract; regenerating `OmiApi.generated.swift` from it is blocked today purely by this gate gap. This is the **second** instance of the push-lane gate over-including internal paths (first: #10387, `tests/`), so it registers `FC-push-gate-internal-path-scope`. ## Verification ``` python3 .github/scripts/test_desktop_changelog.py → Ran 4 tests OK # self-check: is_desktop_change_requiring_changelog(...) # Sources/Generated/OmiApi.generated.swift → False (exempt) # Sources/AppDelegate.swift → True (still gated) scripts/failure-class validate ... → ok ``` Failure-Class: new <!-- This is an auto-generated description by cubic. --> <a href="https://cubic.dev/pr/BasedHardware/omi/pull/10481?utm_source=github" target="_blank" rel="noopener noreferrer" data-no-image-dialog="true"><picture><source media="(prefers-color-scheme: dark)" srcset="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"><source media="(prefers-color-scheme: light)" srcset="https://www.cubic.dev/buttons/review-in-cubic-light.svg"><img alt="Review in cubic" src="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"></picture></a> <!-- End of auto-generated description by cubic. -->
…e toggle The memories baseline toggle (BasedHardware#8728) merged without regenerating the artifacts derived from the app-client OpenAPI contract, so several committed files drifted from source: - backend specs: app-client (+PATCH /v3/memories/{memory_id}/baseline), integration-public (+is_baseline field) - app-client client types: Dart memories_wire, Swift OmiApi.generated, TS omiApi.generated (web app / admin / personas, windows renderer) All add the same is_baseline field / baseline route; method counts and every other model are unchanged. The openapi-contract and generated-type contract checks have been red on main and on every PR based off it, and prod-eligibility inherits the failure. Regenerated with the repo's own generators (no hand edits): export_openapi.py --surface {app-client,integration-public} --write ... generate_dart_models.py --all generate_swift_openapi_types.py generate_ts_openapi_types.py Verified all three OpenAPI surfaces report "up to date" and re-running every generator reproduces the committed bytes, via the .openapi-venv (Python 3.11.15). Stacked on BasedHardware#10481, which exempts Sources/Generated/ from the desktop changelog gate so the regenerated Swift needs no changelog fragment. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Description
Implements Persistent Baseline Memories (resolves #4631).
Baseline memories are critical user facts (name, preferences, core background context that are pinned so the AI always has them available — without the user needing to re-prompt every session. Users can flag any memory as "Baseline" directly from the mobile app, which shows the status in the list view and allows toggling from the edit sheet.
Key Changes
Backend (
backend/)Schema (
backend/models/memories.py)is_baseline: bool = FalsetoMemoryDB— backward-compatible, existing memories default toFalseToggle Endpoint (
backend/routers/memories.py)PATCH /v3/memories/{memory_id}/baseline— secured undermemories:modifyrate limit_validate_mutable_memory(not the simpler_validate_memory) so canonical-path users are validated against the canonicalstore, not the legacy Firestore path
LLM Context Injection (
backend/utils/llms/memory.py)get_prompt_datanow returns a 4-tuple(user_name, baseline, user_made, generated)and handles bothMemorySystem.CANONICAL(via
MemoryService.read()) and the legacy pathget_prompt_memoriesprepends baseline memories first with a distinct label ("baseline facts … always in context") so they arenever dropped regardless of total memory count
Unit Tests (
backend/tests/unit/test_baseline_memories.py)TestBaselineMemoryModel(5 tests): directly instantiatesMemoryDB, assertsis_baselinedefault,Trueassignment,model_dump()output, and dict roundtripTestBaselineMemoryInjection(7 tests): loadsutils.llms.memoryviastub_modules+load_module_fresh(project-standardisolation primitives) and patches
resolve_memory_system,memories_db.get_memories, andget_user_nameto exercise actual bucketrouting and prompt-string formatting
Mobile Client (
app/)Schema (
app/lib/backend/schema/memory.dart)isBaselinefield with serialization (is_baseline↔isBaseline)API (
app/lib/backend/http/api/memories.dart)updateMemoryBaselineServer— sendsPATCH /v3/memories/{id}/baseline?value=true|falseState (
app/lib/providers/memories_provider.dart)toggleMemoryBaseline— calls the API, updates local state, notifies listenersUI
memory_item.dart— blue flag icon in the list row when a memory is baselinememory_edit_sheet.dart— "Baseline Memory" badge next to the category label + flag toggle icon button; useswithValues(alpha:)(not deprecated
withOpacity())app_en.arb—baselineMemory,pinAsBaseline,unpinAsBaselinelocalization keysHow to Test
Backend unit tests (run from
backend/):