Fix/auto fetch upstream model tips - #3096
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review infoConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughRemoved runtime guards that prevented upstream model update detection; UI now invokes detection unconditionally and success messages include total ignored-model counts. Minor struct tag formatting fixes applied to ChannelOtherSettings. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Frontend
participant API
participant Controller
participant Persistence
User->>Frontend: Click "Detect upstream model updates"
Frontend->>API: POST /channels/:id/detect-upstream
API->>Controller: DetectChannelUpstreamModelUpdates(channelID)
Controller->>Controller: checkAndPersistChannelUpstreamModelUpdates(force=true, allowAutoApply=false)
Controller->>Persistence: Read/Write channel update state
Persistence-->>Controller: Persisted result
Controller-->>API: Result (added/removed/ignored counts)
API-->>Frontend: Success payload
Frontend-->>User: Show toast with added/removed/ignored/totalIgnored
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@web/src/hooks/channels/useChannelUpstreamUpdates.jsx`:
- Around line 24-39: The function getManualIgnoredModelCountFromSettings can
pass a string into normalizeModelList causing "map is not a function"; update it
to coerce parsed.upstream_model_update_ignored_models into an array before
calling normalizeModelList: extract const raw =
parsed?.upstream_model_update_ignored_models, if raw is a string try
JSON.parse(raw) in a try/catch, if parsedRaw is not an array but is truthy wrap
it as [parsedRaw], and if falsy use [] — then call normalizeModelList on that
array; reference getManualIgnoredModelCountFromSettings and normalizeModelList
when making the change.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
controller/channel_upstream_update.godto/channel_settings.goweb/src/components/table/channels/ChannelsColumnDefs.jsxweb/src/components/table/channels/modals/EditChannelModal.jsxweb/src/hooks/channels/useChannelUpstreamUpdates.jsx
💤 Files with no reviewable changes (2)
- web/src/components/table/channels/ChannelsColumnDefs.jsx
- controller/channel_upstream_update.go
| const getManualIgnoredModelCountFromSettings = (settings) => { | ||
| let parsed = null; | ||
| if (settings && typeof settings === 'object') { | ||
| parsed = settings; | ||
| } else if (typeof settings === 'string') { | ||
| try { | ||
| parsed = JSON.parse(settings); | ||
| } catch (error) { | ||
| parsed = null; | ||
| } | ||
| } | ||
| if (!parsed || typeof parsed !== 'object') { | ||
| return 0; | ||
| } | ||
| return normalizeModelList(parsed.upstream_model_update_ignored_models).length; | ||
| }; |
There was a problem hiding this comment.
Guard string-form ignored models before normalization.
At Line 38, passing a string directly to normalizeModelList can throw (map is not a function). Coerce to an array first for legacy/malformed payload safety.
Proposed fix
const getManualIgnoredModelCountFromSettings = (settings) => {
let parsed = null;
if (settings && typeof settings === 'object') {
parsed = settings;
} else if (typeof settings === 'string') {
@@
if (!parsed || typeof parsed !== 'object') {
return 0;
}
- return normalizeModelList(parsed.upstream_model_update_ignored_models).length;
+ const rawIgnored = parsed.upstream_model_update_ignored_models;
+ const ignoredModels = Array.isArray(rawIgnored)
+ ? rawIgnored
+ : typeof rawIgnored === 'string'
+ ? rawIgnored.split(',')
+ : [];
+ return normalizeModelList(ignoredModels).length;
};📝 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.
| const getManualIgnoredModelCountFromSettings = (settings) => { | |
| let parsed = null; | |
| if (settings && typeof settings === 'object') { | |
| parsed = settings; | |
| } else if (typeof settings === 'string') { | |
| try { | |
| parsed = JSON.parse(settings); | |
| } catch (error) { | |
| parsed = null; | |
| } | |
| } | |
| if (!parsed || typeof parsed !== 'object') { | |
| return 0; | |
| } | |
| return normalizeModelList(parsed.upstream_model_update_ignored_models).length; | |
| }; | |
| const getManualIgnoredModelCountFromSettings = (settings) => { | |
| let parsed = null; | |
| if (settings && typeof settings === 'object') { | |
| parsed = settings; | |
| } else if (typeof settings === 'string') { | |
| try { | |
| parsed = JSON.parse(settings); | |
| } catch (error) { | |
| parsed = null; | |
| } | |
| } | |
| if (!parsed || typeof parsed !== 'object') { | |
| return 0; | |
| } | |
| const rawIgnored = parsed.upstream_model_update_ignored_models; | |
| const ignoredModels = Array.isArray(rawIgnored) | |
| ? rawIgnored | |
| : typeof rawIgnored === 'string' | |
| ? rawIgnored.split(',') | |
| : []; | |
| return normalizeModelList(ignoredModels).length; | |
| }; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/src/hooks/channels/useChannelUpstreamUpdates.jsx` around lines 24 - 39,
The function getManualIgnoredModelCountFromSettings can pass a string into
normalizeModelList causing "map is not a function"; update it to coerce
parsed.upstream_model_update_ignored_models into an array before calling
normalizeModelList: extract const raw =
parsed?.upstream_model_update_ignored_models, if raw is a string try
JSON.parse(raw) in a try/catch, if parsedRaw is not an array but is truthy wrap
it as [parsedRaw], and if falsy use [] — then call normalizeModelList on that
array; reference getManualIgnoredModelCountFromSettings and normalizeModelList
when making the change.
…ream-model-tips Fix/auto fetch upstream model tips
Summary by CodeRabbit
Bug Fixes
Improvements