Skip to content

refactor: gemini review final batch — MetaAgentSelector single-pass + rawq Duration constants#291

Merged
hang-in merged 3 commits into
mainfrom
chore/gemini-review-final-batch
May 20, 2026
Merged

refactor: gemini review final batch — MetaAgentSelector single-pass + rawq Duration constants#291
hang-in merged 3 commits into
mainfrom
chore/gemini-review-final-batch

Conversation

@hang-in
Copy link
Copy Markdown
Owner

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

Summary

Gemini medium 지적 4건 묶음 처리 — PR #287 + PR #288 의 미처리 review 항목. 모두 refactor only, 동작 변경 0.

Task 영역 파일 내용
T1 Frontend src/components/tunaflow/MetaAgentSelector.tsx engineModels 두 번 순회 → filter 후 한 번 순회로 통일. recommended → first → undefined 우선순위 동등
T2 Frontend 동 + src/tests/metaAgentSelector-modelDiscovery.test.tsx AgentDetection export 추가, test 의 중복 Detection 타입 제거 → import 로 교체
T3 Rust src-tauri/src/agents/rawq.rs MAX_WAIT_MS: u64 / TICK_INTERVAL_MS: u64MAX_WAIT: Duration / TICK_INTERVAL: Duration. Duration::from_millis() 호출 중복 제거
T4 Rust sleep(Duration::from_millis(TICK_INTERVAL_MS))sleep(TICK_INTERVAL) 직접

T3 의 상수 타입 변경이 T4 의 sleep 인자 형태를 강제하므로 동일 hunk — c6ce007 한 commit 으로 묶되 message 에서 양쪽 review 항목 1:1 인용.

Review URLs

Verification

cd src-tauri && cargo check               # PASS
cd src-tauri && cargo test --lib          # 652 passed (baseline 652)
npx tsc --noEmit                          # PASS (no errors)
npx vitest run                            # 453 passed (baseline 453)

회귀 가드 grep

git diff main...HEAD -- src-tauri/ ':!src-tauri/src/agents/rawq.rs'                              # 변경 0
git diff main...HEAD -- src/ ':!.../MetaAgentSelector.tsx' ':!.../metaAgentSelector-modelDiscovery.test.tsx'  # 변경 0

영역 외 변경 0 확인. AgentDetection import 처는 MetaAgentSelector.tsx + test 두 곳뿐 (Rust 의 동명 struct 와 충돌 없음 — 별 언어).

[CI 정책]

PR 직후 gh pr merge --squash --delete-branch --admin 즉시 머지. CI watch 불필요 (refactor only, 동작 변경 0).

🤖 Generated with Claude Code

dghong and others added 3 commits May 20, 2026 17:43
…iew, T1)

Gemini medium 지적: 기존 코드는 engineModels 를 두 번 순회 (recommended 한 번
+ first fallback 한 번). filter 후 한 번 순회로 통일 — 동작 동등 (recommended →
first → undefined 우선순위 보존), 가독성 + 한 번 순회만 수행.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
#287 review, T2)

Gemini medium 지적: test 파일에 Detection 타입이 중복 정의돼 있었음.
MetaAgentSelector 의 AgentDetection 을 export 하고 test 에서 import 로 교체.
타입 schema 는 동일 — 단순 중복 제거.

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

Gemini medium 2건 (PR #288 review):
- T3: u64 ms → std::time::Duration 직접 (MAX_WAIT / TICK_INTERVAL).
  Duration::from_millis 호출 중복 제거.
- T4: std::thread::sleep(TICK_INTERVAL) 로 직접 사용.

T3 의 상수 타입 변경이 T4 의 sleep 인자 형태를 강제하기 때문에 동일 hunk —
1 commit 으로 묶되 message 에 양쪽 review 항목 1:1 인용. 동작 변경 0.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@hang-in hang-in merged commit d250a61 into main May 20, 2026
6 checks passed
@hang-in hang-in deleted the chore/gemini-review-final-batch branch May 20, 2026 08:51
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 time-related constants in the Rust backend to use std::time::Duration for improved type safety and updates the MetaAgentSelector component to export the AgentDetection interface for reuse in tests. Additionally, the model selection logic for CLI engines was refactored. Feedback was provided to optimize this selection logic by pre-processing engine models into a map to reduce time complexity from O(D * M) to O(M + D).

Comment on lines +124 to +128
const modelsForEngine = engineModels.filter((m) => m.engine === d.engine);
const recommendedId = modelsForEngine.find((m) => m.recommended)?.id;
const defaultId = modelsForEngine[0]?.id;
const modelToSelect = recommendedId ?? defaultId;
if (modelToSelect) next[d.engine] = modelToSelect;
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

This implementation iterates over engineModels for each detection inside the loop. While this is an improvement over the previous version, it has a time complexity of roughly O(D * M), where D is the number of detections and M is the number of engine models.

For better performance and scalability, especially if these lists could grow, consider processing engineModels once into a map and memoizing it (e.g., with useMemo). This would change the complexity to O(M + D).

Here's an example of how you could pre-process engineModels:

// Pre-process models once for efficient lookup
const modelsByEngine = useMemo(() => {
  const map = new Map<string, { recommendedId?: string; firstId?: string }>();
  for (const model of engineModels) {
    if (!map.has(model.engine)) {
      map.set(model.engine, {});
    }
    const entry = map.get(model.engine)!;
    if (!entry.firstId) {
      entry.firstId = model.id;
    }
    if (model.recommended && !entry.recommendedId) {
      entry.recommendedId = model.id;
    }
  }
  return map;
}, [engineModels]);

// Then, inside the `useEffect`'s loop over detections:
const modelsInfo = modelsByEngine.get(d.engine);
const modelToSelect = modelsInfo?.recommendedId ?? modelsInfo?.firstId;
if (modelToSelect) {
  next[d.engine] = modelToSelect;
}

This approach processes engineModels only once, making the default model selection more efficient as it avoids repeated iterations.

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