feat(llm): SSOT extract + connector synthesis + conversation topic for desktop invent paths - #11286
Conversation
Add return-only /v1/knowledge-graph/extract through get_llm(knowledge_graph), wire desktop save_knowledge_graph to prefer discovery_text via that endpoint, and accept explicit Luna/auto lane ids for managed desktop chat. Co-authored-by: Cursor <cursoragent@cursor.com>
Add return-only POST /v1/memories/extract through get_llm('memories') and route
Mac/Win memory-log import plus Win local KG synthesis through backend extract
instead of Anthropic Haiku chat completions.
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Failure-Class: none Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Failure-Class: none Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
discovery_text is preferred and nodes/edges are no longer required. Co-authored-by: Cursor <cursoragent@cursor.com>
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_dae915fc-895b-4728-a54e-70abeefe149f) |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
9 issues found across 42 files
Confidence score: 3/5
- In
backend/utils/llm/knowledge_graph.pyanddesktop/macos/Desktop/Sources/Providers/ChatToolExecutor.swift, repeateddiscovery_textsaves can assign fresh IDs to entities that already exist, fragmenting the local graph and weakening incremental merge quality across sessions — derive deterministic IDs (uid+normalized label) and resolve extracted labels/aliases to existing local node IDs before merge. backend/utils/llm/knowledge_graph.py(extraction_to_client_graph) anddesktop/windows/src/main/agentKernel/productToolExecutors.tscan mis-handle extracted entities: shared label/alias ID mapping can collapse distinct nodes while appending duplicates, and Windows type filtering can drop backend-emitted node types, leading to missing or incorrect KG structure — split node/edge ID bookkeeping and broaden or map unsupported node types before save.- In
backend/routers/desktop_chat.py, Luna/auto lane IDs can end up with incorrectgateway_modewhen gateway is disabled or BYOK Anthropic is set, which risks requests being routed through the wrong path and failing unexpectedly — aligngateway_modederivation with lane/config combinations in all branches. - Memory extraction limits are inconsistent between
backend/routers/memories.pyandbackend/utils/llm/memories.py: very largeexisting_memoriescan still blow context/work budgets, and input is silently truncated to 40k despite a 100k contract, causing hidden recall loss or provider failures — enforce per-entry/aggregate prompt budgets and make endpoint limits/truncation behavior explicit and consistent.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="backend/routers/desktop_chat.py">
<violation number="1" location="backend/routers/desktop_chat.py:233">
P2: The new Luna/auto lane ids ('omi-luna', 'omi-auto', 'omi:auto:chat-agent') route to the managed gateway when it is active, but when the gateway feature is off or a BYOK Anthropic key is configured, `gateway_mode` is forced false and `_request()` rejects these models with HTTP 400 'unsupported model' (they are not in `_MODEL_ROUTES`). Desktop traffic that starts sending these aliases (per the PR intent of 'accept explicit Luna/auto managed chat lane ids') will therefore hard-fail in any non-gateway/BYOK environment, where the previous Sonnet aliases (e.g. 'omi-sonnet', which are in `_MODEL_ROUTES`) still worked on the direct path. Consider adding the aliases to `_MODEL_ROUTES` (mapping to the Sonnet model) so the direct/BYOK fallback remains functional, or explicitly fail over to the direct Sonnet path when gateway mode is unavailable, rather than surfacing an 'unsupported model' 400.</violation>
</file>
<file name="desktop/macos/Desktop/Sources/Providers/ChatToolExecutor.swift">
<violation number="1" location="desktop/macos/Desktop/Sources/Providers/ChatToolExecutor.swift:2846">
P2: Repeated `discovery_text` saves create new IDs for already-present entities, fragmenting the incremental local graph; resolve extracted labels/aliases to existing local node IDs before merging.</violation>
<violation number="2" location="desktop/macos/Desktop/Sources/Providers/ChatToolExecutor.swift:2846">
P3: Discovery-text saves report zero nodes and edges in `onboardingChatToolUsed`; derive analytics counts from resolved extraction/save result so onboarding graph telemetry reflects saved data.</violation>
</file>
<file name="backend/routers/memories.py">
<violation number="1" location="backend/routers/memories.py:472">
P2: Oversized `existing_memories` entries can create provider-context failures or excessive request work despite the 200-item cap; bound each entry and the aggregate prompt budget before calling the extractor.</violation>
</file>
<file name="desktop/macos/e2e/flows/chat-first-cohesive.yaml">
<violation number="1" location="desktop/macos/e2e/flows/chat-first-cohesive.yaml:27">
P3: The added `covers:` entry claims this flow exercises "save_knowledge_graph discovery_text resolves through backend KG extract SSOT", but none of the chat-first-cohesive steps (`chat_first_render_fixture_task_card` renders a task card, S14-S16 only start/discuss a capture turn) invoke `save_knowledge_graph` with `discovery_text`, so `KnowledgeGraphToolSupport.resolveDiscoveryText` is never reached. List a file under a flow's `covers:` only when the flow actually exercises that path; this file is already covered by `memory-graph.yaml`, so the entry is both inaccurate and redundant.</violation>
</file>
<file name="backend/utils/llm/knowledge_graph.py">
<violation number="1" location="backend/utils/llm/knowledge_graph.py:174">
P2: Repeated extraction creates a new local node for the same label because client graph IDs are random on every request; derive IDs deterministically from `uid` and normalized label so `save_knowledge_graph` can merge repeated discoveries.</violation>
<violation number="2" location="backend/utils/llm/knowledge_graph.py:174">
P2: In `extraction_to_client_graph`, node ids and edge ids are derived from a single shared `label_to_node_id` map keyed on lowercased label plus every alias, but each node/edge is still appended to the returned arrays. Distinct entities that share a label (e.g. the LLM returns two 'neo' nodes, or one node labeled 'City' collides with another node '$Zion' having alias 'City') collapse to the same `node_id` while both are emitted, so the returned graph contains duplicate nodes carrying the same id; likewise two edges between the same source/target with the same label share one `edge_id`. Because this graph is the input to desktop `save_knowledge_graph` persistence, colliding ids can cause one entity to overwrite another on upsert or drop a relationship. Consider deduplicating nodes by label/alias (emit at most one node per resolved id) and making edge ids unique per instance, e.g. by including an occurrence index.</violation>
</file>
<file name="backend/utils/llm/memories.py">
<violation number="1" location="backend/utils/llm/memories.py:286">
P2: The endpoint advertises that it accepts up to 100,000 characters of text, but the extraction helper silently truncates the input to the first 40,000 characters via `content[:40_000]`. A desktop onboarding/import that pastes a log between 40k and 100k will get a successful 200 with memories extracted only from the head of the log — tail facts are dropped with no signal to the caller. Consider aligning the request contract with the processing cap (e.g. also `max_length=40_000` on the route) or returning a truncation flag in the response so clients know part of the log was not considered.</violation>
</file>
<file name="desktop/windows/src/main/agentKernel/productToolExecutors.ts">
<violation number="1" location="desktop/windows/src/main/agentKernel/productToolExecutors.ts:1088">
P2: The new `discovery_text` path saves backend-extracted nodes through `KG_NODE_TYPES`, which only accepts person/organization/place/thing/concept. But this same PR's `kgSynthesis.mapBackendNodeType` proves the backend extract can return `node_type` of `project`, `interest`, and `org` — none of which are in that set. When a real agent calls `save_knowledge_graph` with `discovery_text`, any project/interest/org entity the backend returns is silently coerced to `thing` in the saved graph, so those entities lose their semantic type. Worth aligning the two consumers on one node_type vocabulary (e.g. extend `KG_NODE_TYPES` to cover project/interest/org) before merge.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| normalized = model.strip().lower() | ||
| if not normalized: | ||
| return True | ||
| if normalized in _MANAGED_CHAT_ALIASES: |
There was a problem hiding this comment.
P2: The new Luna/auto lane ids ('omi-luna', 'omi-auto', 'omi:auto:chat-agent') route to the managed gateway when it is active, but when the gateway feature is off or a BYOK Anthropic key is configured, gateway_mode is forced false and _request() rejects these models with HTTP 400 'unsupported model' (they are not in _MODEL_ROUTES). Desktop traffic that starts sending these aliases (per the PR intent of 'accept explicit Luna/auto managed chat lane ids') will therefore hard-fail in any non-gateway/BYOK environment, where the previous Sonnet aliases (e.g. 'omi-sonnet', which are in _MODEL_ROUTES) still worked on the direct path. Consider adding the aliases to _MODEL_ROUTES (mapping to the Sonnet model) so the direct/BYOK fallback remains functional, or explicitly fail over to the direct Sonnet path when gateway mode is unavailable, rather than surfacing an 'unsupported model' 400.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At backend/routers/desktop_chat.py, line 233:
<comment>The new Luna/auto lane ids ('omi-luna', 'omi-auto', 'omi:auto:chat-agent') route to the managed gateway when it is active, but when the gateway feature is off or a BYOK Anthropic key is configured, `gateway_mode` is forced false and `_request()` rejects these models with HTTP 400 'unsupported model' (they are not in `_MODEL_ROUTES`). Desktop traffic that starts sending these aliases (per the PR intent of 'accept explicit Luna/auto managed chat lane ids') will therefore hard-fail in any non-gateway/BYOK environment, where the previous Sonnet aliases (e.g. 'omi-sonnet', which are in `_MODEL_ROUTES`) still worked on the direct path. Consider adding the aliases to `_MODEL_ROUTES` (mapping to the Sonnet model) so the direct/BYOK fallback remains functional, or explicitly fail over to the direct Sonnet path when gateway mode is unavailable, rather than surfacing an 'unsupported model' 400.</comment>
<file context>
@@ -205,26 +205,33 @@ async def bounded_receive():
+ normalized = model.strip().lower()
+ if not normalized:
+ return True
+ if normalized in _MANAGED_CHAT_ALIASES:
+ return True
if normalized in _MODEL_ROUTES:
</file context>
| discoveryText, expectedOwnerId: expectedOwnerID) | ||
| { | ||
| case .success(let graph): | ||
| nodesArray = graph.nodes |
There was a problem hiding this comment.
P2: Repeated discovery_text saves create new IDs for already-present entities, fragmenting the incremental local graph; resolve extracted labels/aliases to existing local node IDs before merging.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At desktop/macos/Desktop/Sources/Providers/ChatToolExecutor.swift, line 2846:
<comment>Repeated `discovery_text` saves create new IDs for already-present entities, fragmenting the incremental local graph; resolve extracted labels/aliases to existing local node IDs before merging.</comment>
<file context>
@@ -2836,10 +2836,22 @@ class ChatToolExecutor {
+ discoveryText, expectedOwnerId: expectedOwnerID)
+ {
+ case .success(let graph):
+ nodesArray = graph.nodes
+ edgesArray = graph.edges
+ case .failure(let message):
</file context>
|
|
||
| text: str = Field(..., min_length=1, max_length=100_000) | ||
| text_source: str = Field(default="memory_log", min_length=1, max_length=64) | ||
| existing_memories: List[str] = Field(default_factory=list, max_length=200) |
There was a problem hiding this comment.
P2: Oversized existing_memories entries can create provider-context failures or excessive request work despite the 200-item cap; bound each entry and the aggregate prompt budget before calling the extractor.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At backend/routers/memories.py, line 472:
<comment>Oversized `existing_memories` entries can create provider-context failures or excessive request work despite the 200-item cap; bound each entry and the aggregate prompt budget before calling the extractor.</comment>
<file context>
@@ -464,6 +464,48 @@ def _validate_mutable_memory(uid: str, memory_id: str, *, db_client: Any) -> Mem
+
+ text: str = Field(..., min_length=1, max_length=100_000)
+ text_source: str = Field(default="memory_log", min_length=1, max_length=64)
+ existing_memories: List[str] = Field(default_factory=list, max_length=200)
+
+
</file context>
| label_to_node_id: Dict[str, str] = {} | ||
| nodes: List[Dict[str, Any]] = [] | ||
| for node in extraction.nodes: | ||
| node_id = label_to_node_id.get(node.label.lower()) or str(uuid.uuid4()) |
There was a problem hiding this comment.
P2: Repeated extraction creates a new local node for the same label because client graph IDs are random on every request; derive IDs deterministically from uid and normalized label so save_knowledge_graph can merge repeated discoveries.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At backend/utils/llm/knowledge_graph.py, line 174:
<comment>Repeated extraction creates a new local node for the same label because client graph IDs are random on every request; derive IDs deterministically from `uid` and normalized label so `save_knowledge_graph` can merge repeated discoveries.</comment>
<file context>
@@ -135,62 +155,133 @@ def extract_knowledge_from_memory(
+ label_to_node_id: Dict[str, str] = {}
+ nodes: List[Dict[str, Any]] = []
+ for node in extraction.nodes:
+ node_id = label_to_node_id.get(node.label.lower()) or str(uuid.uuid4())
+ label_to_node_id[node.label.lower()] = node_id
+ for alias in node.aliases:
</file context>
| discoveryText, expectedOwnerId: expectedOwnerID) | ||
| { | ||
| case .success(let graph): | ||
| nodesArray = graph.nodes |
There was a problem hiding this comment.
P3: Discovery-text saves report zero nodes and edges in onboardingChatToolUsed; derive analytics counts from resolved extraction/save result so onboarding graph telemetry reflects saved data.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At desktop/macos/Desktop/Sources/Providers/ChatToolExecutor.swift, line 2846:
<comment>Discovery-text saves report zero nodes and edges in `onboardingChatToolUsed`; derive analytics counts from resolved extraction/save result so onboarding graph telemetry reflects saved data.</comment>
<file context>
@@ -2836,10 +2836,22 @@ class ChatToolExecutor {
+ discoveryText, expectedOwnerId: expectedOwnerID)
+ {
+ case .success(let graph):
+ nodesArray = graph.nodes
+ edgesArray = graph.edges
+ case .failure(let message):
</file context>
| - desktop/macos/Desktop/Sources/Chat/AgentClient.swift | ||
| - desktop/macos/Desktop/Sources/Providers/ChatToolExecutor.swift | ||
| # save_knowledge_graph discovery_text resolves through backend KG extract SSOT. | ||
| - desktop/macos/Desktop/Sources/Services/KnowledgeGraphToolSupport.swift |
There was a problem hiding this comment.
P3: The added covers: entry claims this flow exercises "save_knowledge_graph discovery_text resolves through backend KG extract SSOT", but none of the chat-first-cohesive steps (chat_first_render_fixture_task_card renders a task card, S14-S16 only start/discuss a capture turn) invoke save_knowledge_graph with discovery_text, so KnowledgeGraphToolSupport.resolveDiscoveryText is never reached. List a file under a flow's covers: only when the flow actually exercises that path; this file is already covered by memory-graph.yaml, so the entry is both inaccurate and redundant.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At desktop/macos/e2e/flows/chat-first-cohesive.yaml, line 27:
<comment>The added `covers:` entry claims this flow exercises "save_knowledge_graph discovery_text resolves through backend KG extract SSOT", but none of the chat-first-cohesive steps (`chat_first_render_fixture_task_card` renders a task card, S14-S16 only start/discuss a capture turn) invoke `save_knowledge_graph` with `discovery_text`, so `KnowledgeGraphToolSupport.resolveDiscoveryText` is never reached. List a file under a flow's `covers:` only when the flow actually exercises that path; this file is already covered by `memory-graph.yaml`, so the entry is both inaccurate and redundant.</comment>
<file context>
@@ -23,6 +23,8 @@ covers:
- desktop/macos/Desktop/Sources/Chat/AgentClient.swift
- desktop/macos/Desktop/Sources/Providers/ChatToolExecutor.swift
+ # save_knowledge_graph discovery_text resolves through backend KG extract SSOT.
+ - desktop/macos/Desktop/Sources/Services/KnowledgeGraphToolSupport.swift
- desktop/macos/Desktop/Sources/Providers/ChatFirstBlockToolExecutor.swift
- desktop/macos/Desktop/Sources/Chat/ChatFirstBlockValidation.swift
</file context>
![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: 49fb99165a
ℹ️ 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".
| } | ||
| } | ||
| return try await post( | ||
| "v1/memories/extract", |
There was a problem hiding this comment.
When a macOS candidate is released before the independently deployed Python backend contains this route, every real memory-log import now receives a 404 and returns .failed; the same rollout dependency exists for the new knowledge-graph call. I checked gcp_backend_auto_dev.yml, desktop_auto_release.yml, and desktop_qualify_beta.yml: backend deployment is downstream of Release Eligibility, while release planning waits only for that eligibility check and qualification probes /v1/health, not either new route, so a failed or still-running deployment does not block promotion. Add an exact endpoint-capability gate before releasing the client.
AGENTS.md reference: desktop/macos/AGENTS.md:L71-L71
Useful? React with 👍 / 👎.
| if (t === 'project') return 'project' | ||
| if (t === 'interest') return 'interest' | ||
| if (t === 'place' || t === 'thing' || t === 'concept') return 'interest' |
There was a problem hiding this comment.
For Windows graph rebuilds containing project evidence, the new backend contract advertises only person, place, thing, concept, and organization (backend/utils/llm/knowledge_graph.py), whereas the previous buildSynthesisPrompt explicitly requested project nodes. Mapping every returned thing or concept to interest therefore turns projects into interests, so project-specific graph nodes disappear from normal SSOT output. Extend the shared extraction type contract to represent projects instead of relying on a value the backend prompt does not request.
Useful? React with 👍 / 👎.
| const extracted = await backendJsonFetch({ | ||
| method: 'POST', | ||
| path: '/v1/knowledge-graph/extract', | ||
| body: { text: discoveryText, include_existing: false }, | ||
| timeoutMs: 60_000, |
There was a problem hiding this comment.
When the relay socket disconnects during this new network-backed tool call, the executor ignores its ProductToolContext and does not pass ctx.signal to backendJsonFetch. The request can therefore continue for the full 60-second timeout and then execute upsertLocalGraph even though the originating run can no longer receive the result; this violates the existing executor contract that disconnects abort in-flight work. Accept ctx, pass its signal into the request, and check cancellation before the local write.
Useful? React with 👍 / 👎.
| extraction = getattr(kg_mod, "extract_kg_from_text")( | ||
| uid, | ||
| body.text, | ||
| user_name=user_name, | ||
| load_existing_from_db=body.include_existing, |
There was a problem hiding this comment.
When Luna returns malformed structured output, extract_kg_from_text uses its default strict_parse=False and converts the parse failure into an empty extraction, so this new HTTP route returns 200 with empty arrays instead of the intended 502. Windows then treats the request as successful, saves only the deterministic floor, and stamps the graph fresh for 12 hours, suppressing a retry. Pass strict_parse=True at this HTTP boundary so provider/parse failures remain distinguishable from a valid no-entities result.
Useful? React with 👍 / 👎.
| label_to_node_id: Dict[str, str] = {} | ||
| nodes: List[Dict[str, Any]] = [] | ||
| for node in extraction.nodes: | ||
| node_id = label_to_node_id.get(node.label.lower()) or str(uuid.uuid4()) |
There was a problem hiding this comment.
Whenever the same entity is extracted in more than one discovery_text call, this generates a fresh UUID even for an identical normalized label. Both macOS local_kg_nodes and Windows onboarding_kg_nodes upsert only by ID, so repeated onboarding discoveries create duplicate nodes and new duplicate edges rather than merging the entity as the tool contract promises. Derive IDs deterministically from the canonical label/type, or reconcile against the caller's existing graph before returning them.
Useful? React with 👍 / 👎.
| tags=['knowledge_graph'], | ||
| response_model=ExtractKnowledgeGraphResponse, | ||
| ) | ||
| def extract_knowledge_graph( |
There was a problem hiding this comment.
Under concurrent extract requests from multiple users, these synchronous FastAPI handlers hold Starlette's shared sync-route worker pool while get_llm(...).invoke() waits on the provider; the memories endpoint is implemented the same way. A slow provider can therefore exhaust the general pool and starve unrelated synchronous API routes despite the repository's dedicated six-worker LLM lane. Make the handlers async and offload each extraction with await run_blocking(llm_executor, ...).
AGENTS.md reference: backend/AGENTS.md:L283-L292
Useful? React with 👍 / 👎.
| } catch (e) { | ||
| console.warn('[kg] synthesis LLM call failed; saving deterministic floor only', e) | ||
| console.warn('[kg] backend knowledge-graph extract failed; saving deterministic floor only', e) |
There was a problem hiding this comment.
When the new backend extract call fails, Windows silently switches from semantic extraction to a deterministic-only graph and still persists that degraded result, but the branch emits only a renderer console warning. This provider/correctness fallback needs the shared bounded recordFallback telemetry so backend-route outages and authentication failures are operationally visible rather than looking like successful graph builds.
AGENTS.md reference: AGENTS.md:L91-L91
Useful? React with 👍 / 👎.
| 'omi-luna', | ||
| 'omi-auto', | ||
| CHAT_AGENT_AUTO_LANE_ID, |
There was a problem hiding this comment.
When gateway routing is disabled, or an Anthropic BYOK key deliberately changes gateway_mode to false, each newly accepted alias reaches _request, whose model allowlist still excludes omi-luna, omi-auto, and omi:auto:chat-agent; the request therefore returns HTTP 400 instead of taking the recorded Anthropic fallback. Empty or whitespace-only models now have the same inconsistency. Normalize these managed aliases to a supported direct model before _request, or keep them out of managed triage when that fallback cannot serve them.
Useful? React with 👍 / 👎.
| 'Prefer discovery_text with raw findings; backend extract builds the graph. nodes/edges remain accepted.' | ||
| ], | ||
| latency: 'fast local', | ||
| latency: 'fast network', |
There was a problem hiding this comment.
When knowledge-graph extraction takes between 30 and 60 seconds, the Windows relay's normal timeout returns an error at 30 seconds even though the newly added backend request is explicitly allowed to run for 60 seconds. The relay timeout also does not abort its controller, so the executor can subsequently persist the graph after the model was told the tool failed. Moving this tool from local to network latency must also move it to a timeout class that covers the request, or shorten and abort the underlying request at the relay deadline.
Useful? React with 👍 / 👎.
| Does not write Firestore. Desktop onboarding/import should call this instead of inventing | ||
| memories via Anthropic Haiku chat completions, then persist via the normal memory write APIs. | ||
| """ | ||
| from utils.llm import memories as memories_llm |
There was a problem hiding this comment.
Every call to the new memories endpoint performs a function-local import of its LLM business-logic dependency, hiding the router-to-utils dependency from normal import-time validation and violating the backend's strict module hierarchy convention. Import utils.llm.memories at module scope, as required for backend serving code, so dependency and startup failures are detected before the first user request.
AGENTS.md reference: backend/AGENTS.md:L181-L187
Useful? React with 👍 / 👎.
Git-on-my-level
added
needs-maintainer-review
|
Thanks for pushing this toward a single backend-owned extraction path. I reviewed the diff and I’m leaving this for human maintainer review rather than approval because it changes backend LLM routing/API surface, desktop agent tool behavior, and the backend unit suite is currently red. Code-specific notes:
Validation notes:
Recommended next step: keep this as a cohesive PR, but get maintainer sign-off on the desktop agent behavior change and run/record the real desktop paths once the backend suite is green. Automated review generated by glm-5.2 for maintainer triage. A human maintainer should make the final product/merge decision. by AI on behalf of David — if you need David’s attention urgently, please @Git-on-my-level and escalate with |
Add return-only POST /v1/connectors/synthesize backed by get_llm('memories') so the
calendar, gmail and notes prompts live in the backend instead of each desktop client
inventing memories/tasks/profile through Anthropic Haiku chat completions.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Calendar, Gmail and Sticky Notes imports now post rows to POST /v1/connectors/synthesize instead of building their own prompts and calling Anthropic Haiku via /v2/chat/completions. Verified: pnpm vitest run (calendarExtract, gmailExtract, stickyNotesImport) — 12 passed; tsc --noEmit -p tsconfig.web.json clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Calendar, Gmail and Apple Notes readers now post their formatted rows to POST /v1/connectors/synthesize instead of building Haiku synthesis prompts and parsing free-form JSON via AgentClient. Verified: xcrun swift build -c debug --package-path Desktop — Build complete; xctest -XCTest ConnectorSynthesisResponseTests — 2 tests, 0 failures. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Add /v1/connectors to the app-client contract prefixes so the new synthesis endpoint ships in the exported spec, and regenerate the Swift/TS clients. Verified: xcrun swift build -c debug --package-path Desktop — Build complete. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`topBarNewSinceRaw == 0` inside the didBecomeActive closure pushed the modifier
chain past the type-checker budget ("unable to type-check this expression in
reasonable time"), failing xcrun swift build -c debug on a clean tree. `.isZero`
removes the literal overload search with identical behavior.
Verified: xcrun swift build -c debug --package-path Desktop — Build complete
(fails on the same tree without this change).
Failure-Class: none
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Add return-only POST /v1/conversations/topic backed by get_llm('conv_structure')
so the provisional emoji + short title prompt lives in the backend, and route the
Windows fast-titling path there instead of Anthropic Haiku chat completions.
Verified: backend/.venv/bin/python -m pytest tests/unit/test_conversation_topic_helper.py
tests/unit/test_conversation_topic_route.py tests/unit/test_rate_limiting.py — 74 passed;
vitest conversationTopic.test.ts — 2 passed; tsc --noEmit -p tsconfig.web.json clean.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Connector synthesis and memory-log import now run through backend SSOT endpoints, so no production path selects the Haiku synthesis model any more. Verified: xcrun swift build -c debug --package-path Desktop --build-tests — Build complete; xctest -XCTest ModelQoSTests — 16 tests, 0 failures. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0244d3567f
ℹ️ 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".
| with track_usage(uid, Features.MEMORIES): | ||
| response = get_llm('memories').invoke(prompt) |
There was a problem hiding this comment.
When an attacker-controlled Gmail subject/snippet or a synced note contains prompt-like instructions, the connector rows and extraction contract are now sent together as one human message. The previous macOS and Windows connector paths supplied a separate system message; here the schema parser validates only the response shape, so injected but well-formed memories or tasks can be accepted and persisted by both clients. Pass the fixed contract as a system message and the connector rows as separately delimited human content.
Useful? React with 👍 / 👎.
| def _stage2_user_prompt(fresh_profile: str, past_profiles: List[str]) -> str: | ||
| past_section = "\n\n".join(f"--- Profile {i + 1} ---\n{text}" for i, text in enumerate(past_profiles)) |
There was a problem hiding this comment.
For macOS users whose historical profiles contain relative or time-sensitive facts such as “prepare the demo next Thursday,” this strips each record down to text and labels it only by ordinal position. The previous macOS consolidation prompt included each profile's generatedAt date, which allowed the model to interpret relative dates and remove expired commitments as the stage-two rules require; after this change those facts can be retained as current and injected into downstream pipelines. Carry the generation timestamp in the request contract and render it beside each historical profile.
Useful? React with 👍 / 👎.
Git-on-my-level
left a comment
Git-on-my-level
left a comment
There was a problem hiding this comment.
Thanks for the follow-up. One of the previous blockers is fixed on this head: docs/api-reference/app-client-openapi.json now includes POST /v1/conversations/topic, and the current GitHub checks are green.
I’m still requesting changes for the remaining compatibility issue in the shipped desktop tool path:
desktop/macos/Desktop/Sources/Providers/ChatToolExecutor.swift+desktop/macos/Desktop/Sources/Services/KnowledgeGraphToolSupport.swift:executeSaveKnowledgeGraphswitches to backend extraction whenever thediscovery_textkey is present, before checking whether it trims to a non-empty value.KnowledgeGraphToolSupport.resolveDiscoveryTextthen returnsError: 'discovery_text' or 'nodes' is requiredfor blank text. That means a legacy/compatibility call that still provides validnodes/edgesbut also carriesdiscovery_text: " "fails instead of saving the provided graph, even though the tool manifest says nodes/edges remain accepted for compatibility. Please only take the backend-extraction path whendiscovery_text.trimmingCharacters(...)is non-empty, otherwise fall back to the provided nodes/edges.
Other code-specific notes from this head:
desktop/windows/src/main/agentKernel/productToolExecutors.tsalready uses the safer trimmedconst discoveryText = ...trim()and only calls/v1/knowledge-graph/extractwhen that string is non-empty, so the Windows executor preserves the legacy fallback behavior.backend/routers/conversations.py,backend/routers/knowledge_graph.py,backend/routers/memories.py,backend/routers/integrations.py, andbackend/routers/users.pyadd return-only, Firebase-authenticated endpoints with explicit rate-limit policy entries inbackend/route_policy_manifest.yaml; that direction looks coherent for moving desktop-owned prompt invention behind backend SSOT LLM routes.backend/utils/llm/conversation_topic.py,backend/utils/llm/knowledge_graph.py,backend/utils/llm/memories.py,backend/utils/llm/connector_synthesis.py, andbackend/utils/llm/ai_user_profile.pyinclude parser/truncation guards and focused unit coverage for the helper and route seams..github/scripts/product_file_line_count_ratchet_baseline/*.json,backend/scripts/product_capability_synthetics.py,backend/testing/replay_harness_llm_gateway_fake_upstream/oracle.py, andbackend/scripts/export_openapi.pyhave been updated to account for the new backend route and gateway lane behavior.
This PR also changes desktop AI-agent tool behavior: save_knowledge_graph now encourages raw discovery_text and may call the backend knowledge-graph extraction route before local persistence. That is a reasonable SSOT direction, but it changes agent latency/failure modes and the instructions coding/review agents see for this repo, so human maintainer sign-off is still warranted for the product/tool-behavior direction.
Reviewed by gpt-5.5 for the Omi maintainer automation; leaving this for human maintainer review because it changes backend-owned LLM extraction over memory/conversation/connector data and desktop agent tool behavior.
by AI on behalf of David — if you need David’s attention urgently, please @Git-on-my-level and escalate with need human response.
… blank executeSaveKnowledgeGraph switched to backend extraction on the mere presence of a discovery_text key, so a compatibility call carrying valid nodes/edges plus a whitespace-only discovery_text failed with "'discovery_text' or 'nodes' is required" instead of saving the graph the tool supplied. Extraction now runs only for a non-empty trimmed value, and an extract failure falls back to the provided nodes (recorded as a degraded fallback) rather than discarding them. Also drop KnowledgeGraphToolSupport.swift from memory-graph.yaml's covers: none of that flow's steps reach the save_knowledge_graph tool path. Verified: xcrun swift build -c debug --package-path Desktop --build-tests — Build complete; xctest ConnectorSynthesisResponseTests / AIUserProfileSynthesisResponseTests / ModelQoSTests — 20 tests, 0 failures. Failure-Class: none Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The five return-only LLM routes were synchronous handlers, so a slow provider held Starlette's shared sync-route pool instead of the dedicated six-worker LLM lane and could starve unrelated sync APIs. They also applied only an hourly rate limit: the client paths they replaced went through /v2/chat/completions, which refuses a trial-expired account before touching a provider. Each handler is now async, dispatches its extractor with run_blocking(llm_executor, ...), and refuses a paywalled account with 402 before the model call. The knowledge-graph route additionally passes strict_parse=True so a malformed model response fails closed (502) instead of returning 200 with an empty graph that a client cannot tell apart from a genuine "no entities" answer, and the memory-log request cap now matches the helper's own 40k truncation budget instead of advertising 100k and silently dropping the tail. Verified: backend/.venv/bin/python -m pytest on the ten helper/route suites plus test_rate_limiting.py and test_memories_create.py — 129 passed. Failure-Class: none Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…n input Both helpers sent the extraction contract and the untrusted payload as one human message. An imported ChatGPT/Claude log, a Gmail subject or snippet, and a synced note are all attacker-reachable text, and the schema parser only validates the response shape — so a well-formed injected memory or task would be accepted and persisted by both clients. The client paths these replaced kept the contract in a separate system message. The contract is now a system message, the payload a separately delimited human message, and both prompts state that the payload is data rather than instructions. Verified: backend/.venv/bin/python -m pytest tests/unit/test_memories_extract_helper.py tests/unit/test_connector_synthesis_helper.py — passing, with both tests asserting the two-role split. Failure-Class: none Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Selecting omi-structured routed the request to omi:auto:chat-structured, but the gateway path still hardcoded chat_agent in the request headers, the usage context, the latency observation and the result record (including the chat-agent route id). Every structured-lane attempt was therefore written to the accounting ledger and reliability metrics as chat traffic, hiding the new lane's cost and failures. The feature and route are now derived from the selected lane on both the streaming and non-streaming paths. Verified: backend/.venv/bin/python -m pytest tests/unit/test_desktop_chat.py — 59 passed. Failure-Class: none Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…r binding The new extract/synthesis endpoints inherited OmiHTTPTransport's shared 30s request timeout while Windows budgets 60s for the same routes, so a slow managed call was cancelled client-side — worst on the profile route, which runs two sequential model calls. post() now takes a per-request timeout and these four calls use 60s. Profile synthesis also posted without an owner binding while carrying one account's memories, messages and past profiles: a 401 retry after a mid-flight account switch would have replayed them under the new account's token. It now pins an owner authorization snapshot like the other extract methods. Verified: xcrun swift build -c debug --package-path Desktop --build-tests — Build complete. Failure-Class: none Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…action discovery_text turned save_knowledge_graph into a network edge with a 60s backend request, but the Windows executor ignored its ProductToolContext, so a relay disconnect left the request running and still wrote the graph locally; and the tool kept the 'normal' 30s relay deadline, which reports failure while that request is still in flight. The executor now forwards ctx.signal and checks cancellation before the local write, and both manifests move the tool to the 'long' timeout class. Also drop the duplicated discovery_text prompt bullet (the capability doc already states it) and regenerate the Swift tool surfaces, and make the Tier-B aliases assertion actually exercise the mapping instead of matching on undefined. Verified: vitest run src/main/agentKernel src/renderer/src/lib src/main/assistants — 2505 passed; tsc --noEmit on both Windows projects clean. Failure-Class: none Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The connector transport caps each item at 1000 chars, so a Sticky Note line longer than that lost its tail; long lines are now chunked instead of truncated. And when the backend graph extract fails, Windows saves a deterministic-only graph and stamps it fresh for 12 hours — a provider/correctness fallback that emitted only a console.warn. It now writes the structured, loud fallback record the contract asks for (the renderer has no recordFallback emitter; see billing.ts). Verified: vitest run src/renderer/src/lib — passing; tsc --noEmit clean. Failure-Class: none Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Making the handler async left get_user_name as a sync Firestore call on the event loop, which the async-blocker gate rejects. Verified: backend/.venv/bin/python -m pytest tests/unit/test_knowledge_graph_extract_route.py — 3 passed. Failure-Class: none Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…nges Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_38742660-b1b0-47e6-8f0c-78fee1b6dca2) |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ed54939785
ℹ️ 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".
| ) | ||
| log("AIUserProfileService: Stage 2 consolidation complete (\(finalText.count) chars)") | ||
| } | ||
| let synthesis = try await APIClient.shared.synthesizeAIUserProfile( |
There was a problem hiding this comment.
When account A starts generation and the app switches to B during fetchDataSources(), this call captures its authorization snapshot only after the asynchronous reads, so B can become the pinned owner while the payload still contains A's or mixed source lines and historical profiles; the response can then be persisted into B's local profile. Fresh evidence beyond the earlier comment is that the final implementation still calls synthesizeAIUserProfile without the originating expectedOwnerId or snapshot after source collection. Capture one owner snapshot before any reads and thread it through collection, synthesis, and persistence.
AGENTS.md reference: AGENTS.md:L89-L89
Useful? React with 👍 / 👎.
| // still stamped fresh, so this must not look like a successful build. The renderer | ||
| // has no recordFallback emitter (see billing.ts), so this is the same structured, | ||
| // loud console record the fallback contract asks for. | ||
| console.error('[kg] fallback', { |
There was a problem hiding this comment.
When graph extraction fails, this branch persists the deterministic-only graph and stamps it fresh for 12 hours, but console.error does not emit the operational fallback_triggered event; the same Windows renderer already uses trackEvent('fallback_triggered', ...) for degraded branches. Fresh evidence beyond the prior comment is that the revised branch only changed to a structured console record rather than an actual telemetry emitter, so backend extraction outages remain invisible to fallback monitoring.
AGENTS.md reference: AGENTS.md:L91-L91
Useful? React with 👍 / 👎.
| existing = [m.strip() for m in (existing_memories or []) if m.strip()][:MAX_EXISTING] | ||
| existing_block = "\n".join(f"- {m}" for m in existing) if existing else "(none)" |
There was a problem hiding this comment.
When a Windows Gmail or Sticky Notes import supplies long existing memories, this keeps up to 200 strings at unlimited length and inserts them verbatim into the sole LLM prompt. Memory content itself has no model-level length bound, and the Windows transport truncates only the list count, so a few large manual/imported memories can exceed the provider context and make connector synthesis return 502 even though source items are capped. Apply a per-memory character limit here, as the memory-log extraction route already does.
Useful? React with 👍 / 👎.
| if consolidated: | ||
| profile_text = consolidated | ||
| else: | ||
| logger.warning("AI user profile stage 2 returned empty content for uid=%s; keeping stage 1", uid) |
There was a problem hiding this comment.
When historical profiles exist and the second model call succeeds at the transport layer but returns empty content, this silently falls back to the fresh stage-one profile and the route returns 200. Both desktop clients then store and sync that incomplete profile, temporarily dropping stable facts that existed only in historical profiles; the previous Windows transport treated empty content as an error and kept the prior profile instead. Return None so the route responds 502, or explicitly record this correctness-degrading fallback before allowing clients to overwrite the consolidated profile.
AGENTS.md reference: AGENTS.md:L91-L91
Useful? React with 👍 / 👎.
|
|
||
| def cleaned(self) -> "ProfileSources": | ||
| def clean(lines: List[str]) -> List[str]: | ||
| return [line.strip()[:MAX_LINE_CHARS] for line in lines if line.strip()][:MAX_LINES_PER_SOURCE] |
There was a problem hiding this comment.
When a memory, conversation overview, or AI chat message exceeds 1,000 characters, this silently discards the remainder of that source item before synthesis. Both previous desktop implementations joined the complete fetched strings into the stage-one prompt, and the new request schema permits these longer values, so durable facts appearing later in an otherwise valid long message can no longer enter the generated profile. Chunk long entries within the source-count budget or enforce a total prompt budget without dropping each line's tail.
Useful? React with 👍 / 👎.
The entitlement gate imported utils.subscription at module scope, which pulls database.users in at import time and broke the v3 router isolation test's minimal dependency graph. The import now sits beside the LLM helper's deferred import, with the reason recorded, and the isolation stub gains the llm_executor the router now dispatches through. Verified: backend/.venv/bin/python -m pytest tests/unit/test_v3_real_router_default_off_f4.py tests/unit/test_memories_extract_route.py — 19 passed; the four other new route suites — 12 passed; tests/unit/test_import_isolation.py tests/unit/test_agent_tools_isolation.py — 5 passed. Failure-Class: none Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Resolved on current head: OpenAPI contract includes the conversation topic route and macOS save_knowledge_graph now falls back to provided nodes/edges when discovery_text is blank.
|
Thanks for the update. I re-checked the current head and the two prior blocking review concerns are resolved, so I dismissed my stale
Current code-specific observations:
I am not requesting changes on this head. The remaining red This PR also changes desktop AI-agent tool behavior: Reviewed by gpt-5.5 for the Omi maintainer automation. by AI on behalf of David — maintainer sign-off is needed here for the new backend-owned LLM extraction APIs and desktop agent tool behavior change. |
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_f9048c7e-8cc9-410f-86e1-28e973963a10) |
|
Fixed the blocker plus the P1s from the bot reviews on this head. Blocker — Also fixed
Not done, deliberately — deterministic KG entity IDs and the |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1aaaaff72a
ℹ️ 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".
| // Mirrors the lane + endpoint localAgent.ts uses for its agent loop. This is not a | ||
| // chat turn, so it selects the managed structured lane explicitly rather than the | ||
| // conversational chat-agent lane (backend routers/desktop_chat.py _managed_lane_id). | ||
| const AGENT_MODEL = 'omi-structured' |
There was a problem hiding this comment.
When gateway routing is disabled or an Anthropic BYOK key forces the direct path, this new omi-structured model reaches desktop_chat._request, whose _MODEL_ROUTES allowlist does not contain that alias, so the Windows action planner receives HTTP 400 and does not invoke its fallback. Fresh evidence beyond the earlier managed-alias comment is that the final diff now switches both the planner and local-agent callers to this newly added structured alias; normalize it to a supported direct model when the structured gateway lane is unavailable.
Useful? React with 👍 / 👎.
…tion Same defect as the memories router: the entitlement gate imported utils.subscription at module scope, which pulls database.user_usage in at import time and broke the minimal dependency graph that test_conversation_events_bounds.py and test_conversation_search_date_validation.py build. The import now sits beside the LLM helper's deferred import with the reason recorded. Verified: BACKEND_UNIT_TEST_FILE_LIST covering test_conversation_events_bounds.py, test_conversation_search_date_validation.py and test_conversation_topic_route.py via backend/test.sh — 9 + 23 + 3 passed. Failure-Class: none Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: dbb6faffb9
ℹ️ 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".
| their own prompts and inventing memories via Anthropic Haiku chat completions, then | ||
| persist through the normal memory/task write APIs. | ||
| """ | ||
| from utils.llm import connector_synthesis |
There was a problem hiding this comment.
Beyond the already-commented memories endpoint, the new connector, AI-profile, and conversation-topic handlers still import their LLM helpers inside the request function. If one of these dependencies is missing or develops an import-time failure, the backend can pass startup validation and only return 500 when a user first invokes that route; move these dependencies to module scope and adjust the isolation tests instead of hiding the serving dependency.
AGENTS.md reference: backend/AGENTS.md:L179-L187
Useful? React with 👍 / 👎.
| uid, | ||
| body.text, | ||
| user_name=user_name, | ||
| load_existing_from_db=body.include_existing, |
There was a problem hiding this comment.
When a caller sets include_existing=true, this dispatches the entire helper to the six-worker LLM pool, where extract_kg_from_text first performs the blocking Firestore get_knowledge_nodes read before invoking the provider. Concurrent slow database reads can therefore occupy every LLM worker and delay unrelated model work; load the nodes with run_blocking(db_executor, ...) before this call and pass them as existing_nodes.
AGENTS.md reference: backend/AGENTS.md:L289-L292
Useful? React with 👍 / 👎.
| let memoryStrings = synthesis.memories.filter { | ||
| !$0.trimmingCharacters(in: .whitespacesAndNewlines).isEmpty | ||
| } | ||
| let profileSummary = parsed["profile"] as? String ?? "" | ||
| let profileSummary = synthesis.profile |
There was a problem hiding this comment.
When the shared notes prompt classifies an explicit commitment into synthesis.tasks as instructed, this consumer reads only memories and profile, so the commitment never reaches the task store. The Windows Sticky Notes consumer drops the same field and can even report no-new-memories when a task is the only result; unlike the previous notes prompt, which had no task output and asked for plans among memories, the new contract therefore silently loses actionable note content.
Useful? React with 👍 / 👎.
|
|
||
| class ExtractKnowledgeGraphRequest(BaseModel): | ||
| text: str = Field(..., min_length=1, max_length=100_000) | ||
| user_name: Optional[str] = None |
There was a problem hiding this comment.
When an authenticated caller supplies a very large user_name, this field bypasses the request's 100,000-character text limit and is interpolated into the extraction prompt multiple times. A multi-megabyte value can consume substantial memory, occupy an LLM worker, and produce an oversized provider request even though the actual extraction text is bounded; apply a small max_length consistent with user-name fields elsewhere.
Useful? React with 👍 / 👎.
Git-on-my-level
removed
the
needs-tests
…sts (#11360) ## Summary Reverts the two OpenRouter PRs: #11283 (managed product text → OpenRouter Luna) and #11284 (dynamic OpenRouter model catalog). **No `OPENROUTER_API_KEY` is provisioned.** As merged, every managed product-text feature — chat, memories, knowledge graph, conversation processing, goals, notifications, wrapped — resolves to an OpenRouter route and would fail at the provider, and `llm_gateway/routers/health.py` requires that key before reporting ready, so the gateway would never pass readiness. Routing config returns exactly to its pre-#11283 state (`git diff` against the commit before that merge is empty for `model_config.py`, `llm_gateway/config/`, `health.py` and `clients.py`). ## Kept The SSOT work from #11286 and #11325 stays: `/v1/knowledge-graph/extract`, `/v1/memories/extract`, `/v1/connectors/synthesize`, `/v1/conversations/topic`, `/v1/users/ai-profile/synthesize` and deterministic KG ids. Those route through `get_llm(feature)`, so they follow whatever provider `model_config` names — now direct OpenAI/Anthropic again — and keep working. ## Product invariants affected - INV-AGENT-* - INV-CHAT-1 - INV-MEM-1 ## Failure class (fixes) Failure-Class: none ## Test plan - [x] `backend/test.sh` over the 41 gateway/qos/openrouter/SSOT-endpoint test files — all pass file-by-file (one pre-existing fast-unit CPU-time guard trip on `test_llm_gateway_deploy_contract.py`, 13/13 assertions pass, unrelated to this diff) - [x] Routing config byte-identical to pre-#11283 for `model_config.py`, `llm_gateway/config/`, `health.py`, `clients.py` - [ ] Dev backend deploy reports ready without `OPENROUTER_API_KEY` <!-- This is an auto-generated description by cubic. --> <a href="https://cubic.dev/pr/BasedHardware/omi/pull/11360?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. --> <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **High Risk** > Touches core LLM routing, gateway provider execution, readiness, and BYOK behavior for all managed text features; wrong config would break chat and background LLM workloads at scale. > > **Overview** > **Reverts managed product text routing from OpenRouter back to direct providers** so the stack can run without `OPENROUTER_API_KEY` and gateway `/ready` no longer blocks on that credential. > > `model_config` and gateway **generated route overrides** again send most features to **direct OpenAI** (`gpt-5.6-luna` / `gpt-5-nano`), **Gemini** for former flash-lite workloads, **Anthropic** for `chat_agent`, and **OpenRouter** only for `wrapped_analysis`. Inventory, route artifacts, and cost cards are aligned with that map; OpenRouter Luna/nano rate cards are removed. > > The gateway **executor** drops OpenRouter-specific request shaping: no BYOK vendor remapping on OpenRouter routes, no OpenRouter completion clamp, and GPT-5.6 sanitization applies only to `openai` provider refs. **Health** reports `managed_chat_provider: openai` and requires `OPENAI_API_KEY` instead of OpenRouter. > > **Deleted** the dynamic OpenRouter model catalog and shared vendor-prefix helpers; synthetics/replay harnesses register **`openai`** fakes directly again. QoS and gateway unit/integration tests are updated to match the pre–OpenRouter managed-text expectations. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 3b0aa79. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY -->
![https://www.cubic.dev/buttons/review-in-cubic-dark.svg"><source](https://www.cubic.dev/buttons/review-in-cubic-dark.svg"><source){kind=link}
![https://www.cubic.dev/buttons/review-in-cubic-light.svg"><img](https://www.cubic.dev/buttons/review-in-cubic-light.svg"><img){kind=link}
![https://www.cubic.dev/buttons/review-in-cubic-dark.svg"></picture></a](https://www.cubic.dev/buttons/review-in-cubic-dark.svg"></picture></a){kind=link}
2 participants
Summary
POST /v1/knowledge-graph/extract,POST /v1/memories/extract,POST /v1/connectors/synthesizeandPOST /v1/conversations/topicthrough managed Luna features (knowledge_graph/memories/conv_structure).save_knowledge_graphdiscovery_text, Win local KG synthesis, and Mac/Win memory-log import onto those endpoints instead of inventing graphs/memories via Haiku chat completions./v1/connectors/synthesizeinstead of building local Haiku synthesis prompts./v1/conversations/topic; drop the now-deadModelQoS.Claude.synthesistier.POST /v1/users/ai-profile/synthesize(both prompt stages + consolidation backend-side) and move Mac (direct Gemini) and Windows (Haiku) AI-profile generation onto it.openaikey.omi-structuredon desktop chat and route it toomi:auto:chat-structured, moving the Windows automation planner and local-agent loop off their explicit Haiku model id.app/lib/backend/http/openai.dart) and its now-unusedOPENAI_API_KEYenv plumbing.Review fixes on this head
save_knowledge_graph: extract only for a non-empty trimmeddiscovery_text, and fall back to supplied nodes/edges when extraction fails, so the compatibility path stays usable.run_blocking(llm_executor, ...), and refuse a paywalled account (402) before the provider call; KG extract usesstrict_parse=True; the memory-log request cap matches the helper's 40k budget.chat_structured+ its own route instead ofchat_agent.save_knowledge_graphforwardsctx.signaland moves to thelongrelay timeout class; long Sticky Note lines are chunked instead of truncated; the graph-extract fallback emits a structured degraded record.Product invariants affected
Failure class (fixes)
Failure-Class: none
Test plan
backend/.venv/bin/python -m pytest tests/unit/test_knowledge_graph_extract_route.py tests/unit/test_memories_extract_route.py tests/unit/test_memories_extract_helper.py tests/unit/test_rate_limiting.py::TestRouterPolicyMapping/v1/memories/extractsave_knowledge_graphwithdiscovery_texthits/v1/knowledge-graph/extract/v2/chat/completions)backend/.venv/bin/python -m pytest tests/unit/test_connector_synthesis_helper.py tests/unit/test_connector_synthesis_route.py tests/unit/test_rate_limiting.py tests/unit/test_route_policy_inventory.py tests/unit/test_desktop_rest_inventory.py tests/unit/test_app_client_schema_inventory.py tests/unit/test_inventory_whitelist_honesty.py— 124 passeddesktop/windowsvitest: calendarExtract, gmailExtract, stickyNotesImport — 12 passed;tsc --noEmit -p tsconfig.web.jsoncleanxcrun swift build -c debug --package-path Desktop— Build complete;xctest -XCTest ConnectorSynthesisResponseTests— 2 passedbackend/.venv/bin/python -m pytest tests/unit/test_conversation_topic_helper.py tests/unit/test_conversation_topic_route.py tests/unit/test_rate_limiting.py— 74 passedvitest run conversationTopic.test.ts— 2 passed;xctest -XCTest ModelQoSTests— 16 passed/v1/connectors/synthesizebackend/.venv/bin/python -m pytest tests/unit/test_ai_user_profile_helper.py tests/unit/test_ai_user_profile_route.py— 9 passedvitest run src/main/assistants/aiUserProfile— 28 passed;xctest -XCTest AIUserProfileSynthesisResponseTests— 2 passed/v1/conversations/topicbackend/.venv/bin/python -m pytest tests/unit/test_desktop_chat.py— 58 passed;vitest run src/renderer/src/lib src/main/assistants— 1954 passed/v1/users/ai-profile/synthesizepyteston the ten helper/route suites +test_rate_limiting.py+test_memories_create.py— 129 passed;test_desktop_chat.py— 59 passed;vitest run src/main/agentKernel src/renderer/src/lib src/main/assistants— 2505 passed;swift build --build-tests+ 20 Swift tests — 0 failuresNote
Medium Risk
Large cross-surface change (new LLM routes, paywall gates, desktop chat lane accounting, and client import paths); mitigated by return-only contracts, rate limits, and extensive unit tests, but end-to-end desktop flows still need manual verification.
Overview
Moves desktop “invent via Haiku/Gemini” LLM work onto return-only backend routes backed by managed Luna features, with clients persisting through existing write APIs.
New Firebase-authenticated endpoints (rate limits, trial paywall 402,
llm_executor, 502 on failure):POST /v1/knowledge-graph/extract,/v1/memories/extract,/v1/connectors/synthesize,/v1/conversations/topic, and/v1/users/ai-profile/synthesize. Prompts live in newutils/llm/*helpers; untrusted user text is sent in a separate human turn from the system contract.Desktop/macOS stops local synthesis prompts for calendar/gmail/notes imports, memory-log onboarding, AI user profile (was Gemini + local two-stage prompts), and routes
save_knowledge_graphthrough backend extract whendiscovery_textis set (falls back to suppliednodes/edgesif extract fails).ModelQoS.Claude.synthesisis removed; managed synthesis calls can use a longer POST timeout.Desktop chat gateway accepts
omi-structured/omi:auto:chat-structuredand attributes that lane aschat_structured(notchat_agent). Session-title output budget is keyed by feature, not provider.Mobile app drops dead direct OpenAI client code and
OPENAI_API_KEYenv wiring.Policy inventory, OpenAPI export, LLM surface inventory, and gateway smoke/replay fakes are updated; broad unit test coverage added for helpers and routes.
Reviewed by Cursor Bugbot for commit 1aaaaff. Configure here.