Skip to content

fix: revert PR #290 MetaAgentSelector refactor + warn on meta config save failure (gemini A)#292

Merged
hang-in merged 2 commits into
mainfrom
fix/gemini-a-option-revert-and-catch
May 20, 2026
Merged

fix: revert PR #290 MetaAgentSelector refactor + warn on meta config save failure (gemini A)#292
hang-in merged 2 commits into
mainfrom
fix/gemini-a-option-revert-and-catch

Conversation

@hang-in
Copy link
Copy Markdown
Owner

@hang-in hang-in commented May 20, 2026

요약

Gemini PR review 의 A 옵션 (PR #290 revert + PR #289 silent catch 만 fix) 실행. 2 변경, ~10 LoC.

T1 — MetaAgentSelector.tsx PR #290 refactor revert

PR #290 (46b80af) 의 filter + find + [0] single-pass refactor 가 Gemini review 에서 원래 코드보다 비효율 지적을 받음 (engineModels 가 engine 당 2~5개의 짧은 list 라 filter 의 중간 array allocation 이 오히려 overhead, 가독성도 PR #287 쪽이 직관). PR #287 (ae4e652) 의 find + find 2-pass 패턴으로 revert.

동작 동등 — recommended → first → undefined 우선순위 보존.

T2 — AgentsSection.tsx:404 silent catch → console.warn

PR #289 Gemini review 의 low 지적: migrate 후 자동 save 실패를 silent 삼키면 사용자가 stale meta config 으로 계속 진행 가능. CLAUDE.md feedback_error_visibility 정책 (silent catch 금지, 오류는 toast/console 표면화) 과 정합.

비대상 (cut-off)

본 PR 은 A 옵션 결정에 따라 아래 항목 처리 안 함 — v0.1.8-beta-5 후속에서 별도 처리:

본 PR 의 새 Gemini review 가 또 달려도 무시 (cut-off).

Verification

항목 결과
cargo check PASS (Rust 변경 0, sanity OK)
tsc --noEmit PASS (0 errors)
vitest run 478 / 478 PASS (baseline 동일)
영역 외 변경 grep 0 lines (git diff main -- ':!MetaAgentSelector.tsx' ':!AgentsSection.tsx' = 0)

CI 정책

PR 직후 gh pr merge --squash --delete-branch --admin 즉시 머지. CI watch 불필요 (단일 frontend 좁은 영역).

🤖 Generated with Claude Code

dghong and others added 2 commits May 20, 2026 18:17
… was less efficient per gemini)

PR #290 의 filter+find+[0] 패턴은 Gemini review 에서 "원래 코드보다
비효율" 지적을 받음. PR #287 (ae4e652) 의 find+find 2 호출 패턴으로
revert — 동작 동등 (recommended → first → undefined 우선순위 보존),
2 회 store 순회로 끝남.

- modelsForEngine.filter + find + [0] (3-step) → find + find (2-step)
- engineModels 가 통상 짧은 list (engine 당 2~5개) 라 filter 의 중간
  array allocation 이 오히려 overhead. 가독성도 PR #287 쪽이 직관.

Gemini review 후속 항목 (useEffect deps / migrate defensive log /
dynamic import 중복 / Map pre-process) 은 본 PR 비대상 — v0.1.8-beta-5
후속에서 별도 처리.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ini PR #289 review)

Gemini review (PR #289) low 지적: migrate 후 자동 save 실패를 silent
삼키면 사용자가 stale 한 meta config 으로 계속 진행할 수 있음.
console.warn 으로 표면화하여 silent error 정책 (CLAUDE.md
feedback_error_visibility) 과 정합.

본 PR 의 다른 Gemini review 항목 (useEffect deps / migrate defensive
log / dynamic import 중복) 은 v0.1.8-beta-5 후속에서 별도 처리.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@hang-in hang-in merged commit af4ff40 into main May 20, 2026
2 checks passed
@hang-in hang-in deleted the fix/gemini-a-option-revert-and-catch branch May 20, 2026 09:23
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request refactors the model selection logic in MetaAgentSelector.tsx to reduce intermediate array allocations and updates the error handling in AgentsSection.tsx to log auto-save failures. The reviewer suggested implementing a single-pass loop for model selection to improve efficiency and recommended using console.error instead of console.warn to ensure better visibility of configuration save failures.

Comment on lines +124 to +125
const recommended = engineModels.find((m) => m.engine === d.engine && m.recommended)?.id
?? engineModels.find((m) => m.engine === d.engine)?.id;
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

현재의 2-pass find 방식은 권장(recommended) 모델이 없을 경우 engineModels 배열을 최대 두 번 순회하게 됩니다. 이 리버트의 목적이 불필요한 할당을 줄여 효율성을 높이는 것이므로, 단일 패스 루프를 사용하는 것이 더욱 최적입니다. 이는 $O(N)$ 복잡도로 중복 순회를 피하면서도 중간 배열 할당을 발생시키지 않습니다.

          let fallbackId: string | undefined;
          let recommendedId: string | undefined;
          for (const m of engineModels) {
            if (m.engine === d.engine) {
              if (m.recommended) {
                recommendedId = m.id;
                break;
              }
              if (!fallbackId) fallbackId = m.id;
            }
          }
          const recommended = recommendedId ?? fallbackId;

// 값이 실제로 변경됐다면 즉시 저장해서 stale 한 상태 잔존 방지.
if (migrated.engine !== c.engine) {
saveMetaConfig(migrated).catch(() => { /* ignore */ });
saveMetaConfig(migrated).catch((e) => console.warn("Failed to auto-save migrated meta config", e));
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

console.warn을 사용하는 것도 기존의 silent catch보다는 낫지만, 설정 저장 실패(마이그레이션된 설정의 자동 저장)는 시스템의 정합성에 영향을 줄 수 있는 오류 상태입니다. console.error를 사용하면 모니터링 도구나 개발자 콘솔에서 더 높은 가시성을 확보할 수 있으며, 실패 시 사용자의 설정이 stale한 상태로 남게 된다는 점을 고려할 때 더 적절한 심각도입니다.

Suggested change
saveMetaConfig(migrated).catch((e) => console.warn("Failed to auto-save migrated meta config", e));
saveMetaConfig(migrated).catch((e) => console.error("Failed to auto-save migrated meta config", e));

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant