fix(intelligence): replace Workers AI model deprecated 2026-05-30 - #290
Conversation
`@cf/meta/llama-3.1-8b-instruct` aliases `@cf/meta/infire-llama-3.1-8b-instruct`,
which Cloudflare deprecated on 2026-05-30. Every call now fails with error 5028,
verified live against production:
POST connect.chitty.cc/api/thirdparty/cloudflare/ai/run
{"error":"5028: @cf/meta/infire-llama-3.1-8b-instruct was deprecated on
2026-05-30. See the model catalog for alternatives: ..."}
Six intelligence-layer call sites hardcoded that id, so each has been silently
degraded for three months — the failures land in catch blocks that log a warning
and return a placeholder. The visible symptom was MemoryCloude returning
"Failed to generate summary." for every session, but anomaly detection, task
decomposition, synthesis, relationship summaries and intent refinement were all
dead the same way.
Route model selection through a shared `resolveAiModel(env)` (env.AI_MODEL_PRIMARY,
defaulting to @cf/meta/llama-4-scout-17b-16e-instruct) so the next deprecation is
an env var rather than a code change. This generalises the pattern already used at
src/api/routes/prompts.js and adopts it everywhere.
Read generated text through `extractAiText()`. The current envelope carries the
text both as top-level `response` and as `choices[0].message.content`; reading
both means a model that drops either shape cannot degrade to `undefined`.
MemoryCloude now throws rather than caching an empty summary, keeping the failure
loud instead of persisting "" into KV for 90 days.
Replacement models verified live through the same route — both returned "ok":
@cf/meta/llama-4-scout-17b-16e-instruct
@cf/meta/llama-3.3-70b-instruct-fp8-fast
Tests exercise the parser against verbatim recorded production envelopes, not
hand-written fixtures.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DZbwet4A5CENYvuS1KAbSX
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Warning Review limit reachedNext included review available in 44 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (11)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
🟡 Changes recommended
Two small parsing/caching edge cases should be fixed to ensure fallback text extraction works when response is present-but-empty and to prevent caching whitespace-only summaries.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Fixes intelligence-layer Workers AI failures by removing reliance on the deprecated @cf/meta/llama-3.1-8b-instruct model and centralizing model selection + response-text extraction.
Changes:
- Introduces
src/lib/ai-model.jsto resolve the Workers AI model viaAI_MODEL_PRIMARY(with a safe default) and to extract generated text from multiple envelope shapes. - Updates all impacted Workers AI call sites (intelligence + routes) to use the shared resolver/extractor instead of hardcoded model IDs and direct
.responsereads. - Adds Vitest coverage for model resolution and envelope parsing using recorded production envelopes.
File summaries
| File | Description |
|---|---|
| tests/lib/ai-model.test.js | Adds tests for model resolution defaults/overrides and robust text extraction across envelope shapes. |
| src/lib/ai-model.js | New shared resolver (resolveAiModel) and envelope parser (extractAiText) to avoid hardcoded model IDs and brittle response reads. |
| src/intelligence/relationship-engine.js | Switches to resolved model and shared text extractor for relationship summaries. |
| src/intelligence/memory-cloude.js | Switches to resolved model + extractor and avoids caching empty AI summaries. |
| src/intelligence/intent-predictor.js | Switches to resolved model and shared text extractor before JSON extraction. |
| src/intelligence/context-consciousness.js | Switches to resolved model and parses JSON from extracted text. |
| src/intelligence/cognitive-coordination.js | Switches to resolved model and parses JSON from extracted text in multiple AI paths. |
| src/api/routes/prompts.js | Routes prompt execution through shared model resolver and extractor. |
| src/api/routes/mcp.js | Updates MCP sampling default model and response extraction via shared utilities. |
Review details
- Files reviewed: 9/9 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
…uards Two separated-review findings, plus the same defect at a third site the review did not name. All three are the silent-failure class this PR exists to close, so leaving any of them would have shipped a guard that looks like it holds and does not. 1. extractAiText returned `response` even when it was empty, so an envelope carrying `response: ""` alongside real `choices` text returned "" — discarding text the model did generate. `response` is now preferred only when it carries non-whitespace text; otherwise the choices content wins. When neither half has text the original blank `response` is still returned, so the "never undefined" contract holds. 2. memory-cloude checked `if (!summary)`, which passes " " through. A whitespace-only summary would have been cached in KV and stood as a valid summary for the full 90-day retention — precisely the silent failure the guard was added to prevent. Now checks `.trim()`. 3. relationship-engine had `extractAiText(response) || null` at the same kind of boundary, with the same whitespace hole, and was not flagged. Fixed identically; a blank summary now surfaces as absent rather than as relationship intelligence. Each fix verified by reverting it alone and confirming exactly one test fails. Tests exercise the real MemoryCloude and RelationshipEngine classes through their real code paths — the KV and AI bindings are runtime binding stand-ins in the file's existing style, not module mocks. 571 passed / 1 skipped / 0 failed; eslint 0 errors. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Nz8PYvgzrfpm9hM86vSWBc
# Conflicts: # tests/intelligence/memory-cloude.test.js
|
Post-merge verification: this merged but did not deploy. Production is still running pre-#290 code. Ran the verification this PR specified. Summaries still return That alone is ambiguous — it could be an undeployed merge or a bad fix — so I probed a route that distinguishes them. The deployed worker is still reaching for the dead model by default. That code path only exists before this PR, so the code serving Corroborating, not conclusive on its own: Why it did not deploy: there is no deploy workflow in Consequence worth stating plainly: every intelligence-layer AI path in this repo remains dead in production, and will stay dead after any further merge, until the deploy path is repaired. That includes #296. Merging is currently not sufficient to ship. |
) * fix(intelligence): replace Workers AI model deprecated 2026-05-30 `@cf/meta/llama-3.1-8b-instruct` aliases `@cf/meta/infire-llama-3.1-8b-instruct`, which Cloudflare deprecated on 2026-05-30. Every call now fails with error 5028, verified live against production: POST connect.chitty.cc/api/thirdparty/cloudflare/ai/run {"error":"5028: @cf/meta/infire-llama-3.1-8b-instruct was deprecated on 2026-05-30. See the model catalog for alternatives: ..."} Six intelligence-layer call sites hardcoded that id, so each has been silently degraded for three months — the failures land in catch blocks that log a warning and return a placeholder. The visible symptom was MemoryCloude returning "Failed to generate summary." for every session, but anomaly detection, task decomposition, synthesis, relationship summaries and intent refinement were all dead the same way. Route model selection through a shared `resolveAiModel(env)` (env.AI_MODEL_PRIMARY, defaulting to @cf/meta/llama-4-scout-17b-16e-instruct) so the next deprecation is an env var rather than a code change. This generalises the pattern already used at src/api/routes/prompts.js and adopts it everywhere. Read generated text through `extractAiText()`. The current envelope carries the text both as top-level `response` and as `choices[0].message.content`; reading both means a model that drops either shape cannot degrade to `undefined`. MemoryCloude now throws rather than caching an empty summary, keeping the failure loud instead of persisting "" into KV for 90 days. Replacement models verified live through the same route — both returned "ok": @cf/meta/llama-4-scout-17b-16e-instruct @cf/meta/llama-3.3-70b-instruct-fp8-fast Tests exercise the parser against verbatim recorded production envelopes, not hand-written fixtures. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DZbwet4A5CENYvuS1KAbSX * fix(intelligence): close the whitespace holes in the blank-envelope guards Two separated-review findings, plus the same defect at a third site the review did not name. All three are the silent-failure class this PR exists to close, so leaving any of them would have shipped a guard that looks like it holds and does not. 1. extractAiText returned `response` even when it was empty, so an envelope carrying `response: ""` alongside real `choices` text returned "" — discarding text the model did generate. `response` is now preferred only when it carries non-whitespace text; otherwise the choices content wins. When neither half has text the original blank `response` is still returned, so the "never undefined" contract holds. 2. memory-cloude checked `if (!summary)`, which passes " " through. A whitespace-only summary would have been cached in KV and stood as a valid summary for the full 90-day retention — precisely the silent failure the guard was added to prevent. Now checks `.trim()`. 3. relationship-engine had `extractAiText(response) || null` at the same kind of boundary, with the same whitespace hole, and was not flagged. Fixed identically; a blank summary now surfaces as absent rather than as relationship intelligence. Each fix verified by reverting it alone and confirming exactly one test fails. Tests exercise the real MemoryCloude and RelationshipEngine classes through their real code paths — the KV and AI bindings are runtime binding stand-ins in the file's existing style, not module mocks. 571 passed / 1 skipped / 0 failed; eslint 0 errors. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Nz8PYvgzrfpm9hM86vSWBc * fix(memory): scope AI Search per synthetic entity; drop federation Implements the memory instance model decided 2026-09-05 — one AI Search instance per primary synthetic entity — and fixes three defects found on the way. 1. DEAD FLAG. persistInteraction gated indexing on `this.hasVectorize`, which is assigned nowhere; initialize() only ever sets `this.hasAiSearch`. The flag was permanently undefined, so storeEmbedding never ran even with a working binding. Uncommenting the wrangler binding would NOT have produced a single embedding. 2. FEDERATION. semanticRecall passed `instance_ids: ["memory-cloude", "context-embeddings"]`, fanning out across instances. chittysearch's CHARTER forbids this outright: one request would synthesise across unrelated scopes. Now queries exactly one instance. 3. FILTER-AS-BOUNDARY. Session isolation relied on a post-hoc `chunk.item.metadata.sessionId === sessionId` filter. A caller-supplied filter is not a boundary — it fails open the moment it is omitted. The filter is kept ONLY as ranking: every chunk it now sees already belongs to this entity, so dropping it would widen recall within the entity, never across entities. Scoping model: instance `memory-{entityChittyId}`, resolved by memoryInstanceFor(). Between entities is a real trust boundary and gets its own instance. Between sessions of one entity is the product — 90-day continuity — not a boundary. Fails closed throughout. No resolvable entity means: skip the index write, and fall back to KV keyword recall, which reads `session:{id}:*` and therefore cannot cross an entity boundary. It never falls back to a shared instance, because an unscoped write is precisely how one entity's context becomes another's recall. Tests assert the boundary rather than the happy path — two entities must not share an instance, a non-string id must not coerce into `memory-[object Object]`, and the unscoped recall test installs a searchNamespace whose get() throws, so it fails if the semantic path is ever reached without an entity. Full suite 576 passed / 1 skipped; lint 0 errors. Stacked on fix/workers-ai-deprecated-model (PR #290) — same file, different regions. Merge #290 first. Refs: chittysearch docs/NAMESPACE-STRATEGY.md rules 1 and 3 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DZbwet4A5CENYvuS1KAbSX * fix(memory): validate entity ids; derive hasAiSearch before initialize() Separated adversarial review (sonnet-4-5, different host+model) on the entity scoping change. Three findings taken, two rejected with reasons. TAKEN — legacy shared instances were reachable from caller input. entityId is attacker-influenceable (it arrives on request-shaped objects), and `memory-${entity}` was built from it unvalidated. entityId "cloude" resolved to `memory-cloude`, the pre-scoping everyone-bucket — a one-word payload back into shared memory. Now validated against /^[A-Za-z0-9][A-Za-z0-9_-]{2,63}$/ with a denylist for the known shared instances. The `memory-` prefix already made an evidence instance unreachable; it did nothing about a sibling one. TAKEN — hasAiSearch was set only in initialize(), and src/index.js calls `.initialize().catch(...)` WITHOUT awaiting it. A request arriving before that resolves saw undefined and silently skipped indexing. That is the same silent-skip that left storeEmbedding dead behind `this.hasVectorize` — fixed one layer up and reintroduced one layer down. It is a synchronous derivation from env, so it now happens in the constructor and initialize() only logs. TAKEN — the tests could not fail if isolation broke. Added: legacy instances are unreachable by name, non-identifier entity ids are rejected, and hasAiSearch is true before initialize() is awaited. REJECTED — "attacker reaches chittyevidence-arias-2024d007847 via collision." The `memory-` prefix is unconditional, so no entity id produces an evidence instance name. The sibling-instance concern was the real one and is fixed above. REJECTED as out of scope, and documented instead — "authenticate that options.entityId matches the request principal." MemoryCloude is a library, not an edge; it cannot know the caller's principal. Correct layering is that routes pass an entity from authenticated context. Rather than leave that implicit, the resolver's contract now says so: THIS SCOPES; IT DOES NOT AUTHORIZE. Validation bounds the damage of a caller mistake and is not a substitute for authorization at the edge. Full suite 601 passed / 1 skipped. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DZbwet4A5CENYvuS1KAbSX --------- Co-authored-by: NB <nb@chitty.cc> Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
What broke
@cf/meta/llama-3.1-8b-instructaliases@cf/meta/infire-llama-3.1-8b-instruct, which Cloudflare deprecated on 2026-05-30. Every call to it now fails with error 5028. Six intelligence-layer call sites hardcoded that id, so each has been silently dead for three months — the failures land incatchblocks that log a warning and return a placeholder, so nothing surfaced.Found while chasing why MemoryCloude returned
"Failed to generate summary."for every one of 248 recovered session records. Summarization was only the visible symptom; anomaly detection, task decomposition, cognitive synthesis, relationship summaries and intent refinement were all failing the same way.Live evidence
Probed through the deployed diagnostic route (
POST connect.chitty.cc/api/thirdparty/cloudflare/ai/run):Context length was ruled out first: session
session-1777931666-130901has exactly one interaction and failed identically.Both replacement candidates verified live through the same route, both returning
"ok":@cf/meta/llama-4-scout-17b-16e-instruct@cf/meta/llama-3.3-70b-instruct-fp8-fastThe change
src/lib/ai-model.js(new) —resolveAiModel(env)readsenv.AI_MODEL_PRIMARY, defaulting to@cf/meta/llama-4-scout-17b-16e-instruct. This generalises the pattern already live atsrc/api/routes/prompts.js:359and adopts it at all seven sites, so the next deprecation is a config value rather than a code change.Which model production will actually use: this diff makes
AI_MODEL_PRIMARYload-bearing at six sites that previously ignored it, so its current value matters. It is already declared in all threewrangler.jsoncenv blocks (:114,:225,:371) as@cf/meta/llama-4-scout-17b-16e-instruct— identical to the new default, and the exact model verified live above. So the six sites converge on a model proven working, not on an untested one. Caveat: that is the value in the repo config; a dashboard-side override would not be visible here.extractAiText(result)— the current envelope carries the generated text both as top-levelresponseand aschoices[0].message.content. Reading both means a model that drops either shape cannot degrade toundefined. It returns"", neverundefined.memory-cloude.jsnow throws on an empty summary instead of caching it. Persisting""into KV for 90 days would convert a loud failure into a silent one.Seven call sites changed:
memory-cloude,context-consciousness,cognitive-coordination(×2),relationship-engine,intent-predictor,mcp.jssampling default, plusprompts.jsrouted through the shared resolver. The twoAI.runsites left untouched are correct as-is —thirdparty.js:600takes its model from the request body (that's the diagnostic probe), andmcp.js:469consumes the resolved variable.Validation
npx vitest run— 563 passed, 1 skipped, 0 failed (full suite)npm run lint— 0 errors (30 pre-existingno-unused-varswarnings, none in changed files)tests/lib/ai-model.test.jsare the actual bytes Workers AI returned on 2026-09-04.prettier -cflagsprompts.jsandmemory-cloude.js, but they fail identically onorigin/main— pre-existing drift, deliberately not reformatted to keep the diff at 32 lines instead of ~500.Separated adversarial review
Reviewed by
claude-sonnet-4-5on a different host and provider (reviewer ≠ implementer). It raised two CRITICALs; both were refuted against the source, and the refutations are the reason no code changed:JSON.parse('')crashes at 3–4 sites." All three sit insidetry/catchwith fallbacks (context-consciousnesstry@186/catch@215;cognitive-coordinationtry@351/catch@390 and try@473/catch@518). More decisively, the old code wasJSON.parse(response.response)— on an empty envelope that parsedundefinedand threw the sameSyntaxErrorinto the same catch. No new crash path.AI.runinsrc/— nine total, seven changed, two correct untouched (above).Its remaining points (
extractAiText(...) || nullinrelationship-engine,resolveAiModelenv-safety) describe deliberate preservation of existing contracts. Its request for mocked integration tests was declined: addingvi.mock-style coverage is barred by the repo's no-mocks rule.CI
All 23 required checks pass — Lint (20, 22), Test (20, 22), Build, CodeQL/Analyze ×5, Security Scan, governance, gates, compliance, identity-onboarding, pr-validation.
Not in this PR — deploy is a human gate
Merging deploys via Workers Builds, so auto-merge is deliberately not enabled. Post-deploy verification should be: re-probe the sampling route, then
GET /memory/session/:sessionIdon any of the 248 recovered records and confirm a real summary replaces"Failed to generate summary."The second half of the MemoryCloude defect is not addressed here:
hasVectorize: falsebecauseai_search_namespacesis commented out in all threewrangler.jsoncenv blocks (:140,:278,:417), with no commit explaining why. So the 248 records were never embedded and are not semantically searchable. That needs an account-side namespace check plus a backfill decision, and is filed separately.🤖 Generated with Claude Code
https://claude.ai/code/session_01DZbwet4A5CENYvuS1KAbSX