fix(wework): use task model for friendly titles - #2489
Conversation
📝 WalkthroughWalkthroughThe change adds friendly task title settings, supports task-model and explicit-model selection, validates unavailable models, integrates title configuration into runtime task creation, expands model identifier resolution, improves desktop model selection, preserves seeded goals during hydration, and updates English and Chinese documentation. ChangesFriendly task titles
Estimated code review effort: 3 (Moderate) | ~30 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant GeneralSettingsPage
participant AppPreferences
participant WorkbenchRuntimeMessaging
participant friendlyTitleForTask
User->>GeneralSettingsPage: Enable friendly titles and select a model
GeneralSettingsPage->>AppPreferences: Persist friendly-title preferences
WorkbenchRuntimeMessaging->>friendlyTitleForTask: Evaluate preferences and available models
friendlyTitleForTask->>WorkbenchRuntimeMessaging: Return title configuration or null
WorkbenchRuntimeMessaging->>WorkbenchRuntimeMessaging: Create runtime task with optional title configuration
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
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)
wework/src/components/settings/GeneralSettingsPage.tsx (1)
451-479: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winIncrease the mobile selector height.
Line 461 sets the new
<select>control toh-8, or 32px. This is below the required 44px mobile touch target.Proposed fix
- className="h-8 w-full rounded-md border border-border bg-background px-2 text-sm text-text-primary md:w-[220px]" + className="h-8 w-full rounded-md border border-border bg-background px-2 text-sm text-text-primary max-sm:h-11 md:w-[220px]"As per coding guidelines, “Mobile controls must be at least 44px × 44px.”
🤖 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 `@wework/src/components/settings/GeneralSettingsPage.tsx` around lines 451 - 479, Increase the height of the select control in the friendly task title model selector identified by data-testid="friendly-task-title-model-select" from h-8 to a minimum 44px mobile height, while preserving the existing responsive styling and behavior.Source: Coding guidelines
🤖 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 `@wework/src/components/settings/GeneralSettingsPage.tsx`:
- Around line 451-479: Increase the height of the select control in the friendly
task title model selector identified by
data-testid="friendly-task-title-model-select" from h-8 to a minimum 44px mobile
height, while preserving the existing responsive styling and behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 16c85694-6854-4784-b072-4ca397e87b1a
📒 Files selected for processing (8)
docs/en/wegent/user-guide/coding/managing-code-tasks.mddocs/zh/wegent/user-guide/coding/managing-code-tasks.mdwework/src/components/settings/GeneralSettingsPage.test.tsxwework/src/components/settings/GeneralSettingsPage.tsxwework/src/features/workbench/useWorkbenchRuntimeMessaging.test.tswework/src/features/workbench/useWorkbenchRuntimeMessaging.tswework/src/i18n/locales/en/common.jsonwework/src/i18n/locales/zh-CN/common.json
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
wework/src/api/local/localServices.test.ts (1)
1108-1126: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winAssert each generated catalog ID before reusing it.
codexCatalogModelIdis optional onLocalModelConfig. If ID generation regresses, the request payloads and expected selections can useundefined. The test can then pass without proving custom catalog routing.Assert that the
ollama,lmstudio, andcustomIDs exist after saving. Reuse narrowed string values in the later requests and assertions.Based on the supplied
LocalModelConfiginterface, these fixtures need an explicit catalog-ID contract check.Also applies to: 1149-1176, 1192-1194
🤖 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 `@wework/src/api/local/localServices.test.ts` around lines 1108 - 1126, After saving the ollama, lmstudio, and custom fixtures, assert each config’s codexCatalogModelId is defined before constructing later requests or expected selections. Reuse the narrowed catalog ID values throughout those payloads and assertions so the test explicitly validates catalog routing and never relies on undefined IDs.
🤖 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 `@wework/src/features/model-settings/localModelSettings.ts`:
- Around line 625-631: Update the resolver around readStoredConfigs in
wework/src/features/model-settings/localModelSettings.ts: return null before the
fallback lookup when modelName is null or undefined, preventing optional
identifiers from matching unrelated configurations. In
wework/src/features/model-settings/localModelSettings.test.ts, assert
model.codexCatalogModelId is non-empty before lookup, add a regression fixture
without catalog metadata, and add a slug-only fixture to cover the
catalogEntry.slug branch.
---
Nitpick comments:
In `@wework/src/api/local/localServices.test.ts`:
- Around line 1108-1126: After saving the ollama, lmstudio, and custom fixtures,
assert each config’s codexCatalogModelId is defined before constructing later
requests or expected selections. Reuse the narrowed catalog ID values throughout
those payloads and assertions so the test explicitly validates catalog routing
and never relies on undefined IDs.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 71a0fa68-b3aa-4ef4-8326-3b7647340e60
📒 Files selected for processing (4)
wework/e2e/desktop/task-flow.e2e.mjswework/src/api/local/localServices.test.tswework/src/features/model-settings/localModelSettings.test.tswework/src/features/model-settings/localModelSettings.ts
6a1b4cc to
2457b10
Compare
2457b10 to
cb923b5
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@wework/src/components/layout/useWorkbenchPaneSession.ts`:
- Around line 473-478: Update the Runtime goal hydration debug log in the
resolved hydration flow to use resolvedGoal?.status instead of
loadedGoal?.status, so the logged goal status matches the fallback-seeded value
stored and reported by the subsequent logic.
- Around line 472-480: Prevent the async hydration block around getRuntimeGoal
and setRuntimeConversationGoal from applying stale results after
clearCurrentGoal or updateCurrentGoalStatus mutates the goal. Track a per-goal
mutation generation (or equivalent request validity guard), verify it before
resolving and storing loadedGoal/seededGoal, and ignore responses from earlier
generations; add a regression test covering a clear or status update before the
deferred request resolves.
In `@wework/src/features/workbench/WorkbenchProvider.test.tsx`:
- Line 5010: Update the deferred response resolved by getRuntimeGoal in the
relevant test to include the required taskId field, using request.taskId from
runtimeWorkApi.createRuntimeTask.mock.calls[0][0] or the test’s expected task ID
value. Keep the accepted and goal fields unchanged.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: d8262cde-4b58-456c-95a4-ec1a5db45050
📒 Files selected for processing (2)
wework/src/components/layout/useWorkbenchPaneSession.tswework/src/features/workbench/WorkbenchProvider.test.tsx
| const resolvedGoal = loadedGoal ?? seededGoal?.goal ?? null | ||
| if (import.meta.env.VITE_WEWORK_RUNTIME_DEBUG === '1') { | ||
| console.info('[Wework] Runtime goal hydration resolved', { | ||
| address: runtimeAddressDebug(runtimeTaskLoadTarget.address), | ||
| accepted: response.accepted, | ||
| goalStatus: loadedGoal?.status ?? null, | ||
| }) | ||
| } | ||
| setRuntimeConversationGoal(runtimeTaskLoadTarget.address, loadedGoal) | ||
| setRuntimeConversationGoal(runtimeTaskLoadTarget.address, resolvedGoal) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C 6 \
'getRuntimeGoal|setRuntimeConversationGoal|clearRuntimeGoal|clearCurrentGoal|updateCurrentGoalStatus|clearRuntimePaneGoalSeed' \
wework/src/components/layout/useWorkbenchPaneSession.tsRepository: wecode-ai/Wegent
Length of output: 9206
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== outline around hook state/mutations =="
ast-grep outline wework/src/components/layout/useWorkbenchPaneSession.ts \
--match useWorkbenchPaneSession \
--view expanded || true
echo "== goal/state declarations and imports =="
sed -n '1,145p' wework/src/components/layout/useWorkbenchPaneSession.ts
sed -n '340,512p' wework/src/components/layout/useWorkbenchPaneSession.ts
sed -n '2035,2160p' wework/src/components/layout/useWorkbenchPaneSession.ts
sed -n '2330,2365p' wework/src/components/layout/useWorkbenchPaneSession.tsRepository: wecode-ai/Wegent
Length of output: 15799
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== runtimeConversationCache definitions for cache/lifecycle APIs =="
rg -n -C 8 'export .*setRuntimeConversationGoal|export .*updateRuntimeConversation|goalStatusReceived|clearRuntimeGoal|setRuntimeGoal|getRuntimeGoal|setRuntimeGoal|RuntimeConversationGoal|GoalStatus' wework/src/features/workbench wework/src \
-g '*runtimeConversationCache*' -g '*.ts' -g '*.tsx'
echo "== candidate cache file list =="
fd -i 'runtimeConversation|conversationCache|goal' wework/src | sed -n '1,80p'Repository: wecode-ai/Wegent
Length of output: 50373
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== runtimeConversationCache main exports =="
fd -i 'runtimeConversationCache' wework/src
CACHE="$(fd -i 'runtimeConversationCache' wework/src | head -n 1)"
wc -l "$CACHE"
sed -n '1,240p' "$CACHE"Repository: wecode-ai/Wegent
Length of output: 7405
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== local cache/store usages for currentRuntimeTask =="
rg -n -C 4 'setRuntimeConversationGoal|lifecycleStore\.goalStatusReceived|clearRuntimePaneGoalSeed|pendingRuntimeGoalState|getRuntimeConversationMetadata|setRuntimeConversationTaskPlan' wework/src/features/workbench/useWorkbenchRuntimeTasks.ts wework/src/features/workbench/runtimeTaskLifecycle wework/src/features/workbench/runtimeConversationCache* -g '*ts'Repository: wecode-ai/Wegent
Length of output: 8087
Prevent stale hydrated goals from overriding later mutations.
clearCurrentGoal and updateCurrentGoalStatus can write null or a newer status while getRuntimeGoal is pending. If that deferred response then returns goal: null, resolvedGoal falls back to the earlier seededGoal and overwrites the cache/lifecycle store with old state. Track a goal-mutation generation or ignore stale hydration results before applying them. Add a regression test for clear or status change before the deferred request resolves.
🤖 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 `@wework/src/components/layout/useWorkbenchPaneSession.ts` around lines 472 -
480, Prevent the async hydration block around getRuntimeGoal and
setRuntimeConversationGoal from applying stale results after clearCurrentGoal or
updateCurrentGoalStatus mutates the goal. Track a per-goal mutation generation
(or equivalent request validity guard), verify it before resolving and storing
loadedGoal/seededGoal, and ignore responses from earlier generations; add a
regression test covering a clear or status update before the deferred request
resolves.
| if (import.meta.env.VITE_WEWORK_RUNTIME_DEBUG === '1') { | ||
| console.info('[Wework] Runtime goal hydration resolved', { | ||
| address: runtimeAddressDebug(runtimeTaskLoadTarget.address), | ||
| accepted: response.accepted, | ||
| goalStatus: loadedGoal?.status ?? null, | ||
| }) |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Log the resolved goal status.
When the API returns no goal and the seed supplies the fallback, Line 477 logs null even though Lines 480-483 store and report the seeded status. Change loadedGoal?.status to resolvedGoal?.status.
Proposed fix
- goalStatus: loadedGoal?.status ?? null,
+ goalStatus: resolvedGoal?.status ?? null,📝 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.
| if (import.meta.env.VITE_WEWORK_RUNTIME_DEBUG === '1') { | |
| console.info('[Wework] Runtime goal hydration resolved', { | |
| address: runtimeAddressDebug(runtimeTaskLoadTarget.address), | |
| accepted: response.accepted, | |
| goalStatus: loadedGoal?.status ?? null, | |
| }) | |
| if (import.meta.env.VITE_WEWORK_RUNTIME_DEBUG === '1') { | |
| console.info('[Wework] Runtime goal hydration resolved', { | |
| address: runtimeAddressDebug(runtimeTaskLoadTarget.address), | |
| accepted: response.accepted, | |
| goalStatus: resolvedGoal?.status ?? null, | |
| }) |
🤖 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 `@wework/src/components/layout/useWorkbenchPaneSession.ts` around lines 473 -
478, Update the Runtime goal hydration debug log in the resolved hydration flow
to use resolvedGoal?.status instead of loadedGoal?.status, so the logged goal
status matches the fallback-seeded value stored and reported by the subsequent
logic.
| ) | ||
| await waitFor(() => expect(runtimeWorkApi.getRuntimeGoal).toHaveBeenCalled()) | ||
| await act(async () => { | ||
| getRuntimeGoal.resolve({ accepted: true, goal: null }) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '741,746p' wework/src/types/api.ts
sed -n '5008,5013p' wework/src/features/workbench/WorkbenchProvider.test.tsxRepository: wecode-ai/Wegent
Length of output: 500
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the deferred mock setup and nearby createRuntimeTask response used in this test.
sed -n '4960,5016p' wework/src/features/workbench/WorkbenchProvider.test.tsx
# Programmatically inspect the surrounding test to identify the variable name passed by createRuntimeTask
# and whether the deferred resolve omits taskId.
python3 - <<'PY'
from pathlib import Path
p = Path('wework/src/features/workbench/WorkbenchProvider.test.tsx')
text = p.read_text()
target_start = text.find('getRuntimeGoal.resolve({ accepted: true, goal: null })')
print('target_resolve_start=', target_start)
context = text[max(0,target_start-900):min(len(text),target_start+120)]
print('CONTEXT:')
print(context)
print('contains_createRuntimeTask_call=', 'createRuntimeTask' in context)
print('contains_taskId_resolve=', 'taskId' in context)
PYRepository: wecode-ai/Wegent
Length of output: 3487
Include the required taskId in the deferred response.
RuntimeGoalGetResponse requires taskId: string, but getRuntimeGoal.resolve({ accepted: true, goal: null }) omits it. Use request.taskId from runtimeWorkApi.createRuntimeTask.mock.calls[0][0], or add the expected task ID value if this test decouples the response.
🤖 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 `@wework/src/features/workbench/WorkbenchProvider.test.tsx` at line 5010,
Update the deferred response resolved by getRuntimeGoal in the relevant test to
include the required taskId field, using request.taskId from
runtimeWorkApi.createRuntimeTask.mock.calls[0][0] or the test’s expected task ID
value. Keep the accepted and goal fields unchanged.
Summary
Why
The previous empty selection could disable the feature silently and could not express the expected behavior of generating titles with the task's own model.
User impact
Users can enable friendly titles with a clear default model choice. Selecting a dedicated title model still affects only title generation, not the task model.
Validation
pnpm --filter wework exec vitest run src/components/settings/GeneralSettingsPage.test.tsx src/features/workbench/useWorkbenchRuntimeMessaging.test.tspnpm --filter wework exec prettier --check ...pnpm --filter wework exec eslint ...Summary by CodeRabbit
New Features
Bug Fixes
Documentation