Feat/v1.1 final polish#114
Conversation
![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: bd7700c245
ℹ️ 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".
|
|
||
| client.execute_write( | ||
| """ | ||
| MATCH (source:Universe {id: $source_uid})-[:IN_MULTIVERSE]->(mv:Multiverse) |
There was a problem hiding this comment.
For universes created by the standard neo4j_create_universe path, the graph links (m:Multiverse)-[:CONTAINS]->(u:Universe), not (u)-[:IN_MULTIVERSE]->(mv). In that normal environment this MATCH produces no row, so split creates no target universe and subsequent clone writes match nothing while still reporting cloned counts; the merge query repeats the same edge pattern and fails for the same standard universes.
Useful? React with 👍 / 👎.
| from monitor_data.tools.mongodb_tools import mongodb_create_npc_profile | ||
| from monitor_data.tools.neo4j_tools.entities import neo4j_create_entity | ||
|
|
||
| entity = neo4j_create_entity( |
There was a problem hiding this comment.
This new expansion path writes an Entity directly from the UI backend, bypassing the MCP authority middleware that restricts neo4j_create_entity to CanonKeeper and the repo rule that only CanonKeeper writes to Neo4j. Any first chat/expand of a standalone character now creates canon graph state outside the ProposedChange/CanonKeeper review path, so this should go through CanonKeeper or stage a proposal instead.
Useful? React with 👍 / 👎.
| # the cross-incarnation flag by setting a transient attribute on the | ||
| # loop state (read by NPCVoice via the conversation_loop wrapper). | ||
| if include_cross_incarnation and getattr(loop, "state", None) is not None: | ||
| loop.state.include_cross_incarnation = True # type: ignore[attr-defined] |
There was a problem hiding this comment.
When the UI sends with “Remember across incarnations” enabled, this assignment runs against the real ConversationState Pydantic model from ConversationLoop.start; that model does not define include_cross_incarnation, so assigning the unknown field raises before loop.step() runs and the send endpoint returns an error. Add the flag to ConversationState or pass it through an explicit method argument.
Useful? React with 👍 / 👎.
| if universe_id is not None: | ||
| search_kwargs["universe_id"] = str(universe_id) | ||
| if include_cross_incarnation: | ||
| search_kwargs["include_cross_incarnation"] = True |
There was a problem hiding this comment.
Even if the cross-incarnation send path reaches NPCVoice, _recall_memories still includes universe_id and only adds include_cross_incarnation; the data-layer MemorySearchRequest/qdrant_search_memories does not define or use that flag and continues to filter by the current universe. In the opt-in case this means recall remains scoped to one incarnation, so either omit universe_id when this flag is true or add real data-layer support for it.
Useful? React with 👍 / 👎.
…eak) NPCVoice + ConversationLoop now thread universe_id through recall, memory writes, working state, and staged proposals. Each character card holds a versions[] array; every incarnation has its own entity_id + NPCProfile + memory set + relationship deltas, partitioned by universe. - data-layer: NPCProfile gains universe_id, relationship_states_by_universe, current_emotional_state_by_universe (legacy fields remain as fallback). - agents: NPCVoice._recall_memories/_write_npc_memory/_build_*_snapshot read/write the per-universe partition first; add universe_id + include_cross_incarnation kwargs to respond_direct. ConversationLoop stamps universe_id on staged ProposedChanges. - UI backend: character_storage.add_version/get_version/list_versions/ touch_version/delete_version; character_conversation.ensure_character_backed is per-universe idempotent; new delete_incarnation tears down entity + profile. 5 new endpoints (POST/GET/DELETE /versions + universe_id + include_cross_incarnation flags). - frontend: StandaloneCharacter + ConversationStart carry versions / default_universe_id / version_id / universe_id. Roster shows Nx badge, editor has Incarnations panel (list + delete + add by universe), chat has 'Remember across incarnations' toggle. - tests: 13 NPCVoice universe-scoping tests, 3 isolation tests in test_character_conversation, 9 frontend version-API tests; 87 agent tests, 39 backend tests, 38 frontend tests, type-check clean. - docs: docs/use-cases/epic-3-identity-M/M-31/CHARACTER_VERSIONS.md describes the architecture, data model, endpoint catalog, migration. docs/USE_CASES.md adds a VERSIONS row pointing at it. Use cases: M-31, DL-20, P-17, P-20. Co-Authored-By: Claude <noreply@anthropic.com>
bd7700c to
e5f1282
Compare
P1 — packages/data-layer/src/monitor_data/tools/neo4j_tools/core.py Fork/split/merge Cypher matched (u)-[:IN_MULTIVERSE]->(mv) but the schema (core.py:72 / :314) creates (m)-[:CONTAINS]->(u) — opposite direction. Queries matched zero rows, so fork/split/merge silently failed. Flipped all three to (mv)-[:CONTAINS]->(source/new). P1 — packages/ui/backend/src/monitor_ui/routers/character_conversation.py _provision_entity_and_profile called neo4j_create_entity directly, bypassing the CanonKeeper authority check (INV-1: only CanonKeeper writes Neo4j entity nodes). Route through CanonKeeper.create_entity. P2 — packages/agents/src/monitor_agents/loops/conversation_loop.py ConversationState did not declare include_cross_incarnation, so assignments to state.include_cross_incarnation were silently dropped. Added the field with a False default (preserves per-universe incarnation isolation). P2 — packages/agents/src/monitor_agents/npc_voice.py _recall_memories filtered by universe_id even when include_cross_incarnation=True, making the flag a no-op. Drop the universe scope when cross-incarnation recall is requested. Co-Authored-By: Claude <noreply@anthropic.com>
1 participant
v1.1 Final Polish
This PR merges the v1.1-final-polish branch — the work that turned the
v1.1.0 release into a shippable system.
Highlights
cross-incarnation leak. Includes NPCProfile generation prompt, the
include_cross_incarnationflag for opt-in cross-universe recall,and the CharacterCard import / expansion path.
dialogues, relationship deltas, the full NPCVoice pipeline.
{type:"phase"}eventsbefore
start; "Recalling the world… / Rolling… / Writing…"surfacing for the UI's ThinkingIndicator.
session prep, handout generation, story threads.
quick-worldanddemo-worldpaths.scripts/run_e2e_replays.pyand theper-suite scripts (
forge_replay,long_form_narration,subsystem_replay,character_creation_replay) + docs indocs/testing/E2E_REPLAY.md.CharacterCreationProcedureschema: added legacy enum aliases(
choose_name,choose_attributes,choose_disciplines,choose_advantages) so existing bundles load.neo4j_create_fact/neo4j_create_event/neo4j_update_fact:flatten nested Map properties to JSON strings before writing to
Neo4j (Cypher rejects non-primitive property values).
Codex review (all four findings resolved)
core.py—fork_universe/split_universe/merge_universesmatched
(u)-[:IN_MULTIVERSE]->(mv)but the schema creates(m)-[:CONTAINS]->(u). Queries matched zero rows. Fixed: flippedto
(mv)-[:CONTAINS]->(u).character_conversation.py—_provision_entity_and_profilecalled
neo4j_create_entitydirectly, bypassing the CanonKeeperauthority check (INV-1). Fixed: routes through
CanonKeeper.create_entity.conversation_loop.py—ConversationStatedid not declareinclude_cross_incarnation; assignments were silently stripped.Fixed: added the field with a False default.
npc_voice.py—_recall_memoriesfiltered byuniverse_ideven when
include_cross_incarnation=True. Fixed: drops theuniverse scope when cross-incarnation recall is requested.
Use-case traceability
Every change references a use-case ID (
DL-*,P-*,M-*,CF-*,DOC-*). New use-cases documented underdocs/use-cases/epic-9-docs-DOC/DOC-1/.Test coverage
system (
test_npc_voice_universe_scoping*).Files changed
60 files, ~3,500 lines added. See Files Changed tab.
🤖 Generated with Claude Code