feat(cli/tui): P4 9 golden 파일 (SPEC-GOOSE-CLI-TUI-003)#116
Conversation
…SE-CLI-TUI-003) - snapshot_sessionmenu_test.go: TestSnapshot_SessionMenuOpen → session_menu_open.golden - snapshot_i18n_test.go: 8개 i18n 테스트 (ko×4 + en×4) - testdata/snapshots/: 9 신규 golden 파일 생성 - AC-CLITUI3-004/009/010 RED→GREEN - 전체 10 AC GREEN (SPEC 완료) ko golden 검증: 세션:, 대화 명령어, 이 도구 호출을 허용, 최근 세션 en golden 검증: Session:, Conversation commands, Allow this tool call, Recent sessions SPEC: SPEC-GOOSE-CLI-TUI-003 REQ: REQ-CLITUI3-001, -002, -003 AC: AC-CLITUI3-004, AC-CLITUI3-009, AC-CLITUI3-010 Closes #112 🗿 MoAI <email@mo.ai.kr>
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
WalkthroughPR는 TUI 렌더링의 i18n 스냅샷 테스트 스위트를 도입합니다. 한국어와 영어 두 언어로 상태바 유휴, Changes스냅샷 테스트 및 i18n 검증
예상 코드 리뷰 노력🎯 2 (Simple) | ⏱️ ~12 minutes 관련 가능성 있는 PR
검토 시 주의사항
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
- loader.go: LoadFrom(yamlPath) 함수 추가 — CWD 변경 없이 특정 YAML 파일에서 직접 로드 - loader_test.go: os.Chdir 대신 LoadFrom 사용 → t.Parallel() 안전 + CI 격리 보장 이전: os.Chdir(tmpDir) → git rev-parse race 취약 이후: LoadFrom(yamlPath) → CWD 무관, 병렬 안전 SPEC: SPEC-GOOSE-CLI-TUI-003 🗿 MoAI <email@mo.ai.kr>
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (4)
internal/cli/tui/testdata/snapshots/session_menu_open_en.golden (1)
1-1: 💤 Low value
session_menu_open.golden과 세션 이름 불일치 — 의도적인지 확인 필요
session_menu_open.golden(Line 1)은Session: test-session을 사용하는 반면, 이 파일과 KO 대응 파일(session_menu_open_ko.golden)은 모두Session: test를 사용합니다. 두 테스트가 서로 다른 픽스처를 쓰는 것이 의도된 설계라면 괜찮지만,TestSnapshot_SessionMenuOpen과TestSnapshot_I18N_SessionMenuOpen_En/Ko가 동일 기능을 검증하는 경우 픽스처를 통일하는 것이 유지보수에 유리합니다.🤖 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 `@internal/cli/tui/testdata/snapshots/session_menu_open_en.golden` at line 1, The English snapshot file session_menu_open_en.golden uses "Session: test" which mismatches session_menu_open.golden (which uses "Session: test-session"); update the snapshot content so all three snapshots (session_menu_open.golden, session_menu_open_en.golden, session_menu_open_ko.golden) use the same session name used by the tests (align to either "test" or "test-session") to make TestSnapshot_SessionMenuOpen and TestSnapshot_I18N_SessionMenuOpen_En/Ko use a single consistent fixture.internal/cli/tui/snapshot_i18n_test.go (2)
57-58: ⚡ Quick win
updated.(*Model)직접 단정은 테스트 실패 신호를 약하게 만듭니다.type assertion 실패 시 panic 대신
ok체크 +t.Fatalf로 바꾸면 실패 원인이 더 명확해집니다.제안 diff
- updated, _ := m.Update(tea.KeyMsg{Type: tea.KeyEnter}) - m = updated.(*Model) + updated, _ := m.Update(tea.KeyMsg{Type: tea.KeyEnter}) + next, ok := updated.(*Model) + if !ok { + t.Fatalf("expected *Model, got %T", updated) + } + m = nextAlso applies to: 71-72
🤖 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 `@internal/cli/tui/snapshot_i18n_test.go` around lines 57 - 58, Replace the direct type assertion updated.(*Model) with a comma-ok check so tests fail with a clear message instead of panicking: after calling m.Update(tea.KeyMsg{Type: tea.KeyEnter}) and assigning updated, use v, ok := updated.(*Model) and if !ok { t.Fatalf("expected *Model from Update, got %T", updated) } then set m = v; apply the same change for the second occurrence around the lines handling the next KeyEnter (the other updated variable at the 71-72 area).
33-131: ⚡ Quick win각 테스트에
t.Parallel()적용 가능성을 확인해 주세요.테스트 간 상태 충돌이 없다면 병렬 실행으로 race 검출 효율을 높일 수 있습니다. (
snapshots.SetupAsciiTermenv()가 전역 상태를 건드린다면 제외 근거를 코멘트로 남기면 좋습니다.)As per coding guidelines
**/*_test.go: - t.Parallel() 호출 가능한 곳은 호출 (race 검출 강화).🤖 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 `@internal/cli/tui/snapshot_i18n_test.go` around lines 33 - 131, Add t.Parallel() at the start of each test (TestSnapshot_I18N_StatusbarIdle_Ko, TestSnapshot_I18N_StatusbarIdle_En, TestSnapshot_I18N_SlashHelp_Ko, TestSnapshot_I18N_SlashHelp_En, TestSnapshot_I18N_PermissionModal_Ko, TestSnapshot_I18N_PermissionModal_En, TestSnapshot_I18N_SessionMenuOpen_Ko, TestSnapshot_I18N_SessionMenuOpen_En) so they run in parallel; ensure you call t.Parallel() immediately after snapshots.SetupAsciiTermenv() (or at the top of the test) and verify that snapshots.SetupAsciiTermenv() and any shared state (e.g., package-level globals used by newI18NModel, sessionmenu.Open, permission.PermissionModel) are safe for concurrent use—if not, leave that test without t.Parallel() and add a brief comment explaining the global-state conflict.internal/cli/tui/snapshot_sessionmenu_test.go (1)
16-37: ⚡ Quick win
t.Parallel()적용 가능 여부를 검토해 주세요.현재 테스트는 독립 입력으로 보이므로, 병렬 실행이 안전하다면
t.Parallel()을 넣어 race 탐지 강도를 높이는 편이 좋습니다.As per coding guidelines
**/*_test.go: - t.Parallel() 호출 가능한 곳은 호출 (race 검출 강화).🤖 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 `@internal/cli/tui/snapshot_sessionmenu_test.go` around lines 16 - 37, Add t.Parallel() to TestSnapshot_SessionMenuOpen to allow the test to run in parallel: inside the TestSnapshot_SessionMenuOpen function (the function named TestSnapshot_SessionMenuOpen that calls snapshots.SetupAsciiTermenv(), NewModel, sets m.clock/width/height/viewport and m.sessionMenuState), add a t.Parallel() statement near the start of the test (before or immediately after snapshots.SetupAsciiTermenv()) so the test runs concurrently with others per the repository testing 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.
Inline comments:
In `@internal/cli/tui/testdata/snapshots/permission_modal_ko.golden`:
- Line 28: The Korean snapshot string "이 도구 호출을 허용..." uses an ellipsis while
the English snapshot "Allow this tool call?" uses a question mark; update the KO
snapshot to use a question mark for consistency (change "이 도구 호출을 허용..." to "이
도구 호출을 허용?") in the permission modal golden so both locales use the same
interrogative punctuation.
- Line 25: The KO golden shows untranslated strings "Permission Request: Bash"
and navigation hints "[Up/Down] navigate [Enter] select" / "[Esc] deny once" —
add corresponding Korean entries to the i18n catalog used by the TUI permission
modal (search for the exact strings to find the keys), e.g. add translations for
the permission modal header key (the key that formats "Permission Request: %s")
and the nav-hint keys in your YAML/JSON/TOML locales, ensure the localization
loader picks up the new entries, run the provided ripgrep commands to verify the
keys exist, and update the KO golden/test snapshot if needed (or update SPEC/AC
if the untranslated text was intentional).
- Line 25: The header content line "| Permission Request: Bash |" is one
character shorter than the border "+----------------------------------+"
(internal width 34), so update the header line by adding one trailing space to
the right side so it becomes "| Permission Request: Bash |" (total 36
chars including the pipes); make the same adjustment in the corresponding file
permission_modal_en.golden to keep both snapshots' header widths consistent.
---
Nitpick comments:
In `@internal/cli/tui/snapshot_i18n_test.go`:
- Around line 57-58: Replace the direct type assertion updated.(*Model) with a
comma-ok check so tests fail with a clear message instead of panicking: after
calling m.Update(tea.KeyMsg{Type: tea.KeyEnter}) and assigning updated, use v,
ok := updated.(*Model) and if !ok { t.Fatalf("expected *Model from Update, got
%T", updated) } then set m = v; apply the same change for the second occurrence
around the lines handling the next KeyEnter (the other updated variable at the
71-72 area).
- Around line 33-131: Add t.Parallel() at the start of each test
(TestSnapshot_I18N_StatusbarIdle_Ko, TestSnapshot_I18N_StatusbarIdle_En,
TestSnapshot_I18N_SlashHelp_Ko, TestSnapshot_I18N_SlashHelp_En,
TestSnapshot_I18N_PermissionModal_Ko, TestSnapshot_I18N_PermissionModal_En,
TestSnapshot_I18N_SessionMenuOpen_Ko, TestSnapshot_I18N_SessionMenuOpen_En) so
they run in parallel; ensure you call t.Parallel() immediately after
snapshots.SetupAsciiTermenv() (or at the top of the test) and verify that
snapshots.SetupAsciiTermenv() and any shared state (e.g., package-level globals
used by newI18NModel, sessionmenu.Open, permission.PermissionModel) are safe for
concurrent use—if not, leave that test without t.Parallel() and add a brief
comment explaining the global-state conflict.
In `@internal/cli/tui/snapshot_sessionmenu_test.go`:
- Around line 16-37: Add t.Parallel() to TestSnapshot_SessionMenuOpen to allow
the test to run in parallel: inside the TestSnapshot_SessionMenuOpen function
(the function named TestSnapshot_SessionMenuOpen that calls
snapshots.SetupAsciiTermenv(), NewModel, sets m.clock/width/height/viewport and
m.sessionMenuState), add a t.Parallel() statement near the start of the test
(before or immediately after snapshots.SetupAsciiTermenv()) so the test runs
concurrently with others per the repository testing guidelines.
In `@internal/cli/tui/testdata/snapshots/session_menu_open_en.golden`:
- Line 1: The English snapshot file session_menu_open_en.golden uses "Session:
test" which mismatches session_menu_open.golden (which uses "Session:
test-session"); update the snapshot content so all three snapshots
(session_menu_open.golden, session_menu_open_en.golden,
session_menu_open_ko.golden) use the same session name used by the tests (align
to either "test" or "test-session") to make TestSnapshot_SessionMenuOpen and
TestSnapshot_I18N_SessionMenuOpen_En/Ko use a single consistent fixture.
🪄 Autofix (Beta)
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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: a202de8d-c3a4-4ccc-bd85-81c88922fde9
📒 Files selected for processing (11)
internal/cli/tui/snapshot_i18n_test.gointernal/cli/tui/snapshot_sessionmenu_test.gointernal/cli/tui/testdata/snapshots/permission_modal_en.goldeninternal/cli/tui/testdata/snapshots/permission_modal_ko.goldeninternal/cli/tui/testdata/snapshots/session_menu_open.goldeninternal/cli/tui/testdata/snapshots/session_menu_open_en.goldeninternal/cli/tui/testdata/snapshots/session_menu_open_ko.goldeninternal/cli/tui/testdata/snapshots/slash_help_en.goldeninternal/cli/tui/testdata/snapshots/slash_help_ko.goldeninternal/cli/tui/testdata/snapshots/statusbar_idle_en.goldeninternal/cli/tui/testdata/snapshots/statusbar_idle_ko.golden
|
|
||
| > > Type a message... | ||
| +----------------------------------+ | ||
| | Permission Request: Bash | |
There was a problem hiding this comment.
KO 권한 모달에 미번역 영어 문자열 존재 (AC-009 i18n 커버리지 불완전)
Line 25의 Permission Request: Bash 헤더와 Lines 34~35의 [Up/Down] navigate [Enter] select / [Esc] deny once 내비게이션 힌트가 KO 골든에서 영어 그대로 노출됩니다. SPEC-GOOSE-CLI-TUI-003 AC-009가 이 표면의 완전한 한국어 번역을 요구한다면, i18n 카탈로그에 해당 키가 누락된 것입니다.
의도적으로 일부만 번역한 것이라면 SPEC/AC를 업데이트해서 범위를 명시해 주세요.
#!/bin/bash
# i18n 카탈로그에서 "Permission Request" 및 nav hint 키 존재 여부 확인
rg -rn "Permission Request\|navigate\|deny once" --type go
rg -rn "Permission Request\|navigate\|deny once" --glob "*.yaml" --glob "*.json" --glob "*.toml"Also applies to: 34-35
🤖 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 `@internal/cli/tui/testdata/snapshots/permission_modal_ko.golden` at line 25,
The KO golden shows untranslated strings "Permission Request: Bash" and
navigation hints "[Up/Down] navigate [Enter] select" / "[Esc] deny once" — add
corresponding Korean entries to the i18n catalog used by the TUI permission
modal (search for the exact strings to find the keys), e.g. add translations for
the permission modal header key (the key that formats "Permission Request: %s")
and the nav-hint keys in your YAML/JSON/TOML locales, ensure the localization
loader picks up the new entries, run the provided ripgrep commands to verify the
keys exist, and update the KO golden/test snapshot if needed (or update SPEC/AC
if the untranslated text was intentional).
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# permission_modal_en/ko 골든의 Line 25 실제 바이트 길이 확인
echo "=== permission_modal_en.golden line 25 ==="
fd "permission_modal_en.golden" --type f --exec sed -n '25p' {} \; | cat -A | head -1
echo "=== permission_modal_ko.golden line 25 ==="
fd "permission_modal_ko.golden" --type f --exec sed -n '25p' {} \; | cat -A | head -1
echo "=== border line (line 24) for comparison ==="
fd "permission_modal_en.golden" --type f --exec sed -n '24p' {} \; | cat -A | head -1Repository: modu-ai/goose
Length of output: 300
Permission Request: Bash 헤더 라인 너비 불일치 확인됨 — 수정 권장
경계선 +----------------------------------+은 내부 너비 34 컬럼(총 36자)을 정의하지만, | Permission Request: Bash | 콘텐츠 라인은 35자입니다. 양쪽 끝 |를 포함한 실제 측정 결과이므로, 오른쪽에 스페이스 1개를 추가하여 36자로 맞추는 것이 좋습니다. 이 문제는 permission_modal_en.golden Line 25에도 동일하게 존재합니다.
🤖 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 `@internal/cli/tui/testdata/snapshots/permission_modal_ko.golden` at line 25,
The header content line "| Permission Request: Bash |" is one character
shorter than the border "+----------------------------------+" (internal width
34), so update the header line by adding one trailing space to the right side so
it becomes "| Permission Request: Bash |" (total 36 chars including the
pipes); make the same adjustment in the corresponding file
permission_modal_en.golden to keep both snapshots' header widths consistent.
| | Permission Request: Bash | | ||
| +----------------------------------+ | ||
| | | | ||
| | 이 도구 호출을 허용... | |
There was a problem hiding this comment.
KO/EN 권한 모달 질문 문장 부호 불일치
Line 28: KO 골든은 이 도구 호출을 허용...(줄임표)을, EN 골든(permission_modal_en.golden Line 28)은 Allow this tool call?(물음표)를 사용합니다. 의도적인 언어별 표현 차이라면 무방하지만, 카탈로그 문자열이 실수로 다를 경우 UX 일관성이 깨질 수 있습니다.
🤖 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 `@internal/cli/tui/testdata/snapshots/permission_modal_ko.golden` at line 28,
The Korean snapshot string "이 도구 호출을 허용..." uses an ellipsis while the English
snapshot "Allow this tool call?" uses a question mark; update the KO snapshot to
use a question mark for consistency (change "이 도구 호출을 허용..." to "이 도구 호출을 허용?")
in the permission modal golden so both locales use the same interrogative
punctuation.
…n 구현 기록 (#117) - progress.md: run phase 완료 기록 (PR #113~#116, 10 AC GREEN) - Phase 표 🟡 PENDING → 🟢 DONE 갱신 - Iteration Log 4행 추가 (P1~P4 각 phase 결과) - Section 4 (후속) PR 목록 + CHANGELOG 항목 기록 - spec-compact.md: status draft → implemented - CHANGELOG.md: Unreleased 섹션에 CLI-TUI-003 항목 추가 (Ctrl-R sessionmenu, Ctrl-Up edit/regenerate, i18n catalog, 6-tier KeyEscape, 9 golden) - plan-audit: SPEC-GOOSE-CLI-TUI-002-review-1.md, OBS-METRICS-001-review-2/3.md 추가 SPEC: SPEC-GOOSE-CLI-TUI-003 Phase: SYNC 🗿 MoAI <email@mo.ai.kr>
요약
SPEC-GOOSE-CLI-TUI-003 Phase 4 — 9개 신규 golden 파일 생성으로 SPEC 완료.
추가된 Golden 파일 (9개)
i18n 검증
세션:,대화 명령어,이 도구 호출을 허용,최근 세션Session:,Conversation commands,Allow this tool call,Recent sessionsTest Plan
go test -race ./internal/cli/tui/...PASS (전체 패키지)SPEC: SPEC-GOOSE-CLI-TUI-003
Closes #112
🗿 MoAI email@mo.ai.kr
Summary by CodeRabbit