feat(signal): wire Signal conversations into OODA context + graph memory (#2527 follow-up)#2625
Open
rysweet wants to merge 4 commits into
Open
feat(signal): wire Signal conversations into OODA context + graph memory (#2527 follow-up)#2625rysweet wants to merge 4 commits into
rysweet wants to merge 4 commits into
Conversation
…ory (#2527 follow-up) Chatting with Simard over Signal now starts with the SAME OODA-loop context as the CLI meeting and is bidirectionally wired into Simard's graph-based cognitive-memory model: live OODA context is recalled into the system prompt at session start, and conversation memory is written back on /close. Previously src/signal_conversation/channel.rs built per-operator backends via MeetingBackend::new_session("signal", agent, None, load_meeting_system_prompt()) — a bare prompt and a None memory bridge — so over Signal Simard had neither OODA-loop context nor memory write-back (closing.rs consolidation is a no-op while the bridge is None). Changes (DRY, mirrors the CLI run_meeting_repl_command): - operator_commands_meeting::meeting_session: add pub(crate) build_enriched_meeting_system_prompt(bridge) = base meeting_system.md prompt + build_live_meeting_context(bridge), propagating bridge errors (PHILOSOPHY.md: no silent degradation). Refactor the CLI REPL to use it. - operator_commands_meeting::mod: re-export the shared helper. - signal_conversation::channel run(): wire each per-operator backend in the make_backend closure — open its OWN cognitive-memory store via memory_ipc::launch_writer_bridge on state_root::simard_state_root() (recall), build the enriched system prompt via the shared helper, and pass Some(memory) into MeetingBackend::new_session (write-back on /close). The closure now returns SimardResult<MeetingBackend> so recall/memory failures propagate; run_continuous's bound is widened to match. The fail-fast provider check and the UnavailableAgent graceful-degradation path are preserved. The binding is named `memory` (not `bridge`) per the signal_conversation naming guardrail, and the now-unused private load_meeting_system_prompt is removed. Adapted to main's per-operator run_continuous/make_backend structure introduced by the Note-to-Self loop guard (#2575) and continuous per-operator conversations (#2577), not the old single-backend run(). Everything stays behind the default-off "signal" cargo feature. Verified: cargo build --features signal; cargo test --features signal signal_conversation (88 passed); cargo test operator_commands_meeting:: (66 passed); cargo clippy --features signal -- -D warnings; default clippy. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…close comment Address the architect's Step 6b documentation review for the Signal OODA / graph-memory wiring (PR #2625, issue #2527 follow-up). The feature code shipped without the doc updates it implies, leaving three artifacts stale/incomplete. Code-doc: - meeting_backend/closing.rs: the "Memory consolidation" comment claimed the step is a no-op because the bridge is "always None". It no longer is — the Signal channel wires a per-operator cognitive-memory store into the backend, so `/close` consolidates back into graph memory. Rewrite to state exactly when it runs (Signal write-back) vs when it is skipped (CLI meeting REPL and dashboard chat pass None; they recall context into the prompt but do not write back here). - signal_conversation/channel.rs: the module doc now describes the OODA / graph-memory wiring, and the "# Naming" section is made precisely truthful — no symbol *defined here* is named bridge/Bridge, but the wiring does call the external `memory_ipc::launch_writer_bridge`, whose handle is bound locally as `memory`. Reference docs: - signal-continuous-conversation.md: add an "Also new (#2527)" note, two concepts rows (OODA context, cognitive-memory store), and note that `/close` consolidates into graph memory. Refine the naming note for the external helper. - signal-conversation.md: refine the naming note, add an OODA/graph-memory paragraph, and note memory consolidation in the meeting-turn data flow. Verified: mkdocs build --strict (0 broken links/anchors), clippy --features signal, and that build_live_meeting_context stays pub(super) (the channel reaches recall only through build_enriched_meeting_system_prompt). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…2527 follow-up) Add the missing regression tests for the Signal↔OODA-context / graph-memory wiring so the new behaviour is guarded (TDD step of the #2527 follow-up, adapted to the per-operator run_continuous/make_backend structure of #2575/#2577): operator_commands_meeting::tests_live_context — pin the shared build_enriched_meeting_system_prompt helper (the single source of truth every operator↔Simard channel uses): - enriched prompt = base meeting_system.md prompt + "\n\n" + live OODA context (blank-line-separated suffix), regardless of base asset presence; - recalled graph-memory facts reach the prompt verbatim; - every OODA section (meetings, decisions, goals, operator, projects, research, improvements) is carried through; - recall failures PROPAGATE out of both build_enriched_meeting_system_prompt and build_live_meeting_context (PHILOSOPHY.md: no silent degradation). Adds an erroring_bridge() test helper to drive the no-degradation cases. signal_conversation::tests_continuity — pin that a per-operator make_backend failure (recall/memory wiring or enriched-prompt build failing) PROPAGATES out of run_continuous on a real turn and leaves no half-persisted conversation, matching the SimardResult<MeetingBackend> closure contract. Verified: cargo build --features signal; cargo test --features signal signal_conversation (89 passed); cargo test operator_commands_meeting:: (71 passed); cargo clippy --features signal --all-targets -- -D warnings. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…access enriched_prompt_appends_live_context_after_base builds the live OODA context twice (once inside build_enriched_meeting_system_prompt, once directly) and compares them as a blank-line-separated suffix. Because build_live_meeting_context resolves the operator name from the process-global SIMARD_OPERATOR_NAME env var, a concurrent env-mutating test landing between the two builds made the two contexts diverge and the suffix check flake (seen on the push-triggered verify run; the pull_request run passed). Join the cognitive_memory serial group and hold ENV_MUTEX while pinning SIMARD_OPERATOR_NAME to a deterministic (unset) state, matching every other env-touching test in this module, so both builds observe identical env and never interleave with a mutator. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
📊 Coverage Summary
Coverage data from CI run. Test files matching |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The gap
Follow-up to the closed issue #2527, which introduced the shared
ConversationChannel/MeetingBackendabstraction. Chatting with Simard over Signal did not start with the same OODA-loop context as the CLI meeting, and was not wired into the graph-based cognitive-memory model.On
main,src/signal_conversation/channel.rsrun()built per-operator backends inside themake_backendclosure via:— a bare system prompt and a
Nonecognitive-memory bridge. So over Signal Simard had:/close—meeting_backend::closingconsolidation is a no-op while the bridge isNone.The CLI reference (
operator_commands_meeting::meeting_session::run_meeting_repl_command) already does the right thing: launches a real bridge, builds live context, enriches the prompt, and passes the bridge into the backend.The fix (DRY — mirrors the CLI)
Shared helper — add
pub(crate) build_enriched_meeting_system_prompt(bridge)inmeeting_session.rs= basemeeting_system.mdprompt +build_live_meeting_context(bridge), propagating bridge errors (PHILOSOPHY.md: no silent degradation). It is now the single source of truth for "what context a Simard conversation starts with".Re-export it from
operator_commands_meeting::mod.Refactor the CLI
run_meeting_repl_commandto use the shared helper (behavior-preserving).Wire the Signal channel — in
channel.rsrun(), each per-operator backend built inmake_backendnow:memory_ipc::launch_writer_bridgeonstate_root::simard_state_root()(shares the running daemon's store over IPC when one is up),Some(memory)intoMeetingBackend::new_session, so/closeconsolidates episodes / summary facts / goal carryover into graph memory.The
make_backendclosure now returnsSimardResult<MeetingBackend>(andrun_continuous's generic bound is widened to match) so recall/memory failures propagate rather than degrade silently. The fail-fast provider check and theUnavailableAgentgraceful-degradation path are preserved. The binding is namedmemory(neverbridge) per thesignal_conversationnaming guardrail, and the now-unused privateload_meeting_system_promptis removed.Adapted to current
mainRe-implemented against
main's per-operatorrun_continuous/make_backendstructure introduced by the Note-to-Self sync-sent loop guard (#2575) and continuous per-operator conversations (#2577) — not the old single-backendrun(). Each per-operator backend gets its own store instance because the store is moved into the backend.Everything stays behind the default-off
signalcargo feature.Verification
cargo build --features signal✅cargo test --features signal signal_conversation✅ (88 passed)cargo test operator_commands_meeting::✅ (66 passed)cargo clippy --features signal -- -D warnings✅cargo clippy -- -D warnings✅cargo clippy --all-targets --all-features --locked -- -D warnings✅Refs #2527, #2575, #2577.