feat(provider): add TypeSafe Jev provider and judgment model - #2335
Conversation
TypeSafe's Jev is a System One decision model, not a chat model: its documented surface is a single POST /v1/systemone that evaluates typed questions against a state and returns typed answers. It has no chat messages, streaming, tool calls, or text generation, so it cannot reuse any existing @ai-sdk/* transport. Add a `jev` protocol served by a dedicated JevProvider: - discovery and connection check use the authenticated GET /v1/models call, which spends no tokens; - every chat-shaped entry point refuses instead of issuing a request, so selecting a Jev model as a chat model fails at selection time rather than at runtime; - the API key stays in the main process and is sent only to the configured base URL. Add ModelType.Judgment so these models are never offered by a chat picker, and exclude them from type-less ModelSelect pickers to enforce that at selection time. Ship a disabled built-in `typesafe` profile with a static fallback catalog, and let custom providers select the same protocol, which is added to the import and deeplink allow-lists so an imported config keeps its api type instead of degrading to openai-completions. `jev` is deliberately not registered in PROVIDER_API_TYPE_REGISTRY: that registry maps to an AiSdkProviderDefinition and would route the protocol to a transport that cannot express it. The instance branch mirrors how ollama is already handled. Refs #2326
assistantModel serves five readers: permission review, compaction, session titles, translation, and memory consolidation. Issue #2326 wants a System One (Jev) reviewer, which is impossible while review and compaction share one setting. Add a separate, opt-in `judgmentModel` slot restricted to ModelType.Judgment models and read only by the permission reviewer. Compaction and the other four readers stay on assistantModel: Jev does not generate text, so it cannot produce the rolling summary that compaction needs, and the issue's non-goals keep compaction where it is. When the slot is set, review issues one System One call and composes the typed answers in code. When it is unset, the existing generative path runs unchanged. Safety floors are enforced in code and are not overridable by the model: critical still blocks, high still asks the user, and failure, timeout, or malformed answers ask the user. The generative path's actionHash echo is replaced by code-side binding, since Jev cannot echo anything, which keeps a verdict bound to one exact action and its arguments. Rationale is fixed local copy derived from the classification, not model-authored text. Questions, thresholds, and composition live in one file because TypeSafe's own guidance is that those are the parts a human must review. Every threshold is provisional pending the evaluation the issue requires before adoption. Refs #2326
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (83)
Included review availability: Your plan provides up to 8 included reviews per hour; 7 remain after this review. 📝 WalkthroughWalkthroughThe change adds an opt-in Jev judgment model, a TypeSafe System One provider, typed permission-review composition, judgment-model configuration, picker filtering, localization, and related tests. The existing generative review path remains when no judgment model is configured. ChangesJev judgment review
Priority: ➖ Normal Estimated code review effort: 4 (Complex) | ~60 minutes Change: Feature Sequence Diagram(s)sequenceDiagram
participant AgentSettings
participant ToolPermissionReviewer
participant ProviderRuntime
participant JevProvider
participant TypeSafeSystemOne
AgentSettings->>ToolPermissionReviewer: configure judgmentModel
ToolPermissionReviewer->>ProviderRuntime: runJudgment(state, questions)
ProviderRuntime->>JevProvider: runJudgment(request, signal)
JevProvider->>TypeSafeSystemOne: POST /v1/systemone
TypeSafeSystemOne-->>JevProvider: typed answers
JevProvider-->>ProviderRuntime: JevJudgmentResult
ProviderRuntime-->>ToolPermissionReviewer: review result
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 29.17% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 24 functions across 26 files. (57 skipped: 57 unsupported.)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 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 |
zhangmo8
left a comment
zhangmo8
left a comment
There was a problem hiding this comment.
Reviewed at 246ce7477. I read the code rather than only the description, and verified locally: pnpm run typecheck clean, pnpm run format:check clean, and the three touched test files pass (28 tests). CI's test-renderer job also passes at this SHA.
What holds up
- The non-chat guarantee is real, not just documented: every chat-shaped entry point refuses,
coreStreamemits an error event instead of issuing a request, and discovery reportsModelType.Judgmentso the type-level filters keep these models out. - The safety floors are enforced in code and cannot be overridden by the model,
rationaleis fixed local copy rather than model-authored text, and failure / timeout / malformed answers all resolve toask_user. - Binding the verdict with the caller-computed
actionHashinstead of a model echo is the right call for a model that cannot echo, and it removes a whole class of "verdict applies to the wrong action" bugs. - The new tests pin contracts that matter rather than implementation shape: non-chat refusal, the System One request body, the
critical/highfloors, each auto-allow threshold, failure and malformed answers, abort propagation, and the unchanged generative path when the slot is unset.
Findings
Ordered by how much they change behavior. Nothing here blocks a draft, but the first two are worth settling before this leaves draft.
- The static fallback catalog appears unreachable from the picker it is meant to populate, and an empty catalog fetch can clear the cache instead of falling back to it (inline on
defaults.ts). - Under the judgment path a medium-risk action can never be auto-allowed, which makes it strictly more interruptive than the reviewer it replaces — and that policy difference sits in a literal rather than in the threshold block (inline on
jevPermissionQuestions.ts). - The judgment branch skips the post-call abort re-check the generative path performs (inline on
toolPermissionReviewer.ts). - Tool results are truncated from the head only, which limits what the injection question can see (inline on
jevPermissionQuestions.ts). - The new picker exclusion has no test (inline on
ModelSelect.vue). - Minor: unused protocol helper, and a redundant provider-id branch (inline on
jevProtocol.tsandproviderInstanceManager.ts).
Note on the failures I did not report: this machine times out two cases in DeepChatAgentsSettings.test.ts (10s limit), but the base commit 074890266 times out the same two under identical conditions and CI passes test-renderer at this SHA, so I attribute that to the machine, not to this change.
| enable: false, | ||
| // Static fallback so the judgment-model picker is populated before the first catalog refresh. | ||
| // Live discovery from `GET /v1/models` stays authoritative once it succeeds. | ||
| models: [ |
There was a problem hiding this comment.
The static fallback catalog looks unreachable from the picker it is meant to populate. ModelSelect renders modelStore.enabledModels (src/renderer/src/components/ModelSelect.vue:108), and that store is filled from getDbProviderModels (there is no typesafe entry in the provider DB) plus the main-process per-provider model store (src/renderer/src/stores/modelStore.ts:677-717). That store is only ever written by BaseLLMProvider.fetchModels (src/main/provider/baseProvider.ts:314); nothing seeds it from DEFAULT_PROVIDERS[].models. providerModelHelper defaults to models: [] (src/main/provider/providerModelHelper.ts:104-107) and reads only its own store (:239-246), so these two static entries never reach a picker.
There is a second half to this: JevProvider.fetchProviderModels returns [] when apiKey is empty (:228) and on any catalog error (:233-236), and fetchModels persists whatever it receives — so enabling TypeSafe before entering a key, or one transient /v1/models failure, stores an empty catalog and clears anything previously discovered, rather than falling back to the static entries.
If the fallback is meant to be user-visible, it needs a seeding path (or a rule that an empty fetch must not overwrite a non-empty cache). If it is not meant to be user-visible, the claim in the description should be corrected. Please confirm which one is intended.
| : 0 | ||
|
|
||
| const mayAutoAllow = | ||
| riskLevel === 'low' && |
There was a problem hiding this comment.
mayAutoAllow is hardcoded to riskLevel === 'low', so under the judgment path a medium-risk action can never be auto-allowed. The generative reviewer this replaces explicitly allows medium ("Allow low and medium risk actions", toolPermissionReviewer.ts:206), and the medium criteria defined above (line 65: "installing a dependency, or running a routine local command") describe exactly the routine actions the current reviewer approves without asking.
So switching an agent to a judgment model will make it noticeably more interruptive, which may well be the intent — but the difference lives in a literal instead of in JEV_REVIEW_THRESHOLDS, where the rest of the policy is reviewable. Suggest an explicit autoAllowMaxRiskLevel threshold and one line in the spec stating the intended difference between the two paths.
| if (judgmentProviderId && judgmentModelId) { | ||
| // Bound by the same review timeout as the generative path so a stalled judgment still falls | ||
| // back to asking the user instead of hanging the permission flow. | ||
| return await reviewWithJudgmentModel( |
There was a problem hiding this comment.
The judgment branch returns straight out of reviewWithJudgmentModel, so it skips the throwIfAbortRequested(context.signal) re-check that the generative path performs after its provider call (lines 395 and 418). If the user cancels while the judgment call is in flight and the call still resolves, a verdict is returned for a turn that was already cancelled. Adding the same re-check after the judgment call makes the two paths behave alike.
Same family, lower impact: JevProvider.createRequestSignal (src/main/provider/providers/jevProvider.ts:301-317) registers the parent abort listener without checking callerSignal.aborted first, so an already-aborted caller signal is silently dropped — unlike coreStream, which calls throwIfAborted() on entry (:180).
| * to see them. | ||
| */ | ||
| export const JEV_REVIEW_MAX_RECENT_MESSAGES = 6 | ||
| export const JEV_REVIEW_MAX_CONTENT_CHARS = 1500 |
There was a problem hiding this comment.
Tool results are deliberately retained because they are a primary injection vector, but truncateReviewText keeps only the head of each message (value.slice(0, maxChars), toolPermissionReviewer.ts:65). An instruction that tries to steer the decision and appears after the first 1500 characters of a long tool result never reaches the injection_pressure question — the control this is supposed to feed. Head+tail truncation, or a larger budget for role: 'tool' messages, would close that gap.
| !props.type || | ||
| props.type.length === 0 || | ||
| (model.type !== undefined && props.type.includes(model.type as ModelType)) | ||
| props.type && props.type.length > 0 |
There was a problem hiding this comment.
This changes the contents of every type-less picker, but nothing tests it: test/renderer/components/ModelSelect.test.ts is untouched, and the settings test only asserts the prop wiring ([ModelType.Judgment] is passed for the judgment slot). Since "a chat-shaped picker never lists a judgment model" is the user-visible guarantee this PR calls out, please add a ModelSelect case covering both directions: absent with no type, present with type=[Judgment].
| return answer.type === 'choice' | ||
| } | ||
|
|
||
| export function isJevScoreAnswer(answer: JevAnswer): answer is JevScoreAnswer { |
There was a problem hiding this comment.
isJevScoreAnswer has no caller in src/ or test/; JevScoreQuestion/JevScoreAnswer are only referenced from the JevQuestion/JevAnswer unions. Likewise jevProvider.ts:320 re-exports JevQuestion even though every consumer imports it from @shared/jevProtocol. Either use them or drop them — an unused type guard on a protocol boundary reads as a supported answer type that the reviewer does not actually handle.
| return new OllamaProvider(provider, this.options.providerSettings, this.options.locale) | ||
| } | ||
|
|
||
| if (provider.id === 'typesafe' || provider.apiType === 'jev') { |
There was a problem hiding this comment.
provider.id === 'typesafe' is redundant with apiType === 'jev' for the built-in, and it carries a foot-gun: if a user edits the built-in TypeSafe entry to a different apiType (for example to point it at an OpenAI-compatible endpoint), it still resolves to JevProvider, which refuses every chat call. Prefer the apiType-only check unless the id branch is load-bearing for a case I am missing.
Provider marks are not a `websites.icon` field — no code reads that. The convention is a static asset plus a registry entry, which the original provider commit missed. Add the official square mark from typesafe.ai under assets/llm-icons/, and register it under both `typesafe` (the provider id) and `jev` (the api type, which also covers the `jev-latest` and `jev-1.13.0` model ids). The mark is colour on its own background, so it is deliberately not added to monoIconUrls, which drives dark-mode inversion for monochrome currentColor marks. Verified that adding these two keys changes no existing provider's resolved icon: 73 of 75 built-in providers already resolved, and the only two resolutions that change are the new ones. Refs #2326
Make the bundled Jev catalog load-bearing rather than decorative. It is now returned whenever the live catalog is unavailable or empty, because `BaseLLMProvider.fetchModels` persists whatever the provider returns and an empty list would clear a previously discovered catalog. The earlier claim that the static entries populated the picker on their own was wrong: the renderer reads the persisted per-provider model store, and nothing seeds it from `DEFAULT_PROVIDERS[].models`. Promote the auto-allow risk cap from a literal to `autoAllowMaxRiskLevel` so the intentional difference from the generative path — which allows medium — is reviewable, and record its consequence for the evaluation: the two paths do not share a policy, so interruption counts do not isolate the model. Restore parity with the generative path on cancellation: the judgment branch now performs the same post-call abort re-check, so a judgment that resolves after cancellation cannot return a verdict for a cancelled turn. The provider also stops silently dropping an already-aborted caller signal. Keep the tail as well as the head of each message in the judgment state. Head-only truncation hid an instruction placed at the end of a long tool result, which is exactly what the injection question exists to see. The generative path's truncation is unchanged. Select the provider by api type only. The id branch was redundant — the built-in already declares `apiType: 'jev'` — and it would pin the provider to JevProvider if a user repointed that entry. Drop the unused `isJevScoreAnswer` guard and the redundant `JevQuestion` re-export, which read as support for an answer type nothing handles. Refs #2326
|
Addressed all six in 1. Fallback catalog — you were right, and I had it backwardsBoth halves confirmed before changing anything:
Answering your question directly: the fallback is meant to be user-visible, so it now has a real path to the picker. Three cases are now pinned by tests. 2. Medium risk — promoted to a named threshold, and documented
You are right that this makes the path strictly more interruptive, and I recorded the consequence you implied: the two paths do not share a policy, so an evaluation that counts interruptions is not measuring the model alone. The spec now says the cap must be aligned before drawing a conclusion about Jev's judgment quality. A test pins 3. Cancellation parityBoth fixed. The judgment branch now re-checks 4. Truncation directionHead+tail in the judgment state. I did not change the generative path's truncation, since its prompt content is the baseline the evaluation compares against and changing it belongs in its own change. Worth noting the same latent weakness exists there. 5. ModelSelect testAdded, both directions: absent with no 6. CleanupsDropped Provider selection is now api-type-only. On the two
|
zerob13
left a comment
zerob13
left a comment
There was a problem hiding this comment.
Reviewed at f07402f9c (54 files, +2128/-23). Two things block; the rest are smaller and listed after.
Blocking
1. The bundled fallback catalog disappears after a provider reorder
JevProvider.fetchProviderModels falls back to this.provider.models (src/main/provider/providers/jevProvider.ts:232), which is the copy of the static catalog stored in the providers settings JSON. That copy does not survive a provider reorder. The settings sidebar reorders by sending provider summaries, and getProviderSummaries() omits models, customModels, enabledModels and disabledModels (LlmProviderSummarySchema, src/shared/contracts/domainSchemas.ts:429); reorderProvidersAtomic then writes the received array over the whole providers list (src/main/provider/providerHelper.ts:237). After any drag or move up/down, the typesafe entry has no models key, and getProviders() passes entries that already have apiType through unchanged, so nothing restores it.
This matters because the fallback is the only thing standing between a failed fetch and an empty picker. BaseLLMProvider.fetchModels persists whatever the provider returns (src/main/provider/baseProvider.ts:313), so an empty return clears the per-provider model store the picker actually reads. Net effect: after a reorder, the first failed or key-less refresh wipes the last-known TypeSafe catalog and the judgment picker goes empty — the exact regression this fallback was added to prevent.
Suggested fix: prefer the last-known catalog and use the static seed only when there is none.
const bundled = this.models.length > 0 ? this.models : this.getBundledCatalog()this.models is already loaded from the per-provider store in the base constructor (loadCachedModels, src/main/provider/baseProvider.ts:180-189), so it survives a reorder. The comment at jevProvider.ts:228-231 and docs/features/typesafe-jev-provider/spec.md:114-123 should be corrected with it — they claim the fallback protects the previously discovered catalog, which is only true while the settings-JSON seed survives. (The reorder path dropping those four arrays is pre-existing and probably deserves its own issue; it is not something to fix here.)
2. Jev models are still selectable in the MCP sampling picker
ModelSelect now hides ModelType.Judgment when no type filter is passed, but the other model picker in the app did not get the same treatment. ModelChooser returns every enabled model when type is empty (src/renderer/src/components/ModelChooser.vue:135-141), and its only caller — the MCP sampling dialog — does not pass a type (src/renderer/src/components/mcp/McpSamplingDialog.vue:187). A Jev model can be selected there, and the request reaches generateCompletionStandalone (src/main/mcp/mcpClient.ts:1064) before failing with jev-unsupported-capability.
The spec lists this as an invariant and an acceptance criterion (docs/features/typesafe-jev-provider/spec.md:173,194), so either the filter goes into ModelChooser (one line, same condition), or the spec should say which surfaces are actually covered.
Should fix
3. Imported jev providers put Jev models in chat pickers
jev was added to the import allow-list, but the models built during import carry no type (src/main/provider/providerImportService.ts:1388-1400), and the picker filter treats "no type" as "not a judgment model". An imported Jev model therefore appears in every chat picker and fails only at runtime. Tag them with ModelType.Judgment when the target api type is jev.
4. Out-of-range probabilities are accepted
readNoulProbability and the confidence read only check typeof === 'number' && Number.isFinite (src/main/agent/deepchat/runtime/jevPermissionQuestions.ts:119-122,197-206). A noul or confidence outside [0, 1] is invalid output, but it can only ever push toward auto_allow, because authorization >= 0.8 and confidence >= 0.6 are both satisfied by an oversized value. Everything else in that file fails closed; this is the one input that does not. Reject values outside [0, 1] so they resolve to ask_user, or clamp them.
This is not reachable through prompt injection — it needs the endpoint to return off-spec numbers — but the code already tried to validate here and stopped halfway, and a silent scale change on TypeSafe's side would read as "strong yes" instead of an error.
5. Two more surfaces don't know about the new type
- The import dialog renders the raw string
jev, becauseapiTypeLabelhas no case for it andsettings.data.providerImport.apiTypeshas no key (src/renderer/settings/components/ProviderConfigImportDialog.vue:831-851). - The model manager cannot filter by judgment:
TYPE_ORDERdoes not include it (src/renderer/settings/components/ProviderModelList.vue:385-393), so theTYPE_ICONSentry added at:408is unreachable and the filter chip never appears.
6. The judgment model sends data to a third party and the UI does not say so
Once a judgment model is configured, tool arguments and the recent conversation go to the configured service. The field is a label plus a picker and nothing else. Worth one line of copy under the field before this leaves draft; it is also absent from the plan's Deferred list, so nothing tracks it.
7. Spec and plan disagree with the code
spec.md:3saysStatus: proposedwhileplan.md:3saysimplemented.plan.md:42still listsid === 'typesafe' || apiType === 'jev'as done; the code is api-type-only.spec.md:173,194— see finding 2.
Minor
- A failed judgment review is invisible to the user. The reviewer returns a generic rationale for
ask_user, the caller drops it, and the only trace is aconsole.warn(toolPermissionReviewer.ts:464-481,dispatch.ts:1694-1704). A misconfigured judgment model silently turns every auto-approve into a prompt. Fail-safe, but there is no way for a user to find out why. spec.md:79-82says the request "is keyed by the action hash". Nothing is keyed by it: the hash is computed and returned inside one call, and no consumer compares it. The binding is real (same call, same request object, no caching), but the wording promises a check that does not exist. Either add the comparison indispatch.tsor soften the sentence.- Thresholds are pinned by direction only, never by value: no test uses authorization exactly
0.8, confidence exactly0.6, or injection exactly0.2, so moving a threshold keeps the suite green.composeJevReviewDecisionis exported and never imported directly — a boundary test there is cheaper than more end-to-end cases. - The failure test asserts
ask_userbut not that the generative path was skipped (test/main/agent/deepchat/runtime/toolPermissionReviewer.test.ts:262-278). One line:expect(generateCompletionStandalone).not.toHaveBeenCalled(). - Head-and-tail truncation ignores the marker length (17 chars over budget) and can split a surrogate pair at the cut (
toolPermissionReviewer.ts:76-79). Cosmetic; the JSON stays valid. release_dateis parsed but never read, andJEV_PERMISSION_QUESTION_IDS,JEV_RISK_LEVELSandJEV_REVIEW_THRESHOLDSare exported without a consumer outside the module.JevProviderre-implements the request-signal helper thatBaseLLMProvider.createModelRequestSignalalready provides (baseProvider.ts:125-161). The base version aborts with a distinguishableprovider_request_timeoutcode; the private copy aborts bare, so a timeout and a caller cancel are indistinguishable downstream.test/renderer/components/DeepChatAgentsSettings.test.ts:1005— the test name still says "image generation" but it now asserts the judgment selector too.ModelConfigDialoghas nojudgmentoption in its type select, so the dialog for a Jev model shows the placeholder (ModelConfigDialog.vue:186-206).
Verified as holding
- The refusal surface is real:
completions/summaries/generateText/summaryTitlesthrow the typed error,coreStreamyields an error event and stops, and no request is issued. Cleanup sits infinally; no unhandled rejections. - The safety floors hold branch by branch.
critical→ block andhigh→ ask_user come before anything else; an unknown or non-string risk level, a missingrisk_level, a missingconfidence, a missing noul signal, and emptyanswersall resolve toask_user. The four gates are an AND, and onlylowcan reachauto_allow. - Explicit user confirmation is never overridden: the reviewer is only consulted in
auto_approvemode (dispatch.ts:2324-2333,3265-3267). - Abort and timeout semantics match the generative path, and an already-aborted caller signal is no longer dropped. Both are pinned by tests.
- Keeping
jevout ofPROVIDER_API_TYPE_REGISTRYis correct: its only consumer resolves an AI SDK factory and warns on a miss, so no Jev model can reach the AI SDK transport. ModelType.Judgmentis additive. The only exhaustiveRecord<ModelType, …>is patched, there is noswitchover model types,isChatSelectableModelTypealready excludes it, andLlmProviderSchema.modelsis optional so stored rows still parse.- The icon keys are safe:
resolveModelIconKeyis a first-substring match over declaration order, and no existing key is a substring oftypesafe/jevor the reverse, so no existing provider's icon changes. - i18n: the key appears exactly once in all 23 locales, the JSON is valid, and
node scripts/validate-i18n.mjspasses. The Tibetan and Mongolian strings are consistent with their neighbours; I cannot vouch for them as a native speaker either. - The settings row reuses the existing field loop instead of bolting on a special case, and no chat-shaped picker can silently select a judgment model (
chatSelectableModelGroupsalready filters byisChatSelectableModelType). runJudgmentstays in the main process; nothing about it crosses preload or IPC, and the API key never leaves main.- The new tests pin contracts rather than shapes: the System One request body, the
{ models: [...] }catalog shape, the three fallback branches, the non-chat refusal, the floors, the medium-risk policy, head+tail retention, and the unchanged generative path.
Fix the fallback preference order. Seeding only from `this.provider.models` was wrong: the settings sidebar reorders by sending provider summaries, which omit `models`, and the reorder writes that array over the whole providers list, so the seed disappears after any drag or move. The last-known catalog (`this.models`, loaded from the per-provider store, which survives the reorder) is now preferred, with the bundled seed used only when nothing has been discovered. Without this the fallback was empty exactly when it was needed, and the next failed or key-less refresh would have wiped the picker. Extend the judgment exclusion to `ModelChooser`, the MCP sampling picker's source. It was the remaining picker that could select a Jev model and reach `generateCompletionStandalone` before failing. Tag imported models as `ModelType.Judgment` when the target api type is `jev`. Imported sources carry no type, which the picker filters read as "not a judgment model", so an imported Jev model landed in every chat picker and failed only at request time. Reject out-of-range probabilities. An oversized `noul` or `confidence` satisfied its `>=` gate, so invalid output could only push toward `auto_allow` — the one input in the composition that did not fail closed. A silent scale change on the provider's side now reads as an error rather than a strong yes. Teach the remaining surfaces about the type: the import dialog's api type label, the model manager's type filter order and chip label, and the model config dialog's type select. Disclose that a configured judgment model sends tool arguments and the recent conversation to the configured service. Delegate the request signal to `BaseLLMProvider.createModelRequestSignal` instead of re-implementing it, so a timeout aborts with `provider_request_timeout` and stays distinguishable from a caller cancel. Pin the policy boundaries by literal value in a `composeJevReviewDecision` test and make the threshold constants module-private, so moving a boundary turns the suite red instead of staying green. Reconcile spec and plan with the code: status, the api-type-only branch, the action-binding wording (nothing compares a returned hash), and the picker surfaces actually covered. Refs #2326
|
Both blockers confirmed and fixed in 1. Fallback catalog after a provider reorder — confirmedI verified both halves before changing anything: Applied your suggestion — the last-known catalog wins, the static seed is only used when nothing has been discovered: const fallback = this.models.length > 0 ? this.models : this.getBundledCatalog()
You are right that my comment and the spec overstated this. Both claimed the fallback protected the previously discovered catalog, which was only true while the settings-JSON seed survived. Corrected in both places, and the reorder path's data loss is now recorded in the plan's Deferred list rather than fixed here — agreed it is pre-existing and deserves its own issue. 2. MCP sampling picker — fixedSame condition as 3. Imported models — fixed
4. Out-of-range probabilities — fixedBoth reads now go through a 5. Surfaces — fixed
6. Privacy disclosure — addedOne line under the judgment field, in all 23 locales. Also added to the plan's Deferred list alongside the other untracked items. 7. Spec and plan — reconciledStatus, the api-type-only branch, and the covered picker surfaces. Also softened the action-binding wording: you were right that nothing compares a returned hash. The spec now says the binding is structural (one call, one request object, no reuse) and explicitly states what it does not claim. MinorsDone: policy boundaries pinned by literal value in a Deliberately not done:
Gates
|
zerob13
left a comment
zerob13
left a comment
There was a problem hiding this comment.
Re-review: delta f07402f → 93551d5 (second review round)
Verdict: Approve. The new commit addresses every blocking and should-fix finding from the previous review, plus the relevant minor ones. I re-verified each fix against the source, ran the affected test suites and the i18n validation on this branch — all pass. No new blocking findings in the delta.
Blocking findings from last round — both resolved
- Fallback catalog wiped after a provider reorder — fixed.
fetchProviderModelsnow prefers the last-known catalog over the bundled seed:this.models.length > 0 ? this.models : this.getBundledCatalog()(jevProvider.ts). This was the exact suggested fix:this.modelsis loaded from the per-provider model store, which survives the reorder that stripsmodelsfrom the settings JSON. A new test ("prefers the last-known catalog over the bundled seed when a later fetch fails") proves a failed refresh after a successful one keeps the discovered catalog instead of regressing to the seed. - Jev models selectable in the MCP sampling picker — fixed.
ModelChooser.vuenow filters outModelType.Judgmentwhen the caller passes no type filter (which is the MCP sampling dialog's case), and still shows them when a picker explicitly requests that type. New test covers both directions.
Should-fix findings — all resolved
- Imported
jevproviders now tag models asModelType.Judgment(providerImportService.tsbuildModelMeta), so imported Jev models no longer leak into chat pickers; tested via the Hermes import path. - Out-of-range
noul/confidencevalues are now rejected by a sharedreadProbabilitygate ([0, 1], finite) and fail closed toask_user; boundary tests pin the exact thresholds by literal value, and the threshold constants are now module-private so the tests are the contract. - The import dialog renders a localized "Jev" api-type label instead of the raw string; i18n keys added across all 23 locales and validation passes.
- The model manager type filter now includes Judgment in its ordered list with a localized label, and the model config dialog's type select offers it.
- The judgment model setting now shows a disclosure that tool arguments and the recent conversation are sent to the configured third-party service (en + zh-CN wording checked, both accurate).
- Spec and plan are reconciled with the code: status updated to implemented, the api-type-only branch rationale documented, the action-binding wording now correctly states the binding is structural (one call, one request object) rather than a returned-hash comparison, and the covered picker surfaces are listed.
Also in the delta (beyond the findings) — good
JevProvider.createRequestSignalnow delegates toBaseLLMProvider.createModelRequestSignalinstead of re-implementing it, so a timeout aborts withprovider_request_timeoutand stays distinguishable from a caller cancel. Signature verified against the base implementation.- Head-and-tail truncation in
toolPermissionReviewer.tsnow counts the truncation marker against the char budget and refuses to cut inside a surrogate pair — a real correctness improvement for non-ASCII content. - The deferred items (silent judgment-failure visibility, and the pre-existing provider-reorder summary drop that strips
models) are now explicitly documented in the plan instead of being silently ignored. Both are correctly out of scope for this PR.
Verification on this branch
pnpm run i18n:validate— passed (23 locales).pnpm exec vitest runonjevPermissionQuestions,toolPermissionReviewer,jevProvider,providerImportService(main) andModelChooser,DeepChatAgentsSettings(renderer): 85/85 passed.
Detailed references
src/main/provider/providers/jevProvider.ts:229-256— fallback preference order.src/renderer/src/components/ModelChooser.vue:136-144— type-less picker judgment exclusion.src/main/provider/providerImportService.ts:1389-1412— judgment tagging on import.src/main/agent/deepchat/runtime/jevPermissionQuestions.ts:119-131,210-213—readProbabilityand confidence fail-closed.src/main/provider/providers/jevProvider.ts:319-334— signal delegation.src/main/agent/deepchat/runtime/toolPermissionReviewer.ts:82-95— surrogate-safe truncation.test/main/provider/jevProvider.test.ts:142-170— last-known catalog test.test/renderer/components/ModelChooser.test.ts:146-157— both-direction exclusion test.
2 participants
Summary
Adds TypeSafe's Jev as a provider protocol and wires it into an opt-in, per-agent
judgment model used for tool-permission review. Refs #2326.
Why it has this shape
Jev is not a chat model. Its entire documented HTTP surface is one decision endpoint:
No chat messages, no streaming, no tool calls, and — by design — typed answers instead of generated
text. It therefore cannot reuse any
@ai-sdk/*transport, andsrc/mainhas no hand-writtenLanguageModelV2/doStreamto extend, so this is a dedicatedBaseLLMProvidersubclass selected byan explicit instance branch (the same shape
ollamaalready uses).jevis deliberately not added toPROVIDER_API_TYPE_REGISTRY. That registry maps a protocol toan
AiSdkProviderDefinitionand would route Jev to a transport that cannot express it.What's implemented
Provider / protocol (
docs/features/typesafe-jev-provider/spec.md)JevProvider:POST /v1/systemone, Bearer auth,GET /v1/modelsdiscovery ({ models: [...] }isnot the OpenAI
{ data: [...] }shape, so it is parsed directly).TypeSafe bills input tokens per request and a generation probe is meaningless here.
unsupported-capability error,
coreStreamyields it as a stream error event.ModelType.Judgmentkeeps these models out of chat pickers;ModelSelectadditionally hides themfrom any picker that does not explicitly request that type.
typesafeprofile with a static fallback catalog so the picker is not emptybefore the first refresh.
apiType: jev;jevjoins the import and deeplinkallow-lists so an imported config keeps its api type instead of degrading to
openai-completions.assets/llm-icons/, registered under both thetypesafeprovider id and thejevapi type. Note the repo convention is a static asset plus amodelIconRegistry.tsentry — not awebsites.iconfield, which no code reads.Agent judgment model (
docs/features/agent-judgment-model/spec.md)judgmentModel, restricted to judgment models, read only by the permissionreviewer.
unset, the existing generative path runs unchanged.
criticalblocks,highasks the user, and failure / timeout / malformed answers ask the user.
actionHashecho is replaced by code-side binding — Jev cannot echoanything — so a verdict stays bound to one exact action and its arguments.
rationaleis fixed local copy derived from the classification, not model-authored text.jevPermissionQuestions.ts) becauseTypeSafe's own guidance is that those are the parts a human must review.
Scope decision worth reviewing
assistantModelactually has five readers, not the two the issue mentions: permission review,compaction, session titles, translation, and memory consolidation. Only the reviewer is repointed.
Compaction specifically cannot move to Jev:
generateRollingSummaryreturns a summary stringvia
BaseLLMProvider.summaries, and Jev does not generate text. This matches the issue's non-goals,which keep compaction on the assistant model.
UI changes
Verification
pnpm run format— cleanpnpm run i18n— 23 locales, no missing or invalid keyspnpm run lint— 0 warnings, 0 errorspnpm run typecheck— node + web cleanpnpm exec vitest run test/main/provider …— 819 passed / 72 files, no regressions in the existing provider suiteonly resolutions that change are the two new ones
New tests pin real contracts: the non-chat guarantee, the System One request body, the
critical/highfloor, each auto-allow threshold, failure and malformed answers resolving toask_user, abort propagation, and the unchanged generative path when the slot is unset.What this PR does NOT do
jevPermissionQuestions.tsis marked provisional. [Feature] 实验性探索：使用 Jev 作为独立的权限审核后端 #2326 treats evidence (false-allow rate, false-block rate, Chinese
authorization, injection resistance, latency, cost) as the condition for adoption — a working API
call is explicitly not a pass. Hence
Refs #2326, notCloses #2326.are sent to the configured third-party service, and the UI says nothing about it. The issue lists
data-sending scope as a to-verify item; this is the most user-facing gap remaining.
src/types/i18n.d.tsis already stale in the repo relative tozh-CN; runningpnpm run i18n:typesrewrites ~336 unrelated lines. The one
judgmentModelleaf was inserted by hand. Wholesaleregeneration belongs in its own change.
vouch for — worth a native check.
in that select (none of them are i18n'd).
opencode-gois the one built-in provider without a registered mark (acplegitimately resolvesits icon dynamically from the ACP registry). Pre-existing, unrelated to this PR.
Summary by CodeRabbit