Repository navigation
refactor(mindrecord,home): presentation 정리 3종 — 로딩 통합·참조 0건 카드 삭제·no-op 디폴트 50건 (#1540) - #1563
Conversation
`private fun LoadingBox` 4벌과 `DraftListScreen` 의 인라인 `Box + CircularProgressIndicator` 1곳을 승격본으로 치환한다 (#1303 · #1490 — 같은 작업이라 한 번에 닫는다). **렌더가 같다는 근거.** 넷은 전부 `fillMaxSize` + 중앙 정렬 + 기본 지름이라 구조가 `LoadingBody` 와 동일하다. 유일한 차이인 `size(40.dp)` 는 material3 기본값과 같은 값이다 — `CircularProgressIndicator` 가 호출자 modifier 뒤에 `.size(CircularIndicatorDiameter)` 를 붙이고, `CircularIndicatorDiameter = CircularProgressIndicatorTokens.Size = 40.0.dp` 다 (material3 1.5.0-alpha17 소스 확인). 실측으로도 확인했다 — `validateDebugScreenshotTest` 의 실패 골든 집합이 develop 과 **동일** 하다(로컬 macOS 렌더가 CI 골든과 어긋나 develop 자체가 6건 실패한다. 그래서 절대 건수가 아니라 develop 대비 **차이**로 본다). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
#1422 이 넘긴 3건을 정본 시안과 대조해 **전부 삭제**로 판정했다. 셋 다 정의 파일 밖 참조가 0건이고 파일 안에는 자기 자신을 부르는 `@Preview` 만 있었다. 「미구현 화면의 선행 정의」가 아니라 **이미 다른 컴포넌트가 그 자리를 메운 뒤 남은 사본**이었다. | 심볼 | 시안의 그 자리 | 지금 그 자리를 그리는 것 | |---|---|---| | `InsightCard` | 주간리포트 `INSIGHTS` (`4327:77929`) | `EmotionKeywordCard` 의 `descriptionText` — 시안에서도 감정키워드 카드(`4327:99437`)가 같은 두 문단을 흡수했다 | | `WeeklyEmotionCard` | 일기_카드 형 «주간 평균 기분» (`4327:77573`) | `DiaryReportCard` — `이번 달`/`주간 평균 기분` 2열 통계를 실데이터(`monthDiaryCount`·`weeklyMoodEmoji`)로 그린다 | | `WeeklyReportCard` | **없다** | — 문구(`주간 리포트` · `이번 주 나의 기록들을 확인해 보세요.`)가 시안 전체에 0건이다 | 세 사본은 모두 파라미터가 `modifier` 뿐이라 실데이터를 못 받는다. 살려 둬도 배선할 수 없고, 같은 자리에 실구현이 이미 있어 다음 사람이 어느 쪽을 쓸지 헷갈리는 값만 남는다. 전용 문자열 6건도 함께 걷었다(다른 참조 0건 확인). `mindrecord_weekly_report_avg_mood_weekly` (「주간 평균 기분」 — 시안과 일치)는 `DiaryReportCard` 가 쓰므로 남긴다. 이 세 Preview 의 screenshot 골든은 없다. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`onXxx: (...) -> Unit = {}` 는 배선을 빠뜨려도 컴파일이 통과해 조용히 no-op 이 된다 (#1540).
mindrecord 47 + home 3 = **50건을 전부 걷었다.** main 소스 잔여 0건이다.
## 디폴트가 실제로 결함을 가리고 있었다
이슈가 「디폴트가 가려 온 결함이 있는지 확인」을 완료 조건에 뒀다. 걷어내자 컴파일러가
**실호출부 2곳**을 드러냈다. 둘 다 버튼이 보이는데 눌러도 아무 일이 없던 자리다.
**(1) `WriteTextField` → `TextStyleToolbar`** — `onLinkClick`·`onTypeClick` 미전달. 링크
아이콘과 «T» 버튼이 그려지는데 핸들러가 없었다. 같은 파일 `BottomToolbar` 가 같은 두 버튼을
갖고 있어 그 배선을 그대로 가져왔다(스타일 툴바가 하단 툴바를 덮으므로 같은 affordance 를
유지해야 한다).
**(2) `WeeklyReportScreen` HISTORY → `DailyQuestionListCard`** — `onEdit`·`onDelete` 미전달.
«더보기» 메뉴가 뜨고 수정·삭제가 전부 no-op 이었다. `DailyQuestionListCard` 를 nullable
핸들러로 재모델링해 **핸들러가 없으면 메뉴 자체를 그리지 않게** 했고, 읽기 전용 요약인 주간
리포트는 `null` 을 명시한다. #1540 이 든 처방 그대로다.
가드: `RecordActionAffordanceTest` 2건. 역검증 — 항상 그리도록 되돌리면 FAILED.
## 나머지
- 프리뷰·screenshotTest·unit test 호출부 30곳은 `{}` 를 명시로 바꿨다(디폴트와 동작 동일).
- `{ _, _ -> }` 꼴 4건은 첫 정규식이 놓쳤고 `NoOpCallbackDefaultKonsistTest` 가 잡아 함께 걷었다.
- 그 konsist 허용목록에서 mindrecord·home 항목 16개를 비웠다 — 남겨 두면 **다음 위반이 그
자리에 숨는다.** 이제 이 두 모듈은 신규 유입이 곧바로 red 다. timeletter(#1541) 만 남는다.
- 스크린샷 골든: 실패 집합이 develop 과 동일(새로 틀어진 것 0건).
판단이 들어간 (2) 의 근거는 `docs/qa/assumptions.md` 에 적었다.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
OnlyReadOneBook
left a comment
There was a problem hiding this comment.
내용은 좋습니다 — 게이트가 제목에서 죽어 검증이 하나도 돌지 않았습니다. 그것만 고치면 됩니다.
세 갈래를 한 PR 로 묶은 판단에 동의합니다. #1303 과 #1490 이 같은 5개 파일·같은 치환이고 스크린샷 골든 재생성이 세 번 필요해지는 것은 실질적인 비용입니다. 커밋으로 갈라 두신 덕에 갈래별로 읽혔습니다.
[blocker] 제목이 대표 이슈 형식을 벗어났습니다
refactor(mindrecord,home): … no-op 디폴트 50건 (#1540 외 3건)
^^^^^^
validate-pr-issue-link.mjs:91 은 제목이 (#N) 하나로 끝날 것을 요구합니다. 외 3건 이 붙어 파서가 대표 이슈를 못 뽑았고, 그 자리에서 Repository Quality 가 7초 만에 죽었습니다.
| 스텝 | 결과 |
|---|---|
| Require linked Issue | failure |
| Validate CI Test Plan | skipped |
| Install actionlint | skipped |
| Run repository quality checks | skipped |
| Verify failing fixtures | skipped |
(#1540) 로만 끝내 주세요. 본문의 Closes 4개는 그대로 두셔도 됩니다 — 게이트가 보는 것은 「제목의 대표 이슈가 본문에서 Closes/Refs 로 연결되었는가」뿐이고(:98-106), 다중 연결을 막는 규칙은 없습니다. 담당자 검사(:150)도 #1540 이 mindrecord 라 통과합니다.
제목을 고치면 edited 트리거로 재검증이 걸립니다. 다만 지금 Unit Test·Screenshot·Android Lint·Analyze (java-kotlin) 이 pending 이라, 그 결과는 그때 함께 보겠습니다.
확인한 것
참조 0건 주장 — 맞습니다. origin/develop 기준으로 세 심볼 모두 자기 정의 파일 밖 참조가 0개입니다. 본문이 「그 자리를 메웠다」고 지목한 EmotionKeywordCard·DiaryReportCard 도 실재합니다. 세 사본의 파라미터가 modifier 뿐이라 실데이터를 못 받는다는 점까지 합치면 삭제 판정에 이견 없습니다.
로딩 치환 — 렌더가 같습니다. LoadingBody 는 CircularProgressIndicator(Modifier.size(40.dp)) 이고, material3 가 내부에서 붙이는 CircularIndicatorDiameter 도 40.dp 입니다. 호출자 modifier 뒤에 붙으므로 바깥 size 가 제약을 정하는데 값이 같아 결과가 동일합니다 — 본문 논증 그대로입니다. 이슈 본문의 「픽셀 동일」을 근거 없이 옮기지 않고 소스로 확인하신 것이 맞는 순서입니다.
신설 가드 — 계약을 정확히 고정합니다. RecordActionAffordanceTest 2건이 「핸들러 없으면 안 그린다 / 있으면 그린다」 양방향을 잡습니다. nullable 재모델링이 DailyQuestionListCard → TextCardContent·ThumbnailCardContent·CardHeaderRow 까지 일관되게 내려가고, 디폴트를 없애 컴파일이 배선을 강제하게 한 것도 이 이슈의 처방과 맞습니다.
konsist 허용목록 16개 제거 — 이게 이 PR 에서 제일 값이 큽니다. 관대 판정 목록을 남겨 두면 다음 위반이 그 자리에 숨는다는 지적이 정확하고, { _, _ -> } 4건을 그 가드가 실제로 잡아낸 것이 목록을 비울 근거가 됩니다.
판단이 들어간 자리 — 처리 방식에 동의합니다. 주간 리포트 HISTORY 의 «더보기» 를 배선 대신 감춘 것은 판단이고, 그 사실을 코드 주석과 docs/qa/assumptions.md 양쪽에 남기면서 되돌리는 법까지 적어 두셨습니다(「실핸들러를 넘기면 메뉴가 돌아온다」). 판단을 숨기지 않고 다음 사람이 뒤집을 수 있게 남기는 것이 맞는 형태입니다. 읽기 전용 요약이라는 해석 자체도 화면 성격과 어긋나지 않습니다.
검증
| 항목 | 결과 |
|---|---|
| Repository Quality | fail — Require linked Issue, 원인은 제목(위) |
| 나머지 CI | Ktlint·CodeQL(js·classify)·계측 4종 pass / Unit Test·Screenshot·Android Lint·java-kotlin pending |
| 삭제 3건 참조 | origin/develop 에서 0건 실측 (자기 정의 파일 제외) |
| 대체 컴포넌트 실재 | EmotionKeywordCard·DiaryReportCard 확인 |
LoadingBody 렌더 동일성 |
소스 확인 — 40.dp 로 동일, modifier 결합 순서까지 무해 |
| 게이트 규칙 대조 | validate-pr-issue-link.mjs 실코드로 확인 — 제목 1개 필수, 본문 다중 Closes 무관 |
| 시안 대조 | 미실시 — 삭제 판정의 결정 근거를 「참조 0건 + 같은 자리 실구현 존재 + 파라미터가 modifier 뿐이라 배선 불가」로 봤고 그 셋을 코드로 확인했습니다. 본문의 node-id 별 대조는 그대로 받습니다 |
| 실기 QA | 미실시 — 이 PR 이 고친 배선 2건 중 하나는 신설 JVM 가드가, 다른 하나는 컴파일이 강제합니다. 시각 회귀는 Screenshot 이 판정할 자리이고 아직 pending 입니다 |
|
❌ Screenshot test fail — baseline 과 CI rendered PNG 차이. 📦 Artifact: https://github.com/Afternote/Afternote-FE/actions/runs/33292981965 (페이지 하단 Artifacts → 🔍 확인 위치 (artifact zip 내부):
📝 조치 가이드:
CI ↔ 로컬 환경 차이는 본 워크플로의 container 단계가 root fix 한다 (README 의 "Compose Preview Screenshot Testing (docker baseline)" 섹션 참조). |
OnlyReadOneBook
left a comment
There was a problem hiding this comment.
먼저 정정합니다 — 제목은 제가 리뷰를 쓰기 전에 이미 고치셨습니다.
| 시각 | 일 |
|---|---|
| 04:27:24 | Repository Quality 가 옛 제목으로 fail |
| 04:30:23 | 제목을 (#1540) 으로 수정 |
| 04:32:11 | 제가 「제목이 blocker」로 변경요청 |
제가 04:27 의 스테일한 run 을 보고 이미 해결된 것을 지적했습니다. 본문에 규칙 설명까지 덧붙여 두신 것도 확인했습니다.
그런데 그 수정으로 게이트가 열리자 진짜 검증이 돌았고, 거기서 2건이 실패했습니다. 결과적으로 그게 이 PR 의 실제 blocker 입니다.
[blocker] 디폴트를 걷어낸 호출부가 두 소스셋에 남아 있습니다
두 실패가 원인이 같습니다 — = {} 제거가 main·preview·screenshotTest·unit test 까지만 반영되고 나머지 두 곳이 빠졌습니다.
1. :app:compileDebugAndroidTestKotlin FAILED — Unit Test job
app/src/androidTest/…/DailyQuestionResumeFailureAndroidTest.kt:77:65 No value passed for parameter 'onBackClick'
app/src/androidTest/…/DailyQuestionResumeFailureAndroidTest.kt:77:65 No value passed for parameter 'onDraftListClick'
app/src/androidTest/…/DailyQuestionResumeFailureAndroidTest.kt:131:65 (동일 2건)
app/src/androidTest/…/TimeLetterMindRecordCompletionAndroidTest.kt:370:47 No value passed for parameter 'onEditClick'
app/src/androidTest/…/TimeLetterMindRecordCompletionAndroidTest.kt:430:51 (동일)
app/src/androidTest/…/TimeLetterMindRecordCompletionAndroidTest.kt:434:25 onBackClick · onDraftListClick
app/src/androidTest/…/TimeLetterMindRecordCompletionAndroidTest.kt:528:25 onBackClick · onDailyQuestionDraftClick
app/src/androidTest/…/TimeLetterMindRecordCompletionAndroidTest.kt:552:25 onBackClick · onDraftListClick
이 저장소는 8/24 부터 unit-test.yml 이 androidTest 컴파일까지 게이트합니다(#949). app/src/androidTest 는 mindrecord 화면을 직접 호출하는 자리가 있어 이 변경의 사용처에 포함됩니다.
2. :feature:home:presentation:compileDebugScreenshotTestKotlin FAILED — Screenshot job
feature/home/presentation/src/screenshotTest/…/WeeklySummaryGridScreenshotTest.kt:16:27
No value passed for parameter … (3건)
WeeklySummaryGrid.kt 의 디폴트를 걷으면서 같은 모듈 screenshotTest 호출부가 남았습니다.
로컬 검증 범위가 이 둘을 덮지 않았습니다
본문 검증표를 보면 원인이 드러납니다.
| 돌린 것 | 빠진 것 |
|---|---|
:feature:mindrecord:presentation:testDebugUnitTest |
:app:compileDebugAndroidTestKotlin |
:feature:home:presentation:testDebugUnitTest |
:feature:home:presentation:compileDebugScreenshotTestKotlin |
:feature:mindrecord:presentation:compileDebugScreenshotTestKotlin |
mindrecord 쪽 screenshotTest 는 돌리셨는데 home 쪽은 같은 태스크가 빠졌고, app 의 androidTest 컴파일은 양쪽 다 없습니다. 다음에 디폴트를 걷을 때는 이 두 태스크를 함께 거는 편이 안전합니다 — 콜백 시그니처 변경은 소스셋을 가리지 않고 번집니다.
지난 리뷰에서 확인한 것은 그대로 유효합니다
참조 0건(develop 실측)·LoadingBody 렌더 동일성·RecordActionAffordanceTest 의 양방향 가드·konsist 허용목록 16개 제거·판단을 assumptions.md 에 남긴 방식 — 전부 이견 없습니다. 위 두 컴파일만 통과하면 승인할 내용입니다.
검증
| 항목 | 결과 |
|---|---|
| Repository Quality | pass — 제목 수정으로 해소 |
| Unit Test | fail — :app:compileDebugAndroidTestKotlin, 위 8자리 |
| Screenshot | fail — :feature:home:presentation:compileDebugScreenshotTestKotlin, 위 1파일 |
| 나머지 | Ktlint·Android Lint·CodeQL(java-kotlin·js)·계측 4종·Markdown links pass (SUCCESS 15) |
| develop 상존 red 여부 | 해당 없음 — 두 실패 모두 이 PR 이 바꾼 시그니처를 직접 가리킵니다 |
| 시안 대조 | 지난 리뷰와 같음 — 삭제 판정 근거를 코드로 확인, 본문의 node-id 대조는 그대로 받습니다 |
| 실기 QA | 미실시 — 컴파일이 막혀 빌드가 나오지 않습니다 |
앞 커밋이 `:feature:mindrecord:presentation` 과 `:feature:home:presentation` 의 main·test
소스만 컴파일해 두 소스셋을 빠뜨렸다. CI 가 두 갈래로 잡았다.
| 잡 | 원인 |
|---|---|
| `Unit Test / Run Unit Tests` | `:app:compileDebugAndroidTestKotlin` — 계측 2파일 7곳 |
| `Screenshot / Validate Compose Preview Screenshots` | `:feature:home:presentation:compileDebugScreenshotTestKotlin` — `WeeklySummaryGridScreenshotTest` 1곳 |
home 의 screenshotTest 가 컴파일되지 않아 그 모듈은 **골든을 한 장도 렌더하지 못했다** —
아티팩트에 home 파일이 0건인 것이 그 흔적이다. 픽셀 차이가 아니었다.
> mindrecord 쪽은 CI 가 렌더한 11장이 레포 골든과 **바이트 동일**했다. 로딩 통합이 렌더를
> 바꾸지 않는다는 주장이 CI 환경에서도 확인된 셈이다.
`TimeLetterMindRecordCompletionAndroidTest` 의 `onEditClick` 은 `DailyQuestionAnswerListScreen`
쪽이라 `(Long) -> Unit` 이다 — 일괄 치환이 `{ _, _ -> }` 를 넣어 한 번 더 깨졌고 실인자로
고쳤다. 같은 이름이 화면마다 arity 가 다르다.
이번엔 이 변경을 참조할 수 있는 **9개 소스셋 전부**를 컴파일해 확인했다
(app main·unitTest·androidTest / mindrecord·home 의 main·unitTest·screenshotTest).
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
CI 실패 3건을 모두 닫았습니다. 원인은 제가 좁은 범위만 컴파일한 것이었습니다.
곁들여 확인된 것 — mindrecord 쪽은 CI 가 렌더한 11장이 레포 골든과 바이트 동일했습니다. 「로딩 통합이 렌더를 바꾸지 않는다」가 CI 환경에서도 확인된 셈입니다(로컬 macOS 대조로는 develop 자체가 6건 실패해 절대 건수로는 판별이 안 됩니다). 계측 결과 — 선언한 selector 10건을 이번엔 이 변경을 참조할 수 있는 9개 소스셋 전부를 컴파일했습니다 — app 의 main·unitTest·androidTest, mindrecord·home 의 main·unitTest·screenshotTest.
|
…record-presentation-cleanup
|
두 건 모두 맞고,
진단이 정확했습니다 — 특히 두 번째 건은 픽셀 차이가 아니라 컴파일이 안 돼 home 이 골든을 한 장도 렌더하지 못한 것이었습니다. 아티팩트에 home 파일이 0건인 게 그 흔적입니다.
검증 범위 지적을 받아들였습니다말씀대로 콜백 시그니처 변경은 소스셋을 가리지 않고 번지는데 제가 모듈 단위로만 생각했습니다. 이번엔 이 변경을 참조할 수 있는 9개 소스셋 전부를 걸었습니다.
그 뒤에 하나 더 있었습니다계측 파일을 건드렸으므로 일괄 치환이 현재 fail=0 / success=15 입니다. develop(77커밋 뒤처져 있었습니다)도 병합했습니다. |
|
이 PR 은
|
#966 이 착륙하며 이 PR 이 손댄 컴포저블 대부분이 develop 에서 함께 바뀌어 11파일이 충돌했다. **해소 방식**: 충돌 Kotlin 파일은 **develop 판을 채택한 뒤 이 PR 의 변환을 다시 적용**했다. 손으로 hunk 를 고르면 develop 이 새로 넣은 파라미터를 놓치기 쉬운데, 이 PR 의 변경은 「no-op 디폴트를 걷고 호출부에 실인자를 명시한다」는 기계적 규칙이라 재적용이 더 안전하다. develop 판에는 디폴트가 24건 있었다(원래 세었던 것보다 많다 — #966 스택이 컴포저블을 더 늘렸다). 전부 걷고 컴파일러가 드러낸 호출부를 채웠다. **main 소스 잔여 0건.** ## 다시 세운 것 — 병합으로 사라졌던 자리 develop 판을 채택하면서 이 PR 고유의 세 가지가 함께 날아가 하나씩 복원했다. | 잃었던 것 | 복원 | |---|---| | `DailyQuestionListCard` 의 nullable 재모델링 | 시그니처 4곳 + 「핸들러 없으면 «더보기» 를 그리지 않는다」 조건 | | `DailyQuestionAnswerListScreen`·`DiaryScreen` 의 `LoadingBody` 치환 | `private fun LoadingBox` 두 벌 제거·import 정리 | | — | `WriteTextField` 의 툴바 배선과 카드 3종 삭제는 충돌하지 않아 그대로 남았다(확인함) | ## 일괄 치환이 두 번 틀렸다 `{}` 를 arity 무시하고 넣어 `(Long, YearMonth) -> Unit` 자리에서 깨졌다. 컴파일러가 기대 시그니처를 알려 주므로 그 파라미터 수만큼 `{ _, _ -> }` 를 넣도록 고쳤다 — 3-인자 자리도 하나 있었다(`HomeScreen.kt:211`). **같은 이름이 화면마다 arity 가 다르다는 것을 또 놓쳤다.** ## 검증 — 9개 소스셋 전부 ``` mindrecord : testDebugUnitTest · compileDebugScreenshotTestKotlin · ktlintCheck home : testDebugUnitTest · compileDebugScreenshotTestKotlin · ktlintCheck app : compileDebugAndroidTestKotlin · compileDebugUnitTestKotlin · ktlintCheck konsist : test · ktlintCheck ``` 전부 BUILD SUCCESSFUL. `RecordActionAffordanceTest` 도 통과 — 복원한 nullable 계약이 살아 있다. `assumptions.md` 는 양쪽 항목을 모두 남겼다(서로 다른 이슈 기록). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`docs/qa/assumptions.md` 한 블록만 충돌해 양쪽 항목을 모두 남겼다. 앞선 병합에서 이미 develop 판 위에 이 PR 의 변환을 재적용해 둬서 코드 충돌은 남지 않았다. 네 갈래가 병합 뒤에도 그대로인 것을 확인했다. | 갈래 | 확인 | |---|---| | 로딩 통합 (#1303·#1490) | `LoadingBox` 잔여 0 | | 참조 0건 카드 삭제 (#1422) | 세 파일 모두 없음 | | no-op 디폴트 (#1540) | main 소스 잔여 0 · konsist 허용목록 2집합 비어 있음 | | 배선 누락 2건 수정 | `TextStyleToolbar` 배선 유지 · `DailyQuestionListCard` nullable 4곳 유지 | 가드 `RecordActionAffordanceTest` 통과. 검증 — 이 변경을 참조할 수 있는 소스셋 전부: mindrecord : testDebugUnitTest · compileDebugScreenshotTestKotlin · ktlintCheck home : testDebugUnitTest · compileDebugScreenshotTestKotlin · ktlintCheck app : compileDebugAndroidTestKotlin · compileDebugUnitTestKotlin · ktlintCheck konsist : test · ktlintCheck 전부 BUILD SUCCESSFUL. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
develop 이 48커밋 앞서 있었다. `docs/qa/assumptions.md` 한 블록만 충돌해 양쪽 항목을 보존했다. 네 갈래가 그대로인 것을 확인했다 — `LoadingBox` 잔여 0, 삭제 카드 3종 없음, main 소스 no-op 디폴트 0, `TextStyleToolbar` 배선·`DailyQuestionListCard` nullable 4곳 유지. 검증: mindrecord·home 의 unitTest·screenshotTest, app 의 androidTest·unitTest, 네 모듈 ktlint, konsist 전량 — 전부 BUILD SUCCESSFUL. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
@1hyok 지적하신 두 컴파일 실패를 모두 고쳤습니다. 재검토 부탁드립니다. 고친 것 —
|
| 파일 | 변경 |
|---|---|
app/src/androidTest/…/DailyQuestionResumeFailureAndroidTest.kt |
+12/-2 — onBackClick·onDraftListClick (77·131행) |
app/src/androidTest/…/TimeLetterMindRecordCompletionAndroidTest.kt |
+14/-2 — onEditClick·onBackClick·onDraftListClick·onDailyQuestionDraftClick (370·430·434·528·552행) |
feature/home/presentation/src/screenshotTest/…/WeeklySummaryGridScreenshotTest.kt |
+6/-1 — 3건 |
지적하신 8자리 + 1파일과 정확히 같은 자리입니다.
현재 CI
전 항목 green 입니다 — 실패 0.
| 잡 | 결과 |
|---|---|
Unit Test / Run Unit Tests (= :app:compileDebugAndroidTestKotlin 포함) |
SUCCESS |
Screenshot / Validate Compose Preview Screenshots |
SUCCESS |
Repository Quality |
SUCCESS |
| Ktlint · Android Lint · CodeQL(java-kotlin·js) · 계측 4종 · Markdown links | SUCCESS |
검증 범위 지적 — 받습니다
다음에 디폴트를 걷을 때는 이 두 태스크를 함께 거는 편이 안전합니다 — 콜백 시그니처 변경은 소스셋을 가리지 않고 번집니다.
맞는 지적입니다. 로컬에서 mindrecord 쪽 compileDebugScreenshotTestKotlin 은 돌리고 home 쪽 같은 태스크를 빠뜨린 것, :app:compileDebugAndroidTestKotlin 을 양쪽 다 안 건 것이 이번 red 의 직접 원인이었습니다. 시그니처를 바꾸는 변경에서는 모듈이 아니라 소스셋 을 기준으로 태스크를 거는 쪽으로 바꾸겠습니다.
첫 리뷰의 제목 건은 이미 정정해 주신 대로입니다 — 추가로 손댄 것 없습니다.
OnlyReadOneBook
left a comment
There was a problem hiding this comment.
공통 로딩 전환, 미사용 UI 정리, 이전 source-set 컴파일 누락 수정은 확인했고 현재 검사는 모두 통과합니다. 다만 no-op 상호작용 1건이 프로덕션 호출부에 남아 있어 인라인 수정이 필요합니다.
| answer = dailyQuestion, | ||
| onEdit = null, | ||
| onDelete = null, | ||
| onClick = {}, |
There was a problem hiding this comment.
[blocking] 이 줄은 no-op 기본값을 걷어낸 대신 같은 no-op을 호출부에 명시적으로 남깁니다. DailyQuestionListCard는 항상 Role.Button clickable을 붙이므로 HISTORY의 읽기 전용 카드가 스크린리더에는 버튼으로 노출되지만 눌러도 반응하지 않습니다. onClick도 nullable/non-clickable variant로 모델링하거나 실제 상세 이동을 연결하고, click semantics의 존재·부재를 양방향 테스트로 고정해 주세요.
리뷰 지적이 맞다. 디폴트를 걷어낸 대신 **같은 no-op 을 호출부에 명시**해 둔 자리가 남아 있었다.
`DailyQuestionListCard` 는 `onClick` 에 항상 `Role.Button` clickable 을 붙인다. 주간 리포트
HISTORY 는 읽기 전용인데 `onClick = {}` 를 넘겨, **스크린리더가 「버튼」으로 읽는데 눌러도 아무
일이 없었다.** #1540 이 없애려던 「눌러도 아무 일 없는 버튼」이 형태만 바꿔 남은 셈이다.
`onClick` 을 nullable 로 바꿔 **`null` 이면 clickable 자체를 안 붙인다** — `onEdit`·`onDelete` 와
같은 처방이다. 주간 리포트는 셋 다 `null` 이라 클릭 semantics 가 없다.
가드 2건(양방향):
onClick 이 없으면 카드에 클릭 semantics 를 붙이지 않는다 → hasClickAction 노드 0
onClick 이 있으면 카드가 눌린다 → 노드 1 + 콜백 1회
역검증: 항상 clickable 로 되돌리면 앞쪽 FAILED.
> `RecordListMonthTest` 1건이 실패하지만 develop 을 그대로 체크아웃해도 같은 1건이 실패한다
> (`Failed to inject touch input`). 이 PR 과 무관하다.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
지적이 맞습니다.
말씀대로 양방향으로 고정했습니다.
역검증: 항상 clickable 로 되돌리면 앞쪽 FAILED.
|
nullable 로 바꾸며 새 KDoc 을 넣고 옛 것을 안 지워, 파라미터 하나에 KDoc 이 둘이 됐다 (`standard:no-consecutive-comments`). 두 문장을 한 KDoc 으로 합쳤다. `ktlintFormat` 은 이 규칙을 자동 교정하지 못한다 — 포맷만 돌리고 `ktlintCheck` 를 안 돌려 로컬에서 못 봤다. 이번엔 저장소 전체 `ktlintCheck` 로 확인했다. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`RecordListMonthTest > 카드를 누르면 보고 있는 달이 함께 전달된다` 가 이 브랜치에서 `Failed to inject touch input … There are no existing nodes` 로 깨졌다. **이 PR 이 원인이다.** 앞선 커밋·댓글에서 「develop 에서도 실패하는 기존 건」이라고 적었는데 틀렸다 — 그때 `git stash` 로 비교한 것이 develop 이 아니라 이 브랜치의 커밋 상태였다. 격리 실행으로 다시 재면 develop 6회 전부 통과, 이 브랜치 3회 전부 실패다. 원인은 클릭이 아니라 **합성 범위**다. 목록은 캘린더가 먼저 오는 LazyColumn 이고 테스트 창은 320×470px 라, 기록 카드가 보이는 영역 밖이면 아예 합성되지 않아 노드가 없다. 스크롤 위치에 기대고 있던 테스트라 이 PR 의 레이아웃 높이 변화에 그대로 걸렸다. 이 테스트가 보려는 것은 «누르면 무엇이 넘어가는가» 이므로 항목까지 스크롤해 놓고 누른다. 높이가 또 달라져도 깨지지 않는다. 3회 연속 통과를 확인했다. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
앞서 「develop 에서도 실패하는 기존 건」이라고 한 것은 틀렸습니다. 비교를 잘못했습니다 —
원인은 클릭이 아니라 합성 범위입니다. 목록은 캘린더가 먼저 오는 항목까지 스크롤한 뒤 누르도록 고쳤습니다 — 이 테스트가 보려는 건 «누르면 무엇이 넘어가는가» 라서 스크롤 위치에 기대지 않는 편이 맞습니다. 3회 연속 통과를 확인했습니다. ktlint 실패도 함께 고쳤습니다. nullable 로 바꾸며 새 KDoc 을 넣고 옛 것을 안 지워 파라미터 하나에 KDoc 이 둘이었습니다( |
…record-presentation-cleanup # Conflicts: # feature/mindrecord/presentation/src/test/kotlin/com/afternote/feature/mindrecord/presentation/screen/sender/RecordListMonthTest.kt
|
바로 앞 댓글의 「이 PR 이 원인이다」도 과했습니다 — 정정합니다. develop 을 병합하면서 알았는데, 같은 스크롤 수정이 오늘 09:57 에 #1700(@1hyok, 정확한 그림은 이렇습니다.
develop 병합으로 #1700 판을 받았고, 제가 브랜치에서 쓴 중복 수정은 develop 형식에 맞춰 정리했습니다(주석에 #1700 을 남겼습니다). 충돌도 함께 해소했습니다. 세 번 말을 바꿔 죄송합니다. 검증 결과만 정리하면 — 병합 후 |
OnlyReadOneBook
left a comment
There was a problem hiding this comment.
재리뷰 — 3차 변경요청의 blocker 가 head 에서 실제로 닫힌 것을 코드로 확인했습니다. 승인합니다.
이전 지적 대조
| # | 지적 | 판정 | 근거 |
|---|---|---|---|
| 1 | 제목 (#1540 외 3건) 링크 게이트 |
닫힘 | 제목 (#1540), Repository Quality pass |
| 2 | :app:compileDebugAndroidTestKotlin 실인자 8자리 |
닫힘 | DailyQuestionResumeFailureAndroidTest 77·133, TimeLetterMindRecordCompletionAndroidTest 371~570 에 콜백 명시, Unit Test pass |
| 3 | :feature:home:presentation:compileDebugScreenshotTestKotlin |
닫힘 | WeeklySummaryGridScreenshotTest 16~21 명시, Screenshot pass |
| 4 | [blocking] HISTORY 카드 onClick = {} 가 Role.Button 을 실어 스크린리더에 죽은 버튼 노출 |
닫힘 | DailyQuestionListCard.kt:61-63 셋 다 (() -> Unit)?, :72-75 null 이면 clickable 미부착, WeeklyReportScreen.kt:158-163 셋 다 null, RecordActionAffordanceTest.kt:83-107 양방향(클릭 semantics 0건 / 1건+콜백 1회) |
인라인 스레드는 outdated 로 남아 있어 제가 resolve 하지 않았습니다. main 소스에 = {}·{ _, _ -> } 디폴트 잔존 0건, konsist 허용목록 비움 확인.
새 지적
- [nit] 같은 취지 주석 두 벌 →
WeeklyReportScreen.kt:155인라인. - [nit]
onEdit/onDelete한쪽만 넘기면 넘긴 쪽도 사라진다 →DailyQuestionListCard.kt:190인라인. 현재 호출부 2곳은 둘 다 같이 넘기거나 같이 null 이라 영향 없음. - [nit] 판단 기록이 «더보기» 숨김만 적고 카드 탭(#759 상세 이동) 비활성 판단은 코드 주석에만 있다 →
docs/qa/assumptions.md:415인라인.
검증
- CI:
gh pr checks15 pass / 0 fail (head0fd1683ee, 제출 직전 재확인). - 리뷰 이후 커밋 4건 — 저자 실변경 3(
ea3755eb1nullable+테스트,4951c6eaeKDoc 합침,fbd69a15d스크롤 후 클릭) + develop 병합 1.fbd69a15d는 develop #1700 이 같은 수정을 먼저 넣어 net diff 는 주석·인자만 남고 스크롤 호출 1개(중복 없음). WriteTextField.kt:287-288배선 두 줄은BottomToolbar294·298 과 같은 스코프 변수(sheet·showTextStyleToolbar)를 쓴다.LoadingBody는 옛LoadingBox와 구조 동일(fillMaxSize+size(40.dp)). 삭제 문자열 6건 head 참조 0건.- 시안 대조: 삭제 카드 3건의 근거 노드(
4327:77929·4327:77573)는 본문 판정 그대로. HISTORY 카드의 «더보기» 유무는 본문·판단 기록에 시안 노드 링크가 없어 대조하지 못했고, 1차 리뷰에서 동의한 읽기 전용 해석을 유지합니다. 판단 기록에 해당 노드를 붙여 주시면 닫힙니다. - 실기동: 미수행.
TextStyleToolbar링크·«T» 버튼 배선은 JVM 가드가 없어 실기동으로만 확인되는 항목이라 남깁니다 — 머지 전 한 번 눌러 보시길.
| DailyQuestionListCard(answer = dailyQuestion) | ||
| // 주간 리포트는 읽기 전용 요약이다 — 편집·삭제는 데일리질문 목록의 몫이라 | ||
| // 여기서는 «더보기» 를 그리지 않는다 (#1540). | ||
| // 주간 리포트는 **읽기 전용 요약**이다 — 편집·삭제도, 카드 탭도 없다. |
There was a problem hiding this comment.
[nit] 153154행(1차, «더보기»를 그리지 않는다)과 155157행(재리뷰, 편집·삭제도 카드 탭도 없다)이 같은 취지로 겹칩니다. 뒤 문단 하나로 합쳐도 됩니다.
| // 종전에는 `= {}` 디폴트라, 핸들러를 안 넘긴 화면(주간 리포트 HISTORY)에서도 메뉴가 | ||
| // 뜨고 «수정»·«삭제» 를 눌러도 아무 일이 없었다. 눌러도 아무 일 없는 버튼을 그리는 | ||
| // 대신 없는 상호작용은 보여 주지 않는다. | ||
| if (onEdit != null && onDelete != null) { |
There was a problem hiding this comment.
[nit] onEdit != null && onDelete != null 이라 한쪽만 넘긴 호출부는 넘긴 핸들러까지 조용히 사라집니다. 지금 호출부 2곳(DailyQuestionAnswerListScreen:204, WeeklyReportScreen:158)은 둘 다 같이 넘기거나 같이 null 이라 영향은 없지만, 의도가 «둘 다 있을 때만 메뉴» 라면 KDoc 에 한 줄 적어 두면 다음 호출부가 헷갈리지 않습니다.
|
|
||
| `WeeklyReportScreen` 이 `DailyQuestionListCard(answer = …)` 만 넘겨, 수정·삭제 메뉴가 뜨는데 | ||
| 둘 다 no-op 이었다. | ||
|
|
There was a problem hiding this comment.
[nit] 이 절은 «더보기» 숨김만 적고, 재리뷰에서 추가된 카드 탭(#759 상세 이동)도 안 붙인다는 판단은 WeeklyReportScreen.kt 주석에만 있습니다. 나중에 「주간 리포트에서 상세로 못 가네」가 나올 때 문서 근거가 되도록 한 줄 보태 주세요.
…record-presentation-cleanup # Conflicts: # feature/mindrecord/presentation/src/main/kotlin/com/afternote/feature/mindrecord/presentation/component/MemoriesCard.kt
이 PR 이 no-op 디폴트를 걷는 동안 develop 에 새 테스트가 들어와, 병합 뒤 그쪽이 디폴트에 기대 컴파일이 깨졌다. WriteTextField 두 곳과 WeeklyReportReviewCard 한 곳에 인자를 채운다. MemoriesCard 의 onReadAgainClick 은 develop 판(nullable)을 택했다 — 이 PR 의 «no-op 디폴트 제거» 의도를 포함하면서 CTA 부재까지 표현한다 (#793). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Closes #1540
Closes #1490
Closes #1303
Closes #1422
담당 이슈 4건을 한 PR 로 묶었습니다. 셋 다
feature/mindrecord/presentation의 동작 변경 없는 정리라 나누면 같은 파일을 세 번 건드리고 스크린샷 골든도 세 번 재생성해야 합니다. (#1303 과 #1490 은 같은 작업입니다 — 같은 5개 파일, 같은 치환.)세 갈래를 커밋으로 나눠 뒀으니 커밋 단위로 보시면 됩니다.
1. 로딩 스피너 5곳 →
core:uiLoadingBody(#1303 · #1490)private fun LoadingBox4벌 +DraftListScreen인라인 1곳.렌더가 같다는 근거. 넷은 전부
fillMaxSize+ 중앙 정렬 + 기본 지름이라 구조가 동일합니다. 유일한 차이인size(40.dp)는 material3 기본값과 같은 값입니다 —이슈 본문이 「픽셀 동일」이라고 적어 뒀지만 근거가 없어 소스로 확인했습니다.
2. 참조 0건 카드 3건 삭제 (#1422)
이슈가 「삭제로 단정하지 않고 판정째 넘긴다」고 해서 정본 시안과 대조했습니다. 전부 삭제 판정입니다 — 미구현 화면의 선행 정의가 아니라 이미 다른 컴포넌트가 그 자리를 메운 뒤 남은 사본이었습니다.
InsightCardINSIGHTS4327:77929EmotionKeywordCard의descriptionText. 시안에서도 나중에 추가된 감정키워드 카드(4327:99437)가 같은 두 문단을 흡수했고, 옛INSIGHTS컨테이너가 중복으로 남아 있습니다WeeklyEmotionCard4327:77573DiaryReportCard—이번 달/주간 평균 기분2열 통계를 실데이터(monthDiaryCount·weeklyMoodEmoji)로 그립니다WeeklyReportCard주간 리포트·이번 주 나의 기록들을 확인해 보세요.)가 시안 전체에 0건세 사본 모두 파라미터가
modifier뿐이라 실데이터를 못 받습니다. 살려 둬도 배선할 수 없고, 같은 자리에 실구현이 있어 다음 사람이 어느 쪽을 쓸지 헷갈리는 값만 남습니다.전용 문자열 6건도 함께 걷었습니다(다른 참조 0건 확인). 이 세 Preview 의 screenshot 골든은 없습니다.
3. no-op 콜백 디폴트 50건 (#1540)
mindrecord 47 + home 3. main 소스 잔여 0건입니다.
디폴트가 실제로 결함을 가리고 있었습니다
이슈가 「디폴트가 가려 온 결함이 있는지 확인」을 완료 조건에 뒀습니다. 걷어내자 컴파일러가 실호출부 2곳을 드러냈습니다 — 둘 다 사용자에게 버튼이 보이는데 눌러도 아무 일이 없던 자리입니다.
(1)
WriteTextField→TextStyleToolbar—onLinkClick·onTypeClick미전달. 링크 아이콘(core_ui_ic_link, CD 「링크」)과 «T» 버튼이 그려지는데 핸들러가 없었습니다.같은 파일
BottomToolbar가 같은 두 버튼을 갖고 있어 배선이 명확했습니다 — 스타일 툴바가 열리면 하단 툴바를 덮으므로 같은 affordance 를 유지해야 합니다.sheet = KeyboardSheet.MediaSelectshowTextStyleToolbar = !showTextStyleToolbar(2)
WeeklyReportScreenHISTORY →DailyQuestionListCard—onEdit·onDelete미전달. «더보기» 메뉴가 뜨고 수정·삭제가 전부 no-op 이었습니다.DailyQuestionListCard를 nullable 핸들러로 재모델링해 핸들러가 없으면 메뉴 자체를 그리지 않게 했고, 읽기 전용 요약인 주간 리포트는null을 명시합니다. #1540 이 든 처방(「= {}로 죽은 버튼을 그리는 대신 nullable + UI 숨김」) 그대로입니다.가드
RecordActionAffordanceTest2건 — 핸들러가 없으면 안 그리고, 있으면 그린다.역검증: 항상 그리도록(
if (true)+onEdit?.invoke()) 되돌리면핸들러가 없으면 더보기를 그리지 않는다FAILED.konsist 허용목록을 비웠습니다
NoOpCallbackDefaultKonsistTest.LEGACY_NO_OP_DEFAULT_FILES의 mindrecord·home 항목 16개를 걷었습니다. 남겨 두면 다음 위반이 그 자리에 숨습니다(그 목록은 관대 판정이라 위반이 있어도 통과). 이제 두 모듈은 신규 유입이 곧바로 red 입니다.timeletter(#1541) 만 남습니다.나머지 호출부
프리뷰·screenshotTest·unit test 30곳은
{}명시로 바꿨습니다 — 종전 디폴트와 동작이 같습니다.검증
:feature:mindrecord:presentation:testDebugUnitTest:feature:home:presentation:testDebugUnitTest:feature:mindrecord:presentation:compileDebugScreenshotTestKotlinktlintCheck(mindrecord·home·konsist):konsist:test --tests '*NoOpCallbackDefault*'pixel2Api30— 아래 selector 10건tests="10" failures="0" errors="0", 결과 XML 의testcase name이 선언과 정확히 일치)CI Test Plan
{ "androidTest": { "mode": "selected", "reason": "프로덕션 변경은 컴포저블 파라미터 디폴트 제거와 배선 2건이라 내비게이션·네트워크 경로가 그대로다. 다만 그 디폴트에 기대던 계측 호출부 2파일을 함께 고쳤으므로, 두 파일의 @Test 전량이 실제로 통과하는지 API 30 에서 확인한다.", "tests": [ { "path": "app/src/androidTest/java/com/afternote/afternote_fe/DailyQuestionResumeFailureAndroidTest.kt", "selector": "com.afternote.afternote_fe.DailyQuestionResumeFailureAndroidTest#resumeFailure_showsWarningAndBlocksSave", "device": "api30" }, { "path": "app/src/androidTest/java/com/afternote/afternote_fe/DailyQuestionResumeFailureAndroidTest.kt", "selector": "com.afternote.afternote_fe.DailyQuestionResumeFailureAndroidTest#resumeFailure_retryButtonReachesViewModelAndReopensSave", "device": "api30" }, { "path": "app/src/androidTest/java/com/afternote/afternote_fe/TimeLetterMindRecordCompletionAndroidTest.kt", "selector": "com.afternote.afternote_fe.TimeLetterMindRecordCompletionAndroidTest#mindRecordHome_dailyQuestionLoadingEmptyAndErrorRetrySuccess_areRendered", "device": "api30" }, { "path": "app/src/androidTest/java/com/afternote/afternote_fe/TimeLetterMindRecordCompletionAndroidTest.kt", "selector": "com.afternote.afternote_fe.TimeLetterMindRecordCompletionAndroidTest#dailyQuestionWrite_successRefreshesExistingListWithCreatedAnswer", "device": "api30" }, { "path": "app/src/androidTest/java/com/afternote/afternote_fe/TimeLetterMindRecordCompletionAndroidTest.kt", "selector": "com.afternote.afternote_fe.TimeLetterMindRecordCompletionAndroidTest#diaryDraftRow_routesIdAndMonthThenPrefillsAndPublishesWithPatch", "device": "api30" }, { "path": "app/src/androidTest/java/com/afternote/afternote_fe/TimeLetterMindRecordCompletionAndroidTest.kt", "selector": "com.afternote.afternote_fe.TimeLetterMindRecordCompletionAndroidTest#memorySpace_supportedSuccess_opensAndClosesDetailThenNavigatesBack", "device": "api30" }, { "path": "app/src/androidTest/java/com/afternote/afternote_fe/TimeLetterMindRecordCompletionAndroidTest.kt", "selector": "com.afternote.afternote_fe.TimeLetterMindRecordCompletionAndroidTest#weeklyReport_errorRetryThenEmptyAndComplete_preservesRequestedWeekContract", "device": "api30" }, { "path": "app/src/androidTest/java/com/afternote/afternote_fe/TimeLetterMindRecordCompletionAndroidTest.kt", "selector": "com.afternote.afternote_fe.TimeLetterMindRecordCompletionAndroidTest#timeLetterWrite_uiValidationAndRapidRegister_preserveInputAndCreateOnce", "device": "api30" }, { "path": "app/src/androidTest/java/com/afternote/afternote_fe/TimeLetterMindRecordCompletionAndroidTest.kt", "selector": "com.afternote.afternote_fe.TimeLetterMindRecordCompletionAndroidTest#timeLetterRecipientSelector_roundTripPreservesTitleTextAndExactReceiverId", "device": "api30" }, { "path": "app/src/androidTest/java/com/afternote/afternote_fe/TimeLetterMindRecordCompletionAndroidTest.kt", "selector": "com.afternote.afternote_fe.TimeLetterMindRecordCompletionAndroidTest#timeLetterSenderDetail_routeIdLoadingFailureRetryAndSuccess_areConnected", "device": "api30" } ] } }