Repository navigation
release: 0.0.103 — memory/Load-Anyway fix + on-device TTS/vision/tools hardening + promote tooling - #506
Merged
Conversation
The override survival floor refused loads on a PREDICTION computed before eviction (availMB + freedByEvict - incomingDirty), crediting 0 for evicting a clean/mmap model - so it under-counted the RAM iOS reclaims on unload and refused big models the device can actually hold. That is why "Load Anyway" still failed and why "load a small model, wait, then load the big one" worked (users defeating the stale estimate). Regression from 8034754 (2026-07-06). Fix: under override, evict first, then refreshMemoryInfo() + re-read real free RAM, and apply the 1200MB survival floor to the true post-eviction number. The physics guard is preserved (still refuses if genuinely below floor after freeing everything) - it just stops refusing loads that would succeed. Test models real iOS reclaim (free RAM rises after the unload) so it fails on the old predict-then-refuse code and passes on evict-then-measure - the static mock the old tests used structurally could not catch this timing bug. 70/70 pass. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… fix Android versionName leak The pipeline could CUT a beta (uat.sh) but had no way to BLESS a good cut to production, and Android baked "-beta.N" into versionName — frozen into the AAB, user-visible, blocking Play's promote-as-is (internal→production, same bytes). - uat.sh: Android versionName = the plain production version (no -beta suffix), matching iOS's MARKETING_VERSION. The -beta.N label now lives only in the git tag / GitHub prerelease / store notes, never in the shipped binary. Play orders by versionCode (the timestamp), not versionName, so this is safe. Stale header comment corrected (the "-beta versionName" was a misconception, not a Play rule). - scripts/promote.sh <beta-tag>: reuses the EXACT tested bytes — bumps+tags the repo, promotes the Play internal build to production (draft), attaches the TestFlight build to a new App Store version, and cuts a clean vX.Y.Z GitHub release from the beta's APK. No rebuild anywhere. Asserts the beta commit is on main first. Two final gates stay MANUAL by design (Play rollout %, ASC submit). - fastlane: android promote (track_promote_to, no re-upload) + ios promote (skip_binary_upload, attach existing build) lanes. - version math extracted to scripts/lib/version.js (single source of truth for cut + promote), typed via version.d.ts, 26 tests covering every derivation + rejection path. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR adds shared release-promotion tooling, revises model-loading and residency override handling, updates onboarding and audio/reasoning flows, and extends mmproj repair/download metadata plus retry and progress behavior. ChangesRelease Promotion Tooling
Model Residency and Load Override Flow
Reasoning and Streaming Support
Mmproj Repair Download Flow
Onboarding Wednesday Link Update
Repository Metadata and Docs
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
PR Summary by QodoRelease 0.0.103: fix override RAM check + add promote-to-production tooling
AI Description
Diagram
High-Level Assessment
Files changed (8)
|
Code Review by Qodo
Context used✅ Compliance rules (platform):
36 rules 1.
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@fastlane/Fastfile`:
- Around line 227-247: The promote lane currently promotes the entire internal
track without pinning a specific build, so it may ship a newer internal artifact
than the one tested. Update the Fastfile’s :promote lane to accept and use the
tested build’s version_code, and pass that value into upload_to_play_store
alongside track_promote_to so the promotion targets the exact build. Wire the
version_code through from the existing build-capture flow (for example the
cut-time metadata used by uat.sh/beta tagging) and keep the current draft
release behavior intact.
In `@scripts/promote.sh`:
- Around line 105-107: The NOTES_FILE assignment in promote.sh is using a
different path than the one mktemp actually creates, so the script writes and
cleans up the wrong file. Update the NOTES_FILE setup and the release-notes
write path so they reference the same actual temporary file created by mktemp,
and ensure the later rm cleanup targets that exact file.
- Around line 67-78: The idempotency check in promote.sh only looks for the
local v${TARGET_VERSION} tag, so a failed git push can leave the commit/tag
unpushed while later reruns skip the release steps. Update the logic around the
tag/commit block to verify the tag exists on origin as well, or make the flow
always attempt to push the existing commit/tag before continuing, using the same
promote.sh tag creation and git push path. Ensure the check distinguishes
“already released remotely” from “tag exists only locally” so reruns don’t
bypass the push.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 5bbc2c64-8396-4978-9b15-8dff7b7dfeeb
📒 Files selected for processing (8)
__tests__/unit/scripts/version.test.ts__tests__/unit/services/modelResidency.test.tsfastlane/Fastfilescripts/lib/version.d.tsscripts/lib/version.jsscripts/promote.shscripts/uat.shsrc/services/modelResidency/index.ts
Drop the /hire-ai-native-mobile-squad path (and UTM params) so the "made with ♥ by Wednesday" link just goes to https://wednesday.is. Also collapses the DRY break: OnboardingScreen hardcoded its own copy of the URL instead of the WEDNESDAY_URL constant — now imports and uses the single source of truth. Test asserts against the imported constant. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@__tests__/rntl/screens/OnboardingScreen.test.tsx`:
- Around line 184-214: The onboarding scroll test in OnboardingScreen.test.tsx
no longer asserts the real state change, so restore coverage for the
`onMomentumScrollEnd` -> `setCurrentIndex` flow in `OnboardingScreen`. Use the
existing render setup and the `onboarding-screen` testID to trigger a scroll
event, then verify the expected current slide/index change with a meaningful
expectation instead of just grabbing the FlatList child reference. Keep the
existing `show onboarding-skip` and `show onboarding-next` checks separate, and
make sure this test would fail if the scroll/index update logic were broken.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 7c0e3742-6fff-4284-a773-d46090ea06c1
📒 Files selected for processing (3)
__tests__/rntl/screens/OnboardingScreen.test.tsxsrc/constants/index.tssrc/screens/OnboardingScreen.tsx
Drives the real seam end to end — loadModelWithOverride (UI helper) →
activeModelService.loadTextModel({override}) → makeRoomFor — mocking only
the native LLM engine and the hardware RAM sensor. Asserts the outcome the
user feels (model loaded / refused, which dialog shows), not gate calls.
The headline case reproduces the shipped bug via a DYNAMIC RAM mock: free
RAM is below the survival floor before the clean victim's eviction and
above it after (modelling iOS reclaiming the unloaded mmap pages). It goes
RED on origin/main (predictive floor credits 0 for a clean eviction →
false refusal → user sees "Error") and GREEN on the fix (evict-then-
measure → loads). Verified red on main by swapping in main's makeRoomFor.
Corners covered: overridable first-refusal surfaces the Load-Anyway prompt
without evicting; the genuine hard limit refuses once (no infinite offer);
the session override skips the gate on reload.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
__tests__/integration/models/loadAnywayOverrideChain.test.ts (1)
48-51: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueUpdate the setup note to match the mocked boundaries
The comment says onlyllmandhardwareare mocked, but this test also mockslitertandlocalDreamGenerator. Keep the description aligned with the actual boundary set.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@__tests__/integration/models/loadAnywayOverrideChain.test.ts` around lines 48 - 51, The setup note is out of sync with the test’s mocked dependencies, since loadAnywayOverrideChain.test.ts also mocks litert and localDreamGenerator in addition to llm and hardware. Update the surrounding description/comment to reflect the full mocked boundary set used by the test so it matches the actual imports being stubbed.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@__tests__/integration/models/loadAnywayOverrideChain.test.ts`:
- Around line 48-51: The setup note is out of sync with the test’s mocked
dependencies, since loadAnywayOverrideChain.test.ts also mocks litert and
localDreamGenerator in addition to llm and hardware. Update the surrounding
description/comment to reflect the full mocked boundary set used by the test so
it matches the actual imports being stubbed.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 19987dd3-ace9-4723-8fb5-24fc710a5ea9
📒 Files selected for processing (1)
__tests__/integration/models/loadAnywayOverrideChain.test.ts
Retrying a failed vision-model download on iOS re-fetched only the base GGUF and silently dropped the mmproj (vision projector), leaving a vision model registered as text-only. Cause: the download's mmProjDownloadUrl lives in metadataJson, which was handed to the native downloader but never written to the local store row (add() persisted mmProjFileName/Size but not metadataJson). metadataJson only reached the row via a hydrate from native — i.e. after an app restart — so a same-session retry/reconcile (restartIosTextDownload) found meta=null and reconstructed the job without the mmproj sidecar. iOS-only: it can't resume a foreground download so it rebuilds from scratch; Android resumes the existing mmproj row by id. Fix: persist metadataJson on the store row at add() time, exactly as it is sent to native. One field, no behaviour change elsewhere. Tests (red-first): parallelMmproj asserts the added row carries metadataJson.mmProjDownloadUrl (fails before the fix); textDownloadProvider guards that iOS retry rebuilds the main + mmproj sidecar from it, and that a plain text model rebuilds without a mmProjFile. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Drives the REAL chatStore with a dynamic sequence of reasoning-token appends,
builds the in-progress streaming message via the REAL getDisplayMessages, and
renders the REAL MessageAudioMode at each step. Asserts the displayed thinking
text reflects each increment ('' -> partial -> more), not only the final string.
Fails on the pre-fix pro code (thinking-block absent while streaming), passes
with the fix. Bumps the pro submodule pointer to the matching pro fix commit.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The "Repair Vision" action re-downloads a model's missing mmproj (~900MB) but showed only an indeterminate "Repairing" spinner, so a large file looked frozen. Route the repair download through the SAME download-store rows the normal download writes (add/updateProgress/setCompleted/remove), keyed on the model's modelKey, so the existing determinate progress bar lights up: - Extract performMmProjRepairDownload into download.ts (the owner of background-download orchestration). repairMmProj now delegates + persists. - CompletedDownloadCard renders the shared progress-bar markup from the live store row while a repair is in flight, reusing the same styles/tokens as ActiveDownloadCard (no fork). The "Vision Repaired" alert is unchanged. Tests (red-first, dynamic mock at the boundary): - repairVisionProgress: drives the real useDownloadStore, asserts progress advances 0 -> mid -> complete (not terminal-only) and fails through the store on error. Verified red when updateProgress is removed. - CompletedDownloadCardRepair (RNTL): the determinate bar renders and its bytes advance as the store advances; spinner-only before the row is seeded. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Chat pre-checked with the predictive checkMemoryForModel (fileSize×1.5), gating models the MEASURED residency loader (makeRoomFor) accepts — so a model loadable from Home was blocked in chat. Remove the predictive gate; the measured loader is the single authoritative gate and its OverridableMemoryError drives the SAME Load-Anyway affordance via the shared loadModelWithOverride helper. Parity with Home. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
On-device test session (Qwythos-9B vision + memory, 12GB iPhone/Android) surfaced OD1-OD8 (vision-drop-on-retry, repair-no-progress, chat/Home selector split, forced-load freeze, Android retry stall, Kokoro loop, reasoning-toggle detection, voice-mode thinking). Logged with evidence. Also lands the manual-signing ios-device.sh helper used to run + pull device logs. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…mplate signal (OD7) The in-chat thinking/reasoning toggle is gated by supportsNativeThinking(context), which returned context.isJinjaSupported() = "the model ships a parseable jinja template". A community reasoning model like empero-ai/Qwythos-9B-Claude-Mythos emits <think> reasoning the app already renders, but its merged chat template minja cannot flag, so isJinjaSupported() was false and the toggle never appeared - even though the runtime parseThinkingContent renders its reasoning fine. Reasoning was detected two disagreeing ways. Make the load-time capability derive from the SAME reasoning signal the runtime parser uses: the reasoning delimiters (<think>, Gemma <|channel>thought, Qwen <|channel|>analysis) in the model's own chat_template metadata. When the jinja signal is false, fall back to templateEmitsReasoning(chat_template) so a reasoning-capable model surfaces the toggle. No model-name hardcoding; the markers live once in messageContent.ts as the single source of truth. Tests (red-first): reasoning template with isJinjaSupported()=false -> true (reproduces OD7 Qwythos), plain template -> false, plus Gemma/Qwen/alt-key cases and direct templateEmitsReasoning coverage. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The OD3 agent died on a session limit before its test file was green (16 failures on full-file run, though each passed in isolation). Two harness bugs, no source change: - The refactor moved 'await waitForRenderFrame()' (a real 350ms frame) onto the refusal path, so fake-timer Load-Anyway tests hung awaiting a timer they never advanced. Run those on real timers + a flush helper. - InteractionManager.runAfterInteractions doesn't flush under jest fake timers → cross-test pollution timed out later real-timer tests. Stub it synchronously for every test; hard-reset timers + spies in afterEach; mockReset the loader so a stray *Once queue can't leak. 32/32 in this file; 184/184 across the OD-affected suites. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Core: red-first ttsStore test proving checkDownloadStatus re-hydrates a desynced engine from the persisted truth (and does NOT fabricate downloaded when the flag is false, nor re-hydrate mid-download). Bumps pro pointer to the reconcile fix. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Red-first regression tests for the streaming speak path (fix lives in the
pro submodule, branch fix/tts-strip-tool-calls). Feeds a <tool_call> block
that spans multiple sentence boundaries, interleaved with real answer text,
token-by-token, and asserts the OBSERVABLE SPOKEN OUTPUT (what reaches
engine.speak) never contains a tool-call fragment at ANY streaming step —
only the real prose. Covers: block across sentence boundaries; an unclosed
opener mid-stream (withheld, not spoken); a still-forming opener tag; a plain
answer with no tool call (unchanged); and the trailing finishStreamingText
flush. Fails against the pre-fix code (fragment leaks `{"name": …`), passes
after.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…OD13 (reasoning-format mismatch)
…/regression); threads fix makes 9B fast on both platforms
…OS Metal runs 9B well
…(OD14) A tool-using turn reasons, then emits a tool call. That first round's thinking was dropped — the intermediate tool-call assistant message was built with no reasoningContent (generationToolLoop.ts) and the live stream was cleared — so the thinking visibly vanished when the tool fired and only the post-tool-result round's thinking survived on the final answer. On device: 'for a moment it's just the tool calls and the thinking disappears'. Attach this round's accumulated state.reasoningContent to the tool-call assistant message, so each round's chain-of-thought persists on its own message and BOTH rounds are kept in the transcript (the cross-tool CoT demo). Red-first: new test drives round-1 reasoning via onStream and asserts the intermediate tool-call message carries it; proven to fail without the fix line, pass with it. 311 generation/tool-loop tests green. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Targeting id=$DEVICE_ID made xcodebuild block on the device being fully 'available' and fail with 'developer disk image could not be mounted' whenever the phone locked or the tunnel hiccupped mid-build. Compiling needs only the arm64 device slice; install/launch still reach the device via devicectl.
…er behavior
OD3 removed the predictive checkMemoryForModel pre-check from chat model
selection; it now routes through the measured residency loader (loadTextModel),
whose OverridableMemoryError drives the shared "Insufficient Memory / Load
Anyway" affordance. Updated the 6 stale tests (and 2 adjacent ones that were
only passing because a latent mock bug made the load throw "not a function" →
an Error alert) to assert the new behavior: measured loader invoked, refusal
surfaces the Insufficient Memory alert + Load Anyway, override retries with
{override:true}.
Also fixes a pre-existing latent mock-infra bug: the activeModelService mock
factory references mockLoadModel/mockUnloadModel, which are declared after the
hoisted jest.mock, so the load/unload keys held `undefined` until a manual
runtime patch. Bind them in beforeEach so every test routes load/unload through
the mocks (order-independent).
155/155 ChatScreen tests pass; eslint clean.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
… behavior Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The two dual-fork commits fixed the runtime mock-hoisting/TDZ bug and the OD3 assertions, but left mockLoadModel/mockUnloadModel typed as zero-arg jest.fn, so the delegating-arrow factory (...args) and the keyed mockImplementation refusal cases (id,_t,opts) failed tsc. Type both as (...args: any[]) — runtime unchanged, 155/155 still green, tsc clean.
alichherawalla
changed the title
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
__tests__/rntl/screens/ChatScreen.test.tsx (1)
104-113: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winTwo overlapping fixes for the same TDZ problem — clarify which is actually needed.
The delegating-arrow factory (104-113) is documented as solving the TDZ issue by deferring the
mockLoadModel/mockUnloadModellookup to call time. The re-binding at 553-562 cites the identical TDZ reasoning as its own justification. If the arrows already work, this reassignment is redundant; if it's still required, the arrows aren't actually fixing the problem and the comment at 104-109 is stale/misleading. Worth consolidating to one mechanism.Also applies to: 553-562
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@__tests__/rntl/screens/ChatScreen.test.tsx` around lines 104 - 113, The TDZ workaround is duplicated and the two fixes conflict in intent; keep only one mechanism and make the other unnecessary or remove it. Update the `ChatScreen.test.tsx` mock setup around the delegating-arrow factory and the later re-binding so there is a single source of truth for `mockLoadModel`/`mockUnloadModel`, and ensure the remaining code clearly defers lookup until call time without stale commentary.__tests__/integration/models/chatHomeSelectorParity.test.ts (1)
48-49: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winMisleading
requestAnimationFramestub; timing buffer thinner than sibling convention.The comment claims stubbing
requestAnimationFrame"flushes"waitForRenderFramefast, but the sibling test file (useChatModelActions.test.ts) documents that this helper actually resolves viaInteractionManager.runAfterInteractions(() => setTimeout(resolve, 350))and stubsInteractionManager.runAfterInteractionsspecifically for that reason — notrequestAnimationFrame. As written here, the stub likely does nothing useful, and the test instead relies entirely on the flatsetTimeout(resolve, 400)padding at line 203, which is also thinner than the 450ms buffer used elsewhere in this PR for the same 350ms wait.Consider aligning with the sibling file's pattern: stub
InteractionManager.runAfterInteractionsand bump the buffer to 450ms for consistency and reduced flakiness risk.Also applies to: 201-203
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@__tests__/integration/models/chatHomeSelectorParity.test.ts` around lines 48 - 49, The timing setup in chatHomeSelectorParity.test.ts is misleading because waitForRenderFrame resolves through InteractionManager.runAfterInteractions and a 350ms timeout, not requestAnimationFrame. Update the test to stub InteractionManager.runAfterInteractions like the sibling useChatModelActions.test.ts does, and increase the surrounding wait buffer from 400ms to 450ms for consistency and less flakiness. Use the existing waitForRenderFrame helper and the test’s InteractionManager usage as the key touchpoints.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@__tests__/unit/hooks/useChatModelActions.test.ts`:
- Around line 468-484: The test in useChatModelActions should assert the actual
finally cleanup, not just the loading-start callback. Update the failure-path
assertion around initiateModelLoad, makeDeps, and loadAnywayBtn.onPress so it
verifies setIsModelLoading(false) was called after the forced load rejection,
and ideally that the final loading state is cleared even when mockLoadTextModel
fails again. Replace the current toHaveBeenCalledWith(true) check with an
assertion that proves the cleanup path ran.
In `@src/screens/ChatScreen/useChatModelActions.ts`:
- Around line 96-101: The `initiateModelLoad` catch block is re-implementing the
“Load Anyway” override flow instead of reusing `loadModelWithOverride`, creating
duplicate UX and behavior. Update `useChatModelActions` so this path delegates
to the shared helper used by `proceedWithModelLoadFn`, and extend that helper’s
options if needed to support the `onLoadedResume` callback chain. Keep the alert
copy, retry action, and memory-gating logic centralized in the shared
implementation so future changes stay consistent.
---
Nitpick comments:
In `@__tests__/integration/models/chatHomeSelectorParity.test.ts`:
- Around line 48-49: The timing setup in chatHomeSelectorParity.test.ts is
misleading because waitForRenderFrame resolves through
InteractionManager.runAfterInteractions and a 350ms timeout, not
requestAnimationFrame. Update the test to stub
InteractionManager.runAfterInteractions like the sibling
useChatModelActions.test.ts does, and increase the surrounding wait buffer from
400ms to 450ms for consistency and less flakiness. Use the existing
waitForRenderFrame helper and the test’s InteractionManager usage as the key
touchpoints.
In `@__tests__/rntl/screens/ChatScreen.test.tsx`:
- Around line 104-113: The TDZ workaround is duplicated and the two fixes
conflict in intent; keep only one mechanism and make the other unnecessary or
remove it. Update the `ChatScreen.test.tsx` mock setup around the
delegating-arrow factory and the later re-binding so there is a single source of
truth for `mockLoadModel`/`mockUnloadModel`, and ensure the remaining code
clearly defers lookup until call time without stale commentary.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: f5c1efb0-914d-434b-ab6d-3d1b30015852
📒 Files selected for processing (21)
__tests__/integration/models/chatHomeSelectorParity.test.ts__tests__/pro/audio/ui/MessageAudioModeThinkingStream.test.tsx__tests__/rntl/components/CompletedDownloadCardRepair.test.tsx__tests__/rntl/screens/ChatScreen.test.tsx__tests__/unit/audio/streamingSpeech.test.ts__tests__/unit/hooks/useChatModelActions.test.ts__tests__/unit/services/generationToolLoop.test.ts__tests__/unit/services/llmHelpers.test.ts__tests__/unit/services/repairVisionProgress.test.ts__tests__/unit/stores/ttsStore.test.ts__tests__/unit/utils/messageContent.test.tsdocs/GAPS_BACKLOG.mdproscripts/ios-device.shsrc/screens/ChatScreen/useChatModelActions.tssrc/screens/DownloadManagerScreen/items.tsxsrc/services/generationToolLoop.tssrc/services/llmHelpers.tssrc/services/modelManager/download.tssrc/services/modelManager/index.tssrc/utils/messageContent.ts
…predicate (OD7 §C) The §C audit caught it: the OD7 marker list carried a 'SINGLE SOURCE OF TRUTH' comment but was a 4th hand-maintained copy that OMITTED enable_thinking, so it diverged from remoteModelCapabilities — a model exposing only the enable_thinking kwarg in its template was detected reasoning-capable on the gateway but NOT on-device. Add enable_thinking to the marker set, route remoteModelCapabilities' template detection through templateEmitsReasoning (one predicate, local + remote), and correct the comment to state exactly what it owns (template-capability detection) vs what stays in parseThinkingContent (output-delimiter extraction). Red-first: enable_thinking-only template now detects true (was false). 161 tests green (messageContent + llmHelpers + remoteModelCapabilities).
…otency Fixes the release-tooling defects flagged on PR #506: 1. Tag mispoint: promote.sh now bases the v<version> commit/tag on the tested BETA tag's commit (BETA_SHA), never arbitrary HEAD, and pushes to an explicit branch (PROMOTE_BRANCH, default main) fast-forward-only. 2. Partial-push idempotency: the repo step distinguishes "released on origin" (git ls-remote) from "tag only local" (a prior push failed) and re-attempts the push instead of skipping. 3. Build-pin (single source of truth): uat.sh records the tested store build id (Android versionCode / iOS CURRENT_PROJECT_VERSION) in the beta tag annotation ("Build: <n>", format owned by scripts/lib/version.js) and the GH prerelease body; promote.sh recovers it and pins Play (upload_to_play_store version_code:) + TestFlight (upload_to_app_store build_number:). Fails fast if unrecoverable. The Android promote lane FAILS FAST without version_code rather than promoting "latest on internal". 4. NOTES_FILE mktemp: drop the ".md" that orphaned the real temp file. Also: em-dash compliance sweep (rule 1570610) across promote.sh, uat.sh, Fastfile. Adds red-first tests for the new build-id helpers. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…6 (remote-caps flakiness / fixture hardening)
…EM-SM] logs - OnboardingScreen 'updates currentIndex on scroll end' was gutted to zero assertions (CodeRabbit Major) — rewrite it to drive onMomentumScrollEnd and assert the footer flips Next→Get Started (isLastSlide = currentIndex===last), a direct guard on onMomentumScrollEnd→setCurrentIndex. 15/15 green. - Replace 26 Unicode em dashes with ASCII hyphens in modelResidency [MEM-SM] diagnostic log lines (compliance 1570610). 58 residency tests still green.
…dempotency + em-dash sweep Closes the CodeRabbit/Qodo Major findings on promote.sh: build-pin (single-source build id via uat.sh→tag annotation→promote.sh→fastlane version_code/build_number, fail-fast if unrecoverable), tag-from-beta-SHA (not HEAD), remote-aware idempotency, NOTES_FILE mktemp, and the em-dash sweep across promote.sh/uat.sh/Fastfile.
Collaborator
Author
Review findings addressed (pushed in 6271328)All actionable CodeRabbit + Qodo findings fixed, red-first where code:
Also fixed (from the internal hygiene audit): OD7 reasoning-detection was a false "single source of truth" that diverged from Two separate pre-existing bugs surfaced during review (Jinja tool-parser role-alternation error; remote-capabilities response-shape flakiness) are logged in @coderabbitai review |
|
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
scripts/lib/version.js (1)
89-93: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueStray em dash left in error message.
Line 90 still uses an em dash (
—) inside the thrown error string. The PR's review follow-ups explicitly replaced em dashes with hyphens acrosspromote.sh,uat.sh,fastlane/Fastfile, and diagnostic logs for brand-voice consistency, but this occurrence inversion.jsappears to have been missed.✏️ Proposed fix
- 'No "Build: <n>" line in the tag annotation — cannot recover the tested build id. ' + + 'No "Build: <n>" line in the tag annotation - cannot recover the tested build id. ' +🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/lib/version.js` around lines 89 - 93, The thrown error string in version.js still contains a stray em dash, so update the message in the build-id recovery path to use a hyphen instead for consistency with the other reviewed files. Check the error text inside the version-recovery logic that throws when the tag annotation lacks a Build line, and replace the em dash in that message while keeping the rest of the wording unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/GAPS_BACKLOG.md`:
- Line 198: The note in the backlog entry has a Markdown lint issue because the
inline code snippet in the OD14 description includes a trailing space. Update
the OD14 text in GAPS_BACKLOG.md so the code span around content:'', has no
extra space before the closing backtick, keeping the rest of the entry
unchanged.
---
Nitpick comments:
In `@scripts/lib/version.js`:
- Around line 89-93: The thrown error string in version.js still contains a
stray em dash, so update the message in the build-id recovery path to use a
hyphen instead for consistency with the other reviewed files. Check the error
text inside the version-recovery logic that throws when the tag annotation lacks
a Build line, and replace the em dash in that message while keeping the rest of
the wording unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 224691ec-1024-4a0c-bde8-e2b505b016ba
📒 Files selected for processing (30)
__tests__/integration/models/chatHomeSelectorParity.test.ts__tests__/pro/audio/ui/MessageAudioModeThinkingStream.test.tsx__tests__/rntl/components/CompletedDownloadCardRepair.test.tsx__tests__/rntl/screens/ChatScreen.test.tsx__tests__/rntl/screens/OnboardingScreen.test.tsx__tests__/unit/audio/streamingSpeech.test.ts__tests__/unit/hooks/useChatModelActions.test.ts__tests__/unit/scripts/version.test.ts__tests__/unit/services/generationToolLoop.test.ts__tests__/unit/services/llmHelpers.test.ts__tests__/unit/services/repairVisionProgress.test.ts__tests__/unit/stores/ttsStore.test.ts__tests__/unit/utils/messageContent.test.tsdocs/GAPS_BACKLOG.mdfastlane/Fastfileproscripts/ios-device.shscripts/lib/version.d.tsscripts/lib/version.jsscripts/promote.shscripts/uat.shsrc/screens/ChatScreen/useChatModelActions.tssrc/screens/DownloadManagerScreen/items.tsxsrc/services/generationToolLoop.tssrc/services/llmHelpers.tssrc/services/modelManager/download.tssrc/services/modelManager/index.tssrc/services/modelResidency/index.tssrc/stores/remoteModelCapabilities.tssrc/utils/messageContent.ts
✅ Files skipped from review due to trivial changes (1)
- pro
🚧 Files skipped from review as they are similar to previous changes (22)
- tests/unit/utils/messageContent.test.ts
- scripts/lib/version.d.ts
- tests/unit/services/generationToolLoop.test.ts
- tests/unit/services/repairVisionProgress.test.ts
- tests/rntl/components/CompletedDownloadCardRepair.test.tsx
- tests/unit/services/llmHelpers.test.ts
- src/services/generationToolLoop.ts
- tests/pro/audio/ui/MessageAudioModeThinkingStream.test.tsx
- src/services/modelManager/index.ts
- tests/unit/stores/ttsStore.test.ts
- tests/unit/audio/streamingSpeech.test.ts
- src/screens/DownloadManagerScreen/items.tsx
- scripts/ios-device.sh
- src/services/modelManager/download.ts
- fastlane/Fastfile
- tests/integration/models/chatHomeSelectorParity.test.ts
- src/services/llmHelpers.ts
- scripts/promote.sh
- src/screens/ChatScreen/useChatModelActions.ts
- src/services/modelResidency/index.ts
- tests/rntl/screens/ChatScreen.test.tsx
- tests/unit/hooks/useChatModelActions.test.ts
| CPU-with-proper-threads is the path. The devicectl "developer disk image could not be mounted" | ||
| is a Mac-side tooling wedge (iOS 26.5.1), independent of the app — reconnect to clear. | ||
|
|
||
| | OD14 | **Pre-tool-call reasoning not persisted (thinking "disappears" when a tool call starts)** | RESOLVED | In a tool-using turn the model reasons, then emits a tool call; that FIRST round's live thinking is cleared (clearStreamingMessage between rounds) and the intermediate tool-call assistant message is built with `content:'', ` NO reasoningContent (generationToolLoop.ts:447-448). The post-tool-result round's thinking IS shown on the final answer (confirmed on device: "Thought process: The knowledge base search returned no results…"). So thinking visibly vanishes at tool-call start then reappears on the final message — the pre-tool-call reasoning is lost from the transcript. Minor fidelity gap, feature works end-to-end. Fix: attach the accumulated reasoningBuffer to the intermediate tool-call assistant message. NOTE (good): iOS ran this 9B on Metal (99 layers GPU offloaded), 2.0 tok/s, TTFT 10.79s — iOS Metal works well; Android Adreno OpenCL (1GB max-alloc) falls back to CPU. | |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Trim the trailing space inside the code span.
content:'', triggers MD038 because the inline code includes a trailing space. Remove it so the note renders and lints cleanly.
Suggested fix
- ... built with `content:'', ` NO reasoningContent ...
+ ... built with `content:'',` NO reasoningContent ...📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| | OD14 | **Pre-tool-call reasoning not persisted (thinking "disappears" when a tool call starts)** | RESOLVED | In a tool-using turn the model reasons, then emits a tool call; that FIRST round's live thinking is cleared (clearStreamingMessage between rounds) and the intermediate tool-call assistant message is built with `content:'', ` NO reasoningContent (generationToolLoop.ts:447-448). The post-tool-result round's thinking IS shown on the final answer (confirmed on device: "Thought process: The knowledge base search returned no results…"). So thinking visibly vanishes at tool-call start then reappears on the final message — the pre-tool-call reasoning is lost from the transcript. Minor fidelity gap, feature works end-to-end. Fix: attach the accumulated reasoningBuffer to the intermediate tool-call assistant message. NOTE (good): iOS ran this 9B on Metal (99 layers GPU offloaded), 2.0 tok/s, TTFT 10.79s — iOS Metal works well; Android Adreno OpenCL (1GB max-alloc) falls back to CPU. | | |
| | OD14 | **Pre-tool-call reasoning not persisted (thinking "disappears" when a tool call starts)** | RESOLVED | In a tool-using turn the model reasons, then emits a tool call; that FIRST round's live thinking is cleared (clearStreamingMessage between rounds) and the intermediate tool-call assistant message is built with `content:'',` NO reasoningContent (generationToolLoop.ts:447-448). The post-tool-result round's thinking IS shown on the final answer (confirmed on device: "Thought process: The knowledge base search returned no results…"). So thinking visibly vanishes at tool-call start then reappears on the final message — the pre-tool-call reasoning is lost from the transcript. Minor fidelity gap, feature works end-to-end. Fix: attach the accumulated reasoningBuffer to the intermediate tool-call assistant message. NOTE (good): iOS ran this 9B on Metal (99 layers GPU offloaded), 2.0 tok/s, TTFT 10.79s — iOS Metal works well; Android Adreno OpenCL (1GB max-alloc) falls back to CPU. | |
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 198-198: Spaces inside code span elements
(MD038, no-space-in-code)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/GAPS_BACKLOG.md` at line 198, The note in the backlog entry has a
Markdown lint issue because the inline code snippet in the OD14 description
includes a trailing space. Update the OD14 text in GAPS_BACKLOG.md so the code
span around content:'', has no extra space before the closing backtick, keeping
the rest of the entry unchanged.
Source: Linters/SAST tools
This was referenced Jul 9, 2026
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
0.0.103 — memory/Load-Anyway fix + release tooling + on-device hardening
Consolidated release branch. Every change is red-first tested; on-device items are
called out as UNVERIFIED (TestFlight) where a device run is still owed.
Core fixes
makeRoomFornow evicts, then re-measures real free RAM against the override survival floor (was a predictive pre-eviction estimate that crediting 0 for clean evictions → false refusals). + an end-to-end override-chain integration test.metadataJsonon the download store row so a same-session retry/reconcile re-fetches the vision projector instead of re-issuing text-only.checkMemoryForModelpre-check); refusal comes fromOverridableMemoryError→ the shared "Load Anyway" affordance. A model loadable from Home is now loadable in chat.wednesday.is— "made with ♥" link points at plainhttps://wednesday.is; collapsed a DRY break (screen was hardcoding the URL).Pro (audio/TTS) fixes — pro
fix/tts-download-flag-reconcilemodelDownloadedflag, on BOTHcheckDownloadStatusAND thespeak()play gate. Extracted download actions tottsDownloadActions.ts(no eslint-disable).Release tooling
promote.sh+ fastlane promote lanes +scripts/lib/version.js(single-source version math) — promote a tested beta to production without rebuild.ios-device.sh— build against a generic iOS destination (fixes the "developer disk image could not be mounted" build stall).Test-infra fix (surfaced by the above)
ChatScreen.test.tsx— thejest.mockfactory referenced aconstmock before init →loadTextModel: undefined, which had been masking stale assertions. Delegating arrows + typed mocks; 155/155.Gates (local)
NOT yet verified — TestFlight (Release build) is the on-device gate
On-device confirmation is owed for: Load-Anyway on a fittable model, vision-retry, TTS play/stream, voice thinking, cross-tool CoT. Deferred device/perf items logged in
docs/GAPS_BACKLOG.md(OD10 mid-speak stop, OD11 TTS co-residency, OD13 reasoning-format mismatch, threads=1 default).🤖 Generated with Claude Code
Summary by CodeRabbit