feat(ocr): integrate Light-OCR#2005
Open
yyhhyyyyyy wants to merge 28 commits into
Open
Conversation
Contributor
|
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 an offline Light OCR attachment pipeline: a helper process/protocol, extraction service, encrypted cache, and capability router, integrated into session/turn coordination, attachment preflight/blocking UI, OCR settings, CI packaging/size gates, shared contracts, and full localization. It also refreshes the ACP registry and model catalog (unrelated). ChangesOffline Light OCR Attachment Integration
Unrelated ACP Registry & Model Catalog Refresh
Estimated code review effort: 5 (Critical) | ~180 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 13
🧹 Nitpick comments (5)
src/renderer/src/components/chat/nodes/fileAttachment.ts (1)
41-46: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winValidate parsed
requestedRepresentationwith the shared normalizer.
parseHTMLonly defaults to'auto'when the attribute is missing, not when it holds an invalid value.normalizeAttachmentRepresentationPreference(src/shared/utils/attachmentRepresentation.ts) already exists to validate/coerce this exact preference and is presumably relied on elsewhere in the attachment contract chain; using it here keeps deserialized HTML consistent with the rest of the pipeline.♻️ Proposed fix
+import { normalizeAttachmentRepresentationPreference } from '`@shared/utils/attachmentRepresentation`' ... requestedRepresentation: { default: 'auto', - parseHTML: (el) => el.getAttribute('data-requested-representation') || 'auto', + parseHTML: (el) => + normalizeAttachmentRepresentationPreference( + el.getAttribute('data-requested-representation') + ) ?? 'auto', renderHTML: (attrs) => ({ 'data-requested-representation': attrs.requestedRepresentation }) }🤖 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 `@src/renderer/src/components/chat/nodes/fileAttachment.ts` around lines 41 - 46, Update the requestedRepresentation parseHTML handler in the attachment node to pass the retrieved data-requested-representation value through normalizeAttachmentRepresentationPreference, preserving 'auto' as the fallback for missing or invalid values. Keep renderHTML unchanged.test/main/session/chatService.test.ts (1)
178-281: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winSubmission-scoped abort is tested for
sendMessagebut not forsteerActiveTurn.L208-247 only covers
stopStream-triggered abort forsteerActiveTurn; L249-281 covers submission-scopedAbortControllerabort, but only forsendMessage. Consider adding an analogous submission-scoped-abort test forsteerActiveTurnto confirm it doesn't inadvertently cancel generation/session permissions either.🤖 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 `@test/main/session/chatService.test.ts` around lines 178 - 281, Add a submission-scoped AbortController test for steerActiveTurn, analogous to the existing sendMessage test. Keep the steer operation pending until the supplied signal aborts, then assert it rejects with AbortError without calling turn.cancelGeneration or sessionPermissionPort.clearSessionPermissions.test/main/routes/dispatcher.test.ts (2)
4255-4323: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winRenderer-ownership cancellation coverage exists only for
chat.sendMessage.This is a strong regression test for a real authorization boundary (blocking cross-renderer cancellation), but there's no analogous test exercising
chat.cancelSubmissionagainst an in-flightchat.steerActiveTurnorsessions.retryMessageattachment preflight, even though both now surfaceattachmentPreparation/blocking behavior in this same file. Consider extending this pattern to those two paths.🤖 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 `@test/main/routes/dispatcher.test.ts` around lines 4255 - 4323, Extend the renderer-ownership cancellation coverage from the existing chat.sendMessage test to both in-flight chat.steerActiveTurn and sessions.retryMessage attachment-preflight flows. For each path, await the preflight becoming blocked, verify chat.cancelSubmission from a different renderer returns { cancelled: false } without aborting it, then verify cancellation from the owning renderer succeeds and the operation rejects with AbortError; preserve the existing post-cancellation false result.
1-1: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winSubmission-scoped cancellation is only fully tested for
chat.sendMessage, notsteerActiveTurn. Both files establish a solid pattern for verifying the new abort/attachment-preflight semantics, but the pattern isn't extended to the steer path at either layer.
test/main/routes/dispatcher.test.ts#L4255-4323: add an analogous renderer-ownershipchat.cancelSubmissiontest for an in-flightchat.steerActiveTurn(and ideallysessions.retryMessage, pending the separate signal-wiring verification) attachment preflight.test/main/session/chatService.test.ts#L178-281: add a submission-scopedAbortControllerabort test forsteerActiveTurn(mirroring the existingsendMessagetest at L249-281), distinct from the already-coveredstopStream-triggered abort at L208-247.🤖 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 `@test/main/routes/dispatcher.test.ts` at line 1, Extend the existing cancellation coverage to steerActiveTurn: in the dispatcher tests, add a renderer-ownership chat.cancelSubmission case for an in-flight steerActiveTurn attachment preflight, preserving the established sendMessage pattern; in chatService tests, add a submission-scoped AbortController abort test for steerActiveTurn, separate from the stopStream-triggered abort coverage. Also apply the analogous renderer test to sessions.retryMessage if its signal wiring is available.test/main/session/data/tables/deepchatPendingInputsTable.test.ts (1)
157-183: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winAdd a negative-case test for invalid/forged
resolvedRepresentation.This test only verifies the happy path where a valid
resolvedRepresentationis preserved. Given attachment data is treated as untrusted (per PR objectives) and this is the boundary where renderer-supplied payloads enter persisted pending-input state, add a companion case asserting that a forged/invalidresolvedRepresentation(e.g., unexpectedkind, extra fields, or a fakekind: 'image'used to bypass OCR) is rejected or stripped, mirroring how test 6 (lines 134-155) verifies blocking metadata is sanitized.🤖 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 `@test/main/session/data/tables/deepchatPendingInputsTable.test.ts` around lines 157 - 183, Add a companion negative-case test near the existing valid snapshot test for createQueueInput, supplying a forged or invalid resolvedRepresentation such as an unsupported kind or fake image representation with extra fields. Assert that the persisted pending payload rejects or strips the invalid representation, matching the sanitization expectations established by test 6.
🤖 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 `@resources/model-db/providers.json`:
- Line 260229: Update the context value for the Meituan: LongCat 2.0 provider
entry from 1048756 to 1048576 in providers.json, leaving all other provider
configuration unchanged.
- Around line 236569-236573: The gemini-3.6-flash pricing entry has an incorrect
cache_read value. In its cost object, update cache_read from 1.5 to 0.15 while
preserving the input and output values.
- Around line 259585-259661: Update the `type` field in both OpenRouter entries,
`google/gemini-3.5-flash-lite` and `google/gemini-3.6-flash`, from
`imageGeneration` to `chat`; leave their existing modalities, reasoning, limits,
and other capabilities unchanged.
In `@src/main/agent/deepchat/runtime/contextBuilder.ts`:
- Around line 407-428: Update buildResolvedImageRepresentationContext to inspect
the ocr_text representation’s truncated flag and append a clear truncation note
to the generated OCR context when it is true. Preserve the existing unavailable,
empty-text, escaping, metadata, and non-truncated output behavior.
- Around line 430-432: Update escapeUntrustedOcrBlockDelimiters to escape every
angle bracket in the input, rather than pattern-matching only untrusted_ocr_data
tags. Ensure both “<” and “>” are encoded so any OCR text resembling an
HTML-like delimiter cannot bypass the trust boundary.
In `@src/main/ocr/ocrRuntimeAssetResolver.ts`:
- Around line 256-262: Update the escape guard in the runtime asset path
validation to also reject the exact parent-relative value (`relative === '..'`),
while preserving the existing checks for empty, nested-parent, and absolute
paths. Keep the change scoped to the validation logic surrounding `resolvedRoot`
and `resolvedPath`.
In `@src/main/session/data/pendingInputStore.ts`:
- Around line 145-154: Update updateQueueInput to validate the current row.state
before writing payload_json, matching the state-guard behavior of the sibling
mutators. Reject claimed or otherwise non-editable states before mutation, while
preserving the existing blocked-to-pending reset and normal update behavior for
allowed states.
In `@src/main/session/turn.ts`:
- Around line 281-287: In the ACP branch of the turn handling flow, move
commitRetryMessage so it runs only after runtime.send completes successfully.
Preserve the existing sessionId and prepared.sourceOrderSeq arguments, and
ensure send errors propagate without committing or deleting the prior
transcript.
In `@src/renderer/settings/components/OcrSettings.vue`:
- Line 256: Update the status polling error handling around statusErrorNotified
and the logic near status.value so failures after a prior successful poll are
surfaced instead of remaining silent; preserve deduplication by notifying only
on the first failure in each failure period, and reset that failure state when a
subsequent poll succeeds. Ensure the status card no longer presents stale
available data without an indication of polling failure.
In `@src/renderer/src/components/chat/ChatInputBox.vue`:
- Around line 558-563: Update handleKeydown so the non-editable guard does not
prevent Tab navigation or standard copy/select-all shortcuts (Ctrl/Cmd+Tab,
Ctrl/Cmd+C, and Ctrl/Cmd+A as applicable); allow those key-specific paths to
execute before returning for other editing commands. Preserve the existing
prevention behavior for actions that modify the draft while props.editable is
false.
In `@src/renderer/src/components/message/MessageItemUser.vue`:
- Around line 256-263: Update messageFileByKey and its fallback lookup in
MessageItemUser so exact file paths remain uniquely mapped, while basename
matching is only used when exactly one attachment has that name. Do not
overwrite basename candidates when duplicates exist; treat ambiguous names as
unmatched so inline blocks cannot receive incorrect file metadata or OCR
content.
In `@src/renderer/src/i18n/he-IL/chat.json`:
- Line 431: Update the Hebrew translations for image_limit_exceeded and the
corresponding message at line 440 to replace התור with a term clearly meaning
the current turn or round, while preserving the existing message meaning and
formatting.
In `@test/main/ocr/ocrArtifactStore.test.ts`:
- Around line 15-26: Ensure encrypted persistence tests cannot be silently
skipped in required CI: update the OCR artifact store test setup around
sqliteAvailable and persistentIt to honor DEEPCHAT_REQUIRE_NATIVE_SQLITE,
failing when the flag is set but better-sqlite3-multiple-ciphers is unavailable;
also configure that environment variable for the CI command running these tests
if needed.
---
Nitpick comments:
In `@src/renderer/src/components/chat/nodes/fileAttachment.ts`:
- Around line 41-46: Update the requestedRepresentation parseHTML handler in the
attachment node to pass the retrieved data-requested-representation value
through normalizeAttachmentRepresentationPreference, preserving 'auto' as the
fallback for missing or invalid values. Keep renderHTML unchanged.
In `@test/main/routes/dispatcher.test.ts`:
- Around line 4255-4323: Extend the renderer-ownership cancellation coverage
from the existing chat.sendMessage test to both in-flight chat.steerActiveTurn
and sessions.retryMessage attachment-preflight flows. For each path, await the
preflight becoming blocked, verify chat.cancelSubmission from a different
renderer returns { cancelled: false } without aborting it, then verify
cancellation from the owning renderer succeeds and the operation rejects with
AbortError; preserve the existing post-cancellation false result.
- Line 1: Extend the existing cancellation coverage to steerActiveTurn: in the
dispatcher tests, add a renderer-ownership chat.cancelSubmission case for an
in-flight steerActiveTurn attachment preflight, preserving the established
sendMessage pattern; in chatService tests, add a submission-scoped
AbortController abort test for steerActiveTurn, separate from the
stopStream-triggered abort coverage. Also apply the analogous renderer test to
sessions.retryMessage if its signal wiring is available.
In `@test/main/session/chatService.test.ts`:
- Around line 178-281: Add a submission-scoped AbortController test for
steerActiveTurn, analogous to the existing sendMessage test. Keep the steer
operation pending until the supplied signal aborts, then assert it rejects with
AbortError without calling turn.cancelGeneration or
sessionPermissionPort.clearSessionPermissions.
In `@test/main/session/data/tables/deepchatPendingInputsTable.test.ts`:
- Around line 157-183: Add a companion negative-case test near the existing
valid snapshot test for createQueueInput, supplying a forged or invalid
resolvedRepresentation such as an unsupported kind or fake image representation
with extra fields. Assert that the persisted pending payload rejects or strips
the invalid representation, matching the sanitization expectations established
by test 6.
🪄 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
Run ID: ee0267a3-1fb3-4f16-b76a-a4ee94389d34
⛔ Files ignored due to path filters (2)
src/renderer/src/lib/icons/icon-collections.generated.tsis excluded by!**/*.generated.*src/renderer/src/lib/icons/icon-whitelist.generated.tsis excluded by!**/*.generated.*
📒 Files selected for processing (234)
.github/actions/light-ocr-package-size/action.yml.github/workflows/build.yml.github/workflows/release.ymldocs/features/light-ocr-integration/plan.mddocs/features/light-ocr-integration/spec.mddocs/features/light-ocr-integration/tasks.mddocs/issues/light-ocr-follow-up-hardening/spec.mdelectron-builder.ymlelectron.vite.config.tspackage.jsonresources/acp-registry/registry.jsonresources/light-ocr-size-budgets.jsonresources/model-db/providers.jsonresources/runtime-versions.jsonscripts/afterPack.jsscripts/compare-light-ocr-package-size.mjsscripts/install-runtime.mjsscripts/smoke-light-ocr.jssrc/main/agent/deepchat/runtime/contextBuilder.tssrc/main/agent/deepchat/runtime/deepChatRuntimeCoordinator.tssrc/main/agent/deepchat/runtime/turnCoordinator.tssrc/main/agent/manager/deepChatAgentBackend.tssrc/main/agent/manager/directAcpAgentBackend.tssrc/main/agent/manager/sessionHandles.tssrc/main/agent/shared/agentSessionHandle.tssrc/main/agent/shared/agentSessionNormalization.tssrc/main/app/composition.tssrc/main/app/settingsRoutes.tssrc/main/data/databaseConnection.tssrc/main/data/schemaCatalog.tssrc/main/exporter/agentSessionExporter.tssrc/main/exporter/formats/conversationExporter.tssrc/main/exporter/formats/nowledgeMemExporter.tssrc/main/exporter/formats/userMessageText.tssrc/main/lightOcrHelperEntry.tssrc/main/ocr/attachmentCapabilityRouter.tssrc/main/ocr/imagePreprocessor.tssrc/main/ocr/imageTextExtractionService.tssrc/main/ocr/lightOcrHelper.tssrc/main/ocr/lightOcrProcessHost.tssrc/main/ocr/lightOcrProtocol.tssrc/main/ocr/ocrArtifactStore.tssrc/main/ocr/ocrCacheKeyProvider.tssrc/main/ocr/ocrExtractionScheduler.tssrc/main/ocr/ocrRuntimeAssetResolver.tssrc/main/ocr/ocrRuntimeService.tssrc/main/ocr/ocrSettings.tssrc/main/ocr/routes.tssrc/main/remote/conversation/runner.tssrc/main/session/chatService.tssrc/main/session/contracts.tssrc/main/session/data/contracts.tssrc/main/session/data/pendingInputStore.tssrc/main/session/data/pendingInputs.tssrc/main/session/data/tables/deepchatPendingInputs.tssrc/main/session/data/transcript.tssrc/main/session/data/userMessageContent.tssrc/main/session/lifecycle.tssrc/main/session/routes.tssrc/main/session/sessionService.tssrc/main/session/submissionCancellationRegistry.tssrc/main/session/transcriptMutations.tssrc/main/session/turn.tssrc/main/tape/application/recallProjection.tssrc/main/tape/application/recallService.tssrc/main/tape/infrastructure/sqlite/tapeSearchProjectionStore.tssrc/renderer/api/ChatClient.tssrc/renderer/api/OcrClient.tssrc/renderer/api/SessionClient.tssrc/renderer/settings/components/OcrSettings.vuesrc/renderer/settings/settingsRouteComponents.tssrc/renderer/src/components/chat/AttachmentPreparationDialog.vuesrc/renderer/src/components/chat/ChatAttachmentItem.vuesrc/renderer/src/components/chat/ChatInputBox.vuesrc/renderer/src/components/chat/ChatInputToolbar.vuesrc/renderer/src/components/chat/ChatStatusBar.vuesrc/renderer/src/components/chat/PendingInputLane.vuesrc/renderer/src/components/chat/composables/useChatInputFiles.tssrc/renderer/src/components/chat/nodes/FileAttachmentView.vuesrc/renderer/src/components/chat/nodes/fileAttachment.tssrc/renderer/src/components/chat/nodes/symbols.tssrc/renderer/src/components/message/MessageContent.vuesrc/renderer/src/components/message/MessageItemUser.vuesrc/renderer/src/features/chat-page/ChatPage.vuesrc/renderer/src/features/chat-page/composables/useComposerSubmit.tssrc/renderer/src/features/chat-page/composables/useMessageActions.tssrc/renderer/src/features/chat-page/composables/usePendingInputActions.tssrc/renderer/src/features/chat-page/model/composerDraftState.tssrc/renderer/src/features/chat-page/model/displayMessage.tssrc/renderer/src/i18n/da-DK/chat.jsonsrc/renderer/src/i18n/da-DK/routes.jsonsrc/renderer/src/i18n/da-DK/settings.jsonsrc/renderer/src/i18n/de-DE/chat.jsonsrc/renderer/src/i18n/de-DE/routes.jsonsrc/renderer/src/i18n/de-DE/settings.jsonsrc/renderer/src/i18n/en-US/chat.jsonsrc/renderer/src/i18n/en-US/routes.jsonsrc/renderer/src/i18n/en-US/settings.jsonsrc/renderer/src/i18n/es-ES/chat.jsonsrc/renderer/src/i18n/es-ES/routes.jsonsrc/renderer/src/i18n/es-ES/settings.jsonsrc/renderer/src/i18n/fa-IR/chat.jsonsrc/renderer/src/i18n/fa-IR/routes.jsonsrc/renderer/src/i18n/fa-IR/settings.jsonsrc/renderer/src/i18n/fr-FR/chat.jsonsrc/renderer/src/i18n/fr-FR/routes.jsonsrc/renderer/src/i18n/fr-FR/settings.jsonsrc/renderer/src/i18n/he-IL/chat.jsonsrc/renderer/src/i18n/he-IL/routes.jsonsrc/renderer/src/i18n/he-IL/settings.jsonsrc/renderer/src/i18n/id-ID/chat.jsonsrc/renderer/src/i18n/id-ID/routes.jsonsrc/renderer/src/i18n/id-ID/settings.jsonsrc/renderer/src/i18n/it-IT/chat.jsonsrc/renderer/src/i18n/it-IT/routes.jsonsrc/renderer/src/i18n/it-IT/settings.jsonsrc/renderer/src/i18n/ja-JP/chat.jsonsrc/renderer/src/i18n/ja-JP/routes.jsonsrc/renderer/src/i18n/ja-JP/settings.jsonsrc/renderer/src/i18n/ko-KR/chat.jsonsrc/renderer/src/i18n/ko-KR/routes.jsonsrc/renderer/src/i18n/ko-KR/settings.jsonsrc/renderer/src/i18n/ms-MY/chat.jsonsrc/renderer/src/i18n/ms-MY/routes.jsonsrc/renderer/src/i18n/ms-MY/settings.jsonsrc/renderer/src/i18n/pl-PL/chat.jsonsrc/renderer/src/i18n/pl-PL/routes.jsonsrc/renderer/src/i18n/pl-PL/settings.jsonsrc/renderer/src/i18n/pt-BR/chat.jsonsrc/renderer/src/i18n/pt-BR/routes.jsonsrc/renderer/src/i18n/pt-BR/settings.jsonsrc/renderer/src/i18n/ru-RU/chat.jsonsrc/renderer/src/i18n/ru-RU/routes.jsonsrc/renderer/src/i18n/ru-RU/settings.jsonsrc/renderer/src/i18n/tr-TR/chat.jsonsrc/renderer/src/i18n/tr-TR/routes.jsonsrc/renderer/src/i18n/tr-TR/settings.jsonsrc/renderer/src/i18n/vi-VN/chat.jsonsrc/renderer/src/i18n/vi-VN/routes.jsonsrc/renderer/src/i18n/vi-VN/settings.jsonsrc/renderer/src/i18n/zh-CN/chat.jsonsrc/renderer/src/i18n/zh-CN/routes.jsonsrc/renderer/src/i18n/zh-CN/settings.jsonsrc/renderer/src/i18n/zh-HK/chat.jsonsrc/renderer/src/i18n/zh-HK/routes.jsonsrc/renderer/src/i18n/zh-HK/settings.jsonsrc/renderer/src/i18n/zh-TW/chat.jsonsrc/renderer/src/i18n/zh-TW/routes.jsonsrc/renderer/src/i18n/zh-TW/settings.jsonsrc/renderer/src/lib/errors.tssrc/renderer/src/pages/NewThreadPage.vuesrc/renderer/src/stores/ui/attachmentPreparation.tssrc/renderer/src/stores/ui/pendingInput.tssrc/renderer/src/stores/ui/session.tssrc/shadcn/components/ui/dropdown-menu/DropdownMenuContent.vuesrc/shared/chat.d.tssrc/shared/contracts/common.tssrc/shared/contracts/domainSchemas.tssrc/shared/contracts/events/settings.events.tssrc/shared/contracts/routes.tssrc/shared/contracts/routes/chat.routes.tssrc/shared/contracts/routes/ocr.routes.tssrc/shared/contracts/routes/sessions.routes.tssrc/shared/contracts/routes/settings.routes.tssrc/shared/contracts/routes/system.routes.tssrc/shared/settingsNavigation.tssrc/shared/types/agent-interface.d.tssrc/shared/types/attachment.tssrc/shared/types/core/chat.tssrc/shared/utils/attachmentRepresentation.tstest/fixtures/light-ocr/fake-helper.mjstest/main/agent/deepchat/runtime/contextBuilder.test.tstest/main/agent/deepchat/runtime/deepChatRuntimeCoordinator.test.tstest/main/agent/manager/deepChatAgentBackend.test.tstest/main/agent/manager/directAcpAgentBackend.test.tstest/main/data/databaseConnection.test.tstest/main/exporter/agentSessionExporter.test.tstest/main/memory/memoryNativeMigration.test.tstest/main/ocr/attachmentCapabilityRouter.test.tstest/main/ocr/imagePreprocessor.test.tstest/main/ocr/imageTextExtractionService.test.tstest/main/ocr/lightOcrHelper.test.tstest/main/ocr/lightOcrProcessHost.test.tstest/main/ocr/ocrArtifactStore.test.tstest/main/ocr/ocrCacheKeyProvider.test.tstest/main/ocr/ocrExtractionScheduler.test.tstest/main/ocr/ocrRuntimeAssetResolver.test.tstest/main/ocr/ocrRuntimeService.test.tstest/main/ocr/ocrSettings.test.tstest/main/ocr/routes.test.tstest/main/remote/remoteConversationRunner.test.tstest/main/routes/contracts.test.tstest/main/routes/dispatcher.test.tstest/main/scripts/afterPack.test.tstest/main/scripts/installRuntime.test.tstest/main/scripts/lightOcrPackageSize.test.tstest/main/scripts/smokeLightOcr.test.tstest/main/session/assignmentPolicy.test.tstest/main/session/chatService.test.tstest/main/session/data/pendingInputStore.test.tstest/main/session/data/pendingInputs.test.tstest/main/session/data/tables/deepchatPendingInputsTable.test.tstest/main/session/data/tapeRecall.test.tstest/main/session/data/transcript.test.tstest/main/session/lifecycle.test.tstest/main/session/runtimeIntegration.test.tstest/main/session/session.integration.test.tstest/main/session/sessionFixture.tstest/main/session/submissionCancellationRegistry.test.tstest/main/session/transcriptMutations.test.tstest/main/session/turn.test.tstest/main/shared/attachmentRepresentation.test.tstest/main/shared/settingsNavigation.test.tstest/renderer/api/clients.test.tstest/renderer/components/AttachmentPreparationDialog.test.tstest/renderer/components/ChatAttachmentItem.test.tstest/renderer/components/ChatInputBox.test.tstest/renderer/components/ChatInputToolbar.test.tstest/renderer/components/ChatPage.test.tstest/renderer/components/DropdownMenuContent.test.tstest/renderer/components/NewThreadPage.onboarding.test.tstest/renderer/components/NewThreadPage.test.tstest/renderer/components/OcrSettings.test.tstest/renderer/components/PendingInputLane.test.tstest/renderer/components/useChatInputFiles.test.tstest/renderer/features/chat-page/composables/useComposerSubmit.test.tstest/renderer/features/chat-page/composables/useMessageActions.test.tstest/renderer/features/chat-page/composables/usePendingInputActions.test.tstest/renderer/features/chat-page/model/composerDraftState.test.tstest/renderer/lib/errors.test.tstest/renderer/pages/NewThreadPage.test.tstest/renderer/stores/attachmentPreparationStore.test.tstest/renderer/stores/sessionStore.test.tstsconfig.node.json
| ] | ||
| }, | ||
| "limit": { | ||
| "context": 1048756, |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show the local context around the flagged line and search for related values.
sed -n '260210,260245p' resources/model-db/providers.json
printf '\n--- occurrences of 1048756 ---\n'
rg -n '"context": 1048756|1048756' resources/model-db/providers.json || true
printf '\n--- occurrences of 1048576 ---\n'
rg -n '"context": 1048576|1048576' resources/model-db/providers.json || true
printf '\n--- nearby LongCat/Anthropic-style entries ---\n'
rg -n 'LongCat|1048576|1048756' resources/model-db/providers.json | head -n 40Repository: ThinkInAIXYZ/deepchat
Length of output: 19233
🌐 Web query:
LongCat 2.0 context limit 1048756 1048576 provider spec
💡 Result:
LongCat-2.0’s documented context limit is 1,048,576 tokens (1M), not 1,048,756. The provider docs also say the max output length is 131,072 tokens (128K). (long-cat.org)
If you’re mapping this into a provider spec, use:
context_window:1048576max_output_tokens:131072(long-cat.org)
If you want, I can format this as JSON/YAML for your spec.
Citations:
- 1: https://long-cat.org/guides/longcat-api-quick-start?utm_source=openai
- 2: https://long-cat.org/guides/longcat-api-quick-start?utm_source=openai
Fix the LongCat 2.0 context limit. 1048756 should be 1048576 for Meituan: LongCat 2.0.
🔧 Suggested fix
- "context": 1048756,
+ "context": 1048576,📝 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
| "context": 1048756, | |
| "context": 1048576, |
🤖 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 `@resources/model-db/providers.json` at line 260229, Update the context value
for the Meituan: LongCat 2.0 provider entry from 1048756 to 1048576 in
providers.json, leaving all other provider configuration unchanged.
Comment on lines
+407
to
+428
| function buildResolvedImageRepresentationContext(files: MessageFile[]): string { | ||
| const imageFiles = files.filter((file) => isImageAttachment(file)) | ||
| return imageFiles | ||
| .flatMap((file, index) => { | ||
| const resolved = getAttachmentResolvedRepresentation(file) | ||
| if (!resolved || resolved.kind === 'image') return [] | ||
| const fileName = typeof file.name === 'string' ? file.name : `image-${index + 1}` | ||
| const mimeType = resolveFileMimeType(file) | ||
| const metadata = [`name: ${fileName}`, `mime: ${mimeType}`].join('\n') | ||
| if (resolved.kind === 'unavailable') { | ||
| return [ | ||
| `[Attached Image ${index + 1} - content unavailable]\n${metadata}\nreason: ${resolved.reason}` | ||
| ] | ||
| } | ||
|
|
||
| const escapedText = escapeUntrustedOcrBlockDelimiters(resolved.text) | ||
| return [ | ||
| `[Attached Image ${index + 1} - OCR text; untrusted attachment data]\n${metadata}\n<untrusted_ocr_data>\n${escapedText || '[empty]'}\n</untrusted_ocr_data>` | ||
| ] | ||
| }) | ||
| .join('\n\n') | ||
| } |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Truncated OCR text isn't flagged to the model.
resolved of kind 'ocr_text' carries a truncated flag (per the attachment representation contract), but this function never surfaces it. When OCR text was cut short, the model has no signal that the content is incomplete and may treat it as the full image content.
🛠️ Proposed fix: append a truncation note when present
const escapedText = escapeUntrustedOcrBlockDelimiters(resolved.text)
+ const truncationNote = resolved.truncated ? '\n[text truncated]' : ''
return [
- `[Attached Image ${index + 1} - OCR text; untrusted attachment data]\n${metadata}\n<untrusted_ocr_data>\n${escapedText || '[empty]'}\n</untrusted_ocr_data>`
+ `[Attached Image ${index + 1} - OCR text; untrusted attachment data]\n${metadata}\n<untrusted_ocr_data>\n${escapedText || '[empty]'}\n</untrusted_ocr_data>${truncationNote}`
]📝 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
| function buildResolvedImageRepresentationContext(files: MessageFile[]): string { | |
| const imageFiles = files.filter((file) => isImageAttachment(file)) | |
| return imageFiles | |
| .flatMap((file, index) => { | |
| const resolved = getAttachmentResolvedRepresentation(file) | |
| if (!resolved || resolved.kind === 'image') return [] | |
| const fileName = typeof file.name === 'string' ? file.name : `image-${index + 1}` | |
| const mimeType = resolveFileMimeType(file) | |
| const metadata = [`name: ${fileName}`, `mime: ${mimeType}`].join('\n') | |
| if (resolved.kind === 'unavailable') { | |
| return [ | |
| `[Attached Image ${index + 1} - content unavailable]\n${metadata}\nreason: ${resolved.reason}` | |
| ] | |
| } | |
| const escapedText = escapeUntrustedOcrBlockDelimiters(resolved.text) | |
| return [ | |
| `[Attached Image ${index + 1} - OCR text; untrusted attachment data]\n${metadata}\n<untrusted_ocr_data>\n${escapedText || '[empty]'}\n</untrusted_ocr_data>` | |
| ] | |
| }) | |
| .join('\n\n') | |
| } | |
| function buildResolvedImageRepresentationContext(files: MessageFile[]): string { | |
| const imageFiles = files.filter((file) => isImageAttachment(file)) | |
| return imageFiles | |
| .flatMap((file, index) => { | |
| const resolved = getAttachmentResolvedRepresentation(file) | |
| if (!resolved || resolved.kind === 'image') return [] | |
| const fileName = typeof file.name === 'string' ? file.name : `image-${index + 1}` | |
| const mimeType = resolveFileMimeType(file) | |
| const metadata = [`name: ${fileName}`, `mime: ${mimeType}`].join('\n') | |
| if (resolved.kind === 'unavailable') { | |
| return [ | |
| `[Attached Image ${index + 1} - content unavailable]\n${metadata}\nreason: ${resolved.reason}` | |
| ] | |
| } | |
| const escapedText = escapeUntrustedOcrBlockDelimiters(resolved.text) | |
| const truncationNote = resolved.truncated ? '\n[text truncated]' : '' | |
| return [ | |
| `[Attached Image ${index + 1} - OCR text; untrusted attachment data]\n${metadata}\n<untrusted_ocr_data>\n${escapedText || '[empty]'}\n</untrusted_ocr_data>${truncationNote}` | |
| ] | |
| }) | |
| .join('\n\n') | |
| } |
🤖 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 `@src/main/agent/deepchat/runtime/contextBuilder.ts` around lines 407 - 428,
Update buildResolvedImageRepresentationContext to inspect the ocr_text
representation’s truncated flag and append a clear truncation note to the
generated OCR context when it is true. Preserve the existing unavailable,
empty-text, escaping, metadata, and non-truncated output behavior.
Comment on lines
+430
to
+432
| function escapeUntrustedOcrBlockDelimiters(value: string): string { | ||
| return value.replace(/<(\/?untrusted_ocr_data)(?=[\s>])/gi, '<$1') | ||
| } |
Contributor
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Delimiter-escaping regex can be bypassed, weakening the untrusted-OCR trust boundary.
The regex /<(\/?untrusted_ocr_data)(?=[\s>])/gi only matches when < is immediately followed by the tag name. Attacker-controlled OCR text (from a crafted image) can insert whitespace, e.g. < untrusted_ocr_data> or < /untrusted_ocr_data>, to slip past this check while still visually/structurally resembling the delimiter to the model — undermining the purpose of wrapping OCR text as untrusted content. Since the goal here is simply to prevent any <...>-shaped token from being mistaken for the trust-boundary tag, escaping every < unconditionally is both simpler and closes this gap.
🔒 Proposed fix: escape all angle brackets instead of pattern-matching the tag name
function escapeUntrustedOcrBlockDelimiters(value: string): string {
- return value.replace(/<(\/?untrusted_ocr_data)(?=[\s>])/gi, '<$1')
+ return value.replace(/</g, '<')
}📝 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
| function escapeUntrustedOcrBlockDelimiters(value: string): string { | |
| return value.replace(/<(\/?untrusted_ocr_data)(?=[\s>])/gi, '<$1') | |
| } | |
| function escapeUntrustedOcrBlockDelimiters(value: string): string { | |
| return value.replace(/</g, '<') | |
| } |
🧰 Tools
🪛 ast-grep (0.44.1)
[warning] 430-430: Avoid hand-rolled HTML escaping (replacing characters with HTML entities); use a vetted encoder/sanitizer such as DOMPurify or sanitize-html.
Context: value.replace(/<(/?untrusted_ocr_data)(?=[\s>])/gi, '<$1')
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation ('Cross-site Scripting').
(manual-sanitization-typescript)
🤖 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 `@src/main/agent/deepchat/runtime/contextBuilder.ts` around lines 430 - 432,
Update escapeUntrustedOcrBlockDelimiters to escape every angle bracket in the
input, rather than pattern-matching only untrusted_ocr_data tags. Ensure both
“<” and “>” are encoded so any OCR text resembling an HTML-like delimiter cannot
bypass the trust boundary.
Source: Linters/SAST tools
| const statusLoading = ref(false) | ||
| const cacheClearInFlight = ref(false) | ||
| const clearDialogOpen = ref(false) | ||
| const statusErrorNotified = ref(false) |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Repeated status-poll failures after first success stay silent forever.
statusErrorNotified blocks the failure toast whenever status.value is already set (line 381), and it's only reset to false on success (line 379). Once the initial load succeeds, subsequent polling failures (e.g., the OCR runtime process crashing) never surface any indication — the panel keeps showing stale "available" data indefinitely.
🩹 Suggested approach
try {
status.value = await ocrClient.getRuntimeStatus()
statusErrorNotified.value = false
} catch {
- if (!status.value && !statusErrorNotified.value) {
+ if (!statusErrorNotified.value) {
statusErrorNotified.value = true
showFailure('settings.ocr.statusLoadFailed')
}
} finally {Alternatively, surface a lightweight "stale" indicator on the status card instead of a toast on every repeated failure.
Also applies to: 374-388
🤖 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 `@src/renderer/settings/components/OcrSettings.vue` at line 256, Update the
status polling error handling around statusErrorNotified and the logic near
status.value so failures after a prior successful poll are surfaced instead of
remaining silent; preserve deduplication by notifying only on the first failure
in each failure period, and reset that failure state when a subsequent poll
succeeds. Ensure the status card no longer presents stale available data without
an indication of polling failure.
Comment on lines
558
to
+563
| function handleKeydown(e: KeyboardEvent) { | ||
| if (!props.editable) { | ||
| e.preventDefault() | ||
| return | ||
| } | ||
|
|
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Non-editable guard blocks copy and Tab-out, not just editing.
if (!props.editable) { e.preventDefault(); return } runs before any key-specific logic, so while the box is temporarily non-editable (attachment preparation/blocked state) it also blocks Ctrl/Cmd+C, Ctrl/Cmd+A, and Tab — preventing users from copying their draft text or tabbing focus away, effectively trapping keyboard focus in a disabled input.
🐛 Proposed fix to allow navigation/copy through
function handleKeydown(e: KeyboardEvent) {
if (!props.editable) {
- e.preventDefault()
- return
+ const isCopyOrSelectAll =
+ (e.metaKey || e.ctrlKey) && ['c', 'a'].includes(e.key.toLowerCase())
+ const isNavigationKey = [
+ 'ArrowUp', 'ArrowDown', 'ArrowLeft', 'ArrowRight', 'Home', 'End', 'Tab', 'Escape'
+ ].includes(e.key)
+ if (!isCopyOrSelectAll && !isNavigationKey) {
+ e.preventDefault()
+ }
+ return
}📝 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
| function handleKeydown(e: KeyboardEvent) { | |
| if (!props.editable) { | |
| e.preventDefault() | |
| return | |
| } | |
| function handleKeydown(e: KeyboardEvent) { | |
| if (!props.editable) { | |
| const isCopyOrSelectAll = (e.metaKey || e.ctrlKey) && ['c', 'a'].includes(e.key.toLowerCase()) | |
| const isNavigationKey = ['ArrowUp', 'ArrowDown', 'ArrowLeft', 'ArrowRight', 'Home', 'End', 'Tab', 'Escape'].includes(e.key) | |
| if (!isCopyOrSelectAll && !isNavigationKey) { | |
| e.preventDefault() | |
| } | |
| return | |
| } |
🤖 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 `@src/renderer/src/components/chat/ChatInputBox.vue` around lines 558 - 563,
Update handleKeydown so the non-editable guard does not prevent Tab navigation
or standard copy/select-all shortcuts (Ctrl/Cmd+Tab, Ctrl/Cmd+C, and Ctrl/Cmd+A
as applicable); allow those key-specific paths to execute before returning for
other editing commands. Preserve the existing prevention behavior for actions
that modify the draft while props.editable is false.
| "reasons": { | ||
| "automatic_ocr_disabled": "OCR אוטומטי כבוי.", | ||
| "image_dimensions_exceeded": "ממדי התמונה המפוענחת חורגים ממגבלת הבטיחות.", | ||
| "image_limit_exceeded": "התור הזה מכיל יותר תמונות מכפי שמנוע ה־OCR יכול לעבד.", |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Use a turn-specific Hebrew term instead of “queue”.
Lines [431] and [440] translate the current turn as התור (“the queue”), which can be confused with the pending-input queue. Use wording that clearly means the current turn/round.
Also applies to: 440-440
🤖 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 `@src/renderer/src/i18n/he-IL/chat.json` at line 431, Update the Hebrew
translations for image_limit_exceeded and the corresponding message at line 440
to replace התור with a term clearly meaning the current turn or round, while
preserving the existing message meaning and formatting.
Comment on lines
+15
to
+26
| const sqliteModule = await import('better-sqlite3-multiple-ciphers').catch(() => null) | ||
| let sqliteAvailable = false | ||
| if (sqliteModule) { | ||
| try { | ||
| const smokeDatabase = new sqliteModule.default(':memory:') | ||
| smokeDatabase.close() | ||
| sqliteAvailable = true | ||
| } catch { | ||
| sqliteAvailable = false | ||
| } | ||
| } | ||
| const persistentIt = sqliteAvailable ? it : it.skip |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check whether CI/test setup forces the native module to be present, or allows silent skip.
fd -t f 'vitest*.config*' -x cat {}
rg -nC3 'better-sqlite3-multiple-ciphers' package.json
rg -nC3 -- 'sqliteAvailable|persistentIt' test/main/ocr/ocrArtifactStore.test.tsRepository: ThinkInAIXYZ/deepchat
Length of output: 8738
🏁 Script executed:
#!/bin/bash
# Search for CI configuration files
find . -type f \( -name '.gitlab-ci.yml' -o -name '*.yml' -o -name '*.yaml' \) -path '*/.github/workflows/*' | head -20
cat .github/workflows/*.yml 2>/dev/null | head -200
# Check test setup files
cat test/setup.ts 2>/dev/nullRepository: ThinkInAIXYZ/deepchat
Length of output: 15808
🏁 Script executed:
#!/bin/bash
# Look for build/installation scripts and README hints
cat package.json | grep -A5 -B5 '"scripts"'
cat README.md 2>/dev/null | head -100
# Check if there are any environment variable checks or CI-specific behavior
rg -n 'CI|BUILD|NODE_ENV|GITHUB_' test/main/ocr/ocrArtifactStore.test.ts test/setup.tsRepository: ThinkInAIXYZ/deepchat
Length of output: 7432
🏁 Script executed:
#!/bin/bash
# Check how persistentIt is actually used and whether failures are expected
rg -B5 -A15 'persistentIt' test/main/ocr/ocrArtifactStore.test.ts | head -80
# Look for similar patterns in other test files for comparison
fd -t f '*.test.ts' -x rg -l 'it.skip|skipIf' {} \; | head -5Repository: ThinkInAIXYZ/deepchat
Length of output: 2530
🏁 Script executed:
#!/bin/bash
# Check prcheck.yml to see unit test execution
cat .github/workflows/prcheck.yml | head -200Repository: ThinkInAIXYZ/deepchat
Length of output: 4630
🏁 Script executed:
#!/bin/bash
# Verify if there's any explicit enforcement or CI-specific behavior for sqlite availability
rg -n 'CI|SKIP|sqlite' test/main/ocr/ocrArtifactStore.test.ts
# Check if there are other conditional test skips in the codebase for comparison
fd -t f '*.test.ts' -x rg -l 'it\.skip|skipIf' {} \; 2>/dev/null | head -5Repository: ThinkInAIXYZ/deepchat
Length of output: 542
Enforce native SQLite availability in CI for encrypted persistence tests. The persistentIt tests (lines 110, 127) currently skip silently if better-sqlite3-multiple-ciphers fails to load, leaving encrypted persistence with zero coverage in the standard PR check workflow. The prcheck.yml does not run test:main, and there is no assertion that sqliteAvailable === true.
Apply the same environment-variable enforcement used in memory-native-validation (lines 52–55 of prcheck.yml) by either:
- Adding
DEEPCHAT_REQUIRE_NATIVE_SQLITE: '1'when running the OCR artifact store tests in CI, or - Asserting
sqliteAvailable === truewith a hard fail when the flag is set.
This mirrors the pattern already established for memory tests and ensures the encrypted-persistence path is actually exercised before merge.
🤖 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 `@test/main/ocr/ocrArtifactStore.test.ts` around lines 15 - 26, Ensure
encrypted persistence tests cannot be silently skipped in required CI: update
the OCR artifact store test setup around sqliteAvailable and persistentIt to
honor DEEPCHAT_REQUIRE_NATIVE_SQLITE, failing when the flag is set but
better-sqlite3-multiple-ciphers is unavailable; also configure that environment
variable for the CI command running these tests if needed.
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
resources/model-db/providers.json (1)
236574-236580: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winClamp Gemini 3.6 Flash output to 65536
resources/model-db/providers.json:236576-236578advertises"output": 1048576, but this model’s published max output is 65,536 tokens. That value should be lowered to match the actual limit.🤖 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 `@resources/model-db/providers.json` around lines 236574 - 236580, Update the Gemini 3.6 Flash model entry’s limit.output value from 1048576 to 65536, while leaving its context and tool_call settings 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.
Outside diff comments:
In `@resources/model-db/providers.json`:
- Around line 236574-236580: Update the Gemini 3.6 Flash model entry’s
limit.output value from 1048576 to 65536, while leaving its context and
tool_call settings unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 19abf7c0-bc5f-4a74-8284-a134be85d1f8
📒 Files selected for processing (15)
resources/acp-registry/registry.jsonresources/model-db/providers.jsonscripts/afterPack.jsscripts/smoke-light-ocr.jssrc/main/ocr/lightOcrNativePayload.tssrc/main/ocr/lightOcrProcessHost.tssrc/main/ocr/ocrRuntimeAssetResolver.tssrc/main/ocr/ocrRuntimeService.tstest/main/ocr/attachmentCapabilityRouter.test.tstest/main/ocr/lightOcrNativePayload.test.tstest/main/ocr/lightOcrProcessHost.test.tstest/main/ocr/ocrRuntimeAssetResolver.test.tstest/main/ocr/routes.test.tstest/main/scripts/afterPack.test.tstest/main/scripts/smokeLightOcr.test.ts
🚧 Files skipped from review as they are similar to previous changes (8)
- test/main/ocr/routes.test.ts
- test/main/ocr/ocrRuntimeAssetResolver.test.ts
- src/main/ocr/ocrRuntimeService.ts
- src/main/ocr/ocrRuntimeAssetResolver.ts
- src/main/ocr/lightOcrProcessHost.ts
- test/main/scripts/afterPack.test.ts
- resources/acp-registry/registry.json
- test/main/ocr/attachmentCapabilityRouter.test.ts
Contributor
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 `@test/main/scripts/lightOcrPackageSize.test.ts`:
- Around line 94-107: Extend the assertions in the test covering applicationStep
and feishuStep so both unsigned steps are verified not to contain CSC_LINK or
CSC_KEY_PASSWORD. Keep the existing signed cuaStep assertions 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
Run ID: 83b44083-fcc9-4fdc-818c-9c660367df64
📒 Files selected for processing (2)
.github/actions/light-ocr-package-size/action.ymltest/main/scripts/lightOcrPackageSize.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/actions/light-ocr-package-size/action.yml
Comment on lines
+94
to
+107
| expect(applicationStep).toBeDefined() | ||
| expect(applicationStep).not.toContain('CSC_LINK:') | ||
| expect(cuaStep).toBeDefined() | ||
| expect(cuaStep).toContain('CSC_LINK: ${{ inputs.csc-link }}') | ||
| expect(cuaStep).toContain('CSC_KEY_PASSWORD: ${{ inputs.csc-key-password }}') | ||
| expect(cuaStep).toContain( | ||
| "build_for_release: ${{ inputs.platform == 'darwin' && '2' || '' }}" | ||
| ) | ||
| expect(cuaStep?.indexOf('build_for_release:')).toBeLessThan( | ||
| cuaStep?.indexOf('pnpm --dir .ocr-size-base run plugin:bundle') ?? -1 | ||
| ) | ||
| expect(feishuStep).toBeDefined() | ||
| expect(feishuStep).not.toContain('CSC_LINK:') | ||
| }) |
Contributor
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
Assert that both signing secrets are absent from unsigned steps.
The test only checks CSC_LINK; it can pass if CSC_KEY_PASSWORD is accidentally exposed to the baseline application or Feishu step. Assert both signing inputs are absent from both unsigned steps.
Proposed test adjustment
- expect(applicationStep).not.toContain('CSC_LINK:')
+ expect(applicationStep).not.toContain('CSC_LINK:')
+ expect(applicationStep).not.toContain('CSC_KEY_PASSWORD:')
...
- expect(feishuStep).not.toContain('CSC_LINK:')
+ expect(feishuStep).not.toContain('CSC_LINK:')
+ expect(feishuStep).not.toContain('CSC_KEY_PASSWORD:')📝 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
| expect(applicationStep).toBeDefined() | |
| expect(applicationStep).not.toContain('CSC_LINK:') | |
| expect(cuaStep).toBeDefined() | |
| expect(cuaStep).toContain('CSC_LINK: ${{ inputs.csc-link }}') | |
| expect(cuaStep).toContain('CSC_KEY_PASSWORD: ${{ inputs.csc-key-password }}') | |
| expect(cuaStep).toContain( | |
| "build_for_release: ${{ inputs.platform == 'darwin' && '2' || '' }}" | |
| ) | |
| expect(cuaStep?.indexOf('build_for_release:')).toBeLessThan( | |
| cuaStep?.indexOf('pnpm --dir .ocr-size-base run plugin:bundle') ?? -1 | |
| ) | |
| expect(feishuStep).toBeDefined() | |
| expect(feishuStep).not.toContain('CSC_LINK:') | |
| }) | |
| expect(applicationStep).toBeDefined() | |
| expect(applicationStep).not.toContain('CSC_LINK:') | |
| expect(applicationStep).not.toContain('CSC_KEY_PASSWORD:') | |
| expect(cuaStep).toBeDefined() | |
| expect(cuaStep).toContain('CSC_LINK: ${{ inputs.csc-link }}') | |
| expect(cuaStep).toContain('CSC_KEY_PASSWORD: ${{ inputs.csc-key-password }}') | |
| expect(cuaStep).toContain( | |
| "build_for_release: ${{ inputs.platform == 'darwin' && '2' || '' }}" | |
| ) | |
| expect(cuaStep?.indexOf('build_for_release:')).toBeLessThan( | |
| cuaStep?.indexOf('pnpm --dir .ocr-size-base run plugin:bundle') ?? -1 | |
| ) | |
| expect(feishuStep).toBeDefined() | |
| expect(feishuStep).not.toContain('CSC_LINK:') | |
| expect(feishuStep).not.toContain('CSC_KEY_PASSWORD:') | |
| }) |
🤖 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 `@test/main/scripts/lightOcrPackageSize.test.ts` around lines 94 - 107, Extend
the assertions in the test covering applicationStep and feishuStep so both
unsigned steps are verified not to contain CSC_LINK or CSC_KEY_PASSWORD. Keep
the existing signed cuaStep assertions unchanged.
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
resources/model-db/providers.json (1)
260548-260614: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winFix the Gemma model types in
resources/model-db/providers.jsongoogle/gemma-4-26b-a4b-it,google/gemma-4-26b-a4b-it:free,google/gemma-4-31b-it, andgoogle/gemma-4-31b-it:freeare text-only, sotype: "imageGeneration"is incorrect here.🤖 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 `@resources/model-db/providers.json` around lines 260548 - 260614, Update the provider entries for google/gemma-4-26b-a4b-it, google/gemma-4-26b-a4b-it:free, google/gemma-4-31b-it, and google/gemma-4-31b-it:free to use the appropriate text-model type instead of type: "imageGeneration"; preserve their existing text output and capability metadata.
🤖 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.
Outside diff comments:
In `@resources/model-db/providers.json`:
- Around line 260548-260614: Update the provider entries for
google/gemma-4-26b-a4b-it, google/gemma-4-26b-a4b-it:free,
google/gemma-4-31b-it, and google/gemma-4-31b-it:free to use the appropriate
text-model type instead of type: "imageGeneration"; preserve their existing text
output and capability metadata.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f2f92920-45c6-4610-81cd-5e0feb466c71
📒 Files selected for processing (10)
docs/features/light-ocr-integration/plan.mddocs/features/light-ocr-integration/spec.mddocs/features/light-ocr-integration/tasks.mdelectron-builder.ymlresources/acp-registry/registry.jsonresources/model-db/providers.jsonscripts/apple-notarization.jsscripts/notarize-dmg.jsscripts/notarize.jstest/main/scripts/notarizeDmg.test.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- docs/features/light-ocr-integration/spec.md
- docs/features/light-ocr-integration/plan.md
- resources/acp-registry/registry.json
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.
Summary
Integrate
@arcships/light-ocrinto DeepChat to provide fully offline OCR for image attachments, especially when the selected model does not support vision.What changed
Runtime and packaging
@arcships/light-ocr@0.3.0.ppocrv6-small-native-20260719.1.24.14.1.Safety and reliability
Scope
Included:
Not included:
Validation
App.startup.test.tsmock failures outside this diff.Summary by CodeRabbit