fix: stringFormatter suffix 애니메이션 안정화#11
Conversation
stringFormatter로 ms 같은 suffix를 붙인 값에서 길이 변경 시 suffix가 깜빡이고 easing 효과가 약하게 보이는 문제를 보정했습니다. - 문자열 길이가 바뀔 때 기존 digit 컬럼을 오른쪽 기준으로 보존하고 새 digit은 0 placeholder로 채웁니다. - 숫자가 아닌 suffix, 공백, 구분자는 목표 문자열 위치에 즉시 고정해 ms가 흔들리지 않도록 했습니다. - 변경이 없는 suffix는 state set 대상에서 제외해 불필요한 non-animation transaction을 만들지 않습니다. - non-digit 업데이트를 먼저 처리하고 실제 변경된 digit만 마지막에 animation transaction으로 적용해 easeOut 같은 느려지는 옵션이 숫자 컬럼에 반영되도록 했습니다. 검증: - swift test - DOCS_OUTPUT_PATH=.build/docc-site DOCC_ARCHIVE_PATH=.build/AnimateNumberText.doccarchive ./GeneratingDocumentationSite - git diff --check
|
Warning Review limit reached
More reviews will be available in 6 minutes and 13 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (6)
Walkthrough숫자 판별과 컬럼 재구성이 바뀌었고, 애니메이션 설정은 smooth/reel 스타일 기반으로 전환됐다. 렌더링은 릴 상태를 추적해 분기하며, 관련 테스트와 문서가 함께 갱신됐다. Changes텍스트 자리 재구성 및 릴 애니메이션 분기
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
![mlx-review-bot[bot]](https://avatars.githubusercontent.com/in/3088706?s=60&v=4){kind=small}
There was a problem hiding this comment.
MLX 리뷰
이 PR은 숫자와 문자를 포함한 텍스트의 애니메이션 업데이트 로직을 개선하여, 고정된 접미사(예: "ms")가 유지되며, 숫자 자리수 변경 시 기존 숫자 컬럼을 효율적으로 재사용하도록 수정했습니다. 기존의 placeholder 기반 초기화 방식을 개선하여, 문자열의 고정 부분을 안정적으로 보존하는 동작을 보장합니다.
라인 단위 코멘트
- 라인 단위로 남길 개선 사항은 발견되지 않았습니다.
사용된 모델: mlx-community/Qwen3-30B-A3B-Instruct-2507-8bit
![gemini-pr-review-bot[bot]](https://avatars.githubusercontent.com/in/3443580?s=60&v=4){kind=small}
There was a problem hiding this comment.
이번 PR은 숫자 애니메이션 진행 시 suffix(예: ms)의 위치를 즉시 고정하고 불필요한 트랜잭션을 배제하여 레이아웃이 흔들리는 현상을 훌륭하게 보정했습니다. TextColumn의 ID(UUID)를 보존하여 SwiftUI의 뷰 노드 재사용성을 높인 설계와 NSLock을 사용한 NumberFormatter의 동시성 제어가 매우 인상적입니다. 다만, 아라비아 숫자 외의 일부 특수 유니코드 숫자 문자가 들어왔을 때 오프셋 오류로 화면에서 사라지는 엣지 케이스를 보완하는 등 몇 가지 세부적인 안정성 개선을 제안합니다.
좋은 점
- TextColumn의 UUID를 매핑하여 보존하는 구조: resizeForAnimation에서 기존 숫자 컬럼을 우측 정렬 기준으로 재사용하여 SwiftUI가 뷰 노드를 삭제 후 재구성하지 않고 부드러운 슬라이딩 오프셋 애니메이션을 그리도록 설계한 점이 훌륭합니다.
- NSLock을 통한 NumberFormatter 스레드 안전성 확보: 멀티스레드 환경에서 데이터 경합을 유발할 수 있는 NumberFormatter 인스턴스에 NSLock을 활용하여 안정성을 보장한 점이 매우 좋습니다.
- Non-digit 문자의 동기적 선처리: 공백, 마침표, 단위 suffix 등의 상태 변경은 withoutAnimation 트랜잭션을 활용해 즉시 고정하고, 실제 변경된 자릿수만 애니메이션을 부여해 텍스트 요동을 방지한 패턴이 뛰어납니다.
개선할 점
- 유니코드 입력에 대응하는 숫자 판별 기준 고도화: Character.wholeNumberValue는 분수나 로마 숫자 등도 정수로 반환하므로, 0-9 범위를 벗어나는 문자 입력에 대해 .string으로 안전하게 폴백할 수 있도록 결합도를 낮추는 보강 설계가 필요합니다.
기술 단위 코멘트 1건은 각 라인에 별도 표시됩니다.
(주: 다음 2개 코멘트는 PR diff 범위 밖이라 본문에 모았습니다.)
드롭된 라인 지적
Sources/AnimateNumberText/Private/TextType.swift:15— [Major] value.wholeNumberValue를 통해 숫자를 판별하면 09 이외의 범위(예: 분수, 대형 로마 숫자 등)를 가진 유니코드 문자까지 .number로 오분류될 수 있습니다. 이 경우 09까지만 렌더링되도록 정의된 digitColumn의 오프셋 범위를 크게 벗어나 텍스트가 화면 밖으로 밀려 안 보이게 되는 버그가 생길 수 있으므로, 해당 값이 0...9 범위에 포함되는지 등의 경계 검증을 추가하는 것이 안전합니다.Sources/AnimateNumberText/Private/TextType.swift:42— [Major] init(placeholderFor:)에서도 동일하게 value.wholeNumberValue가 nil이 아닌지를 검사합니다. 0~9 범위를 넘어서는 숫자로 판정된 특수 문자가 들어오면 원래의 문자 정보가 유실되고 무조건 .number(0) placeholder로 치환될 위험이 있으므로 판별 기준을 아라비아 숫자로 좁히는 개선을 권장합니다.
리뷰 생성 모델: Gemini 3.5 Flash (High) #1
| targetColumns[targetIndex] = preservedColumn | ||
| } | ||
|
|
||
| for (index, character) in targetCharacters.enumerated() where !TextType(character).isNumber { |
There was a problem hiding this comment.
[Suggestion] where 절 조건 내의 TextType(character) 판별식과 루프 내부의 let targetValue = TextType(character) 선언에서 동일한 인스턴스 생성이 매번 중복 발생합니다. 루프의 시작 시점에 한 번만 인스턴스를 생성하고 guard 문으로 처리하도록 구조를 리팩터링하면 가독성과 성능 면에서 더 깔끔해집니다.
There was a problem hiding this comment.
수정 완료: 중복 TextType(character) 생성을 제거했습니다. 루프 본문에서 targetValue를 한 번만 만들고 guard !targetValue.isNumber else { continue }로 처리하도록 변경했습니다. 검증: swift test, DocC 생성, git diff --check.
There was a problem hiding this comment.
최신 코드에서는 루프 본문에서 let targetValue = TextType(character)를 한 번만 생성하고 guard !targetValue.isNumber else { continue }로 처리하도록 바뀌어 이 지적은 해결된 것으로 보입니다.
![codex-review-bot[bot]](https://avatars.githubusercontent.com/in/3409825?s=60&v=4){kind=small}
There was a problem hiding this comment.
변경사항은 문자열 길이 변화 시 숫자 컬럼과 suffix 처리를 분리해 기존 깜빡임 문제를 줄이는 방향으로 잘 정리되어 있습니다. 특히 non-digit 업데이트를 비애니메이션으로 먼저 처리하고 digit만 별도 애니메이션으로 갱신하는 흐름은 이번 문제 원인과 잘 맞습니다. 병합을 막을 만한 버그, 예외 처리 누락, 데이터 정합성 문제는 확인되지 않았습니다.
좋은 점
- 숫자 컬럼과 non-digit 컬럼의 갱신 경로를 분리해 suffix 흔들림과 digit 애니메이션 간섭을 줄인 점
- 길이 증가 케이스인
0 ms -> 306.26 ms,9 ms -> 10 ms를 단위 테스트로 고정한 점 - 변경이 없는 suffix를
needsUpdate에서 제외해 불필요한 상태 갱신을 피한 점
💡 권장 개선 사항
- 현재 테스트는
TextColumn값 변화 중심이라 실제 SwiftUI transaction에서 suffix가 재삽입될 때 시각적으로 깜빡이지 않는지는 직접 검증하지 못합니다. 후속으로 가능하다면 대표 formatter 케이스의 뷰 렌더링 또는 스냅샷성 테스트를 보강하면 회귀 방지력이 더 좋아집니다.
리뷰 모델: gpt-5.5
![claudecode-pr-review-bot[bot]](https://avatars.githubusercontent.com/in/3920205?s=60&v=4){kind=small}
There was a problem hiding this comment.
suffix 애니메이션 안정화를 위해 resizeForAnimation 로직을 오른쪽 기준 digit 보존 방식으로 재작성하고, settingAnimationRange에서 non-digit 즉시 적용과 digit 애니메이션 적용을 분리한 변경입니다. 핵심 접근 방식이 합리적이며, 변경이 없는 suffix에 대한 불필요한 상태 업데이트 제거와 함께 회귀 테스트도 추가되어 있습니다. 차단할 만한 버그나 위험 요소는 발견되지 않았으며, 몇 가지 소소한 개선 제안만 남깁니다.
좋은 점
- digit 컬럼을 오른쪽 기준으로 보존하여 기존 UUID를 유지하는 방식이 SwiftUI ForEach 식별자 안정성 관점에서 적절합니다.
- settingAnimationRange에서 non-digit 즉시 적용과 digit 애니메이션 적용을 두 단계로 분리하여, withAnimation 블록에 불필요한 non-digit 변경이 포함되지 않도록 한 설계가 명확합니다.
- needsUpdate(to:index:) 메서드를 도입하여 변경이 없는 suffix에 대한 불필요한 transaction을 방지한 점이 좋습니다.
- '0 ms → 306.26 ms', '9 ms → 10 ms' 등 길이 변경 시나리오에 대한 회귀 테스트가 구체적으로 추가되어 있습니다.
💡 권장 개선 사항
- 문자열이 줄어드는 시나리오(예: '306.26 ms' → '0 ms')에 대한 회귀 테스트도 추가해 주시면, resizeForAnimation의 역방향 축소 경로까지 검증할 수 있어 안전합니다.
기술 단위 코멘트 3건은 각 라인에 별도 표시됩니다.
리뷰 모델: Claude Opus 4.6 (Thinking) #2
| mutating func resizeForAnimation(to string: String) { | ||
| let extra = string.count - count | ||
| guard extra != 0 else { return } | ||
| let targetCharacters = string.map { $0 } |
There was a problem hiding this comment.
[Suggestion] string.map { $0 }는 Array(string)과 동일한 결과를 만듭니다. 같은 파일의 AnimateNumberText.swift 201번 줄에서는 이미 Array(string)을 사용하고 있으므로, 일관성을 위해 Array(string)으로 통일해 주시면 좋겠습니다.
There was a problem hiding this comment.
수정 완료: extension Array where Element == TextColumn 내부에서는 Array(string)이 [TextColumn]로 해석되어 컴파일 오류가 나므로 Swift.Array<Character>(string)로 명시했습니다. 의도는 동일하게 문자열을 [Character]로 변환하는 것입니다. 검증: swift test.
| targetColumns[targetIndex] = preservedColumn | ||
| } | ||
|
|
||
| for (index, character) in targetCharacters.enumerated() where !TextType(character).isNumber { |
There was a problem hiding this comment.
[Minor] where !TextType(character).isNumber에서 TextType(character)를 생성한 뒤, 바로 다음 줄(67)에서 let targetValue = TextType(character)로 동일한 인스턴스를 다시 생성하고 있습니다. 문자열 길이가 짧아 성능 영향은 미미하지만, let targetValue = TextType(character) 선언을 루프 본문 맨 위로 올리고 where 절 대신 guard !targetValue.isNumber else { continue }로 변경하시면 중복 생성을 제거할 수 있습니다.
There was a problem hiding this comment.
수정 완료: 지적대로 where !TextType(character).isNumber 중복 생성을 없애고, 루프 안에서 targetValue를 한 번만 생성한 뒤 guard로 분기하도록 정리했습니다. 검증: swift test.
| let extra = string.count - count | ||
| guard extra != 0 else { return } | ||
| let targetCharacters = string.map { $0 } | ||
| guard targetCharacters.count != count else { return } |
There was a problem hiding this comment.
[Suggestion] 현재 guard는 문자 수가 같으면 즉시 반환하므로, 같은 길이에서 digit/non-digit 패턴이 달라지는 경우(예: 소수점 위치 이동) 컬럼 재배치가 생략됩니다. 실제 숫자 포맷팅에서 이런 상황은 극히 드물고, 값 업데이트는 settingAnimationRange에서 처리되므로 당장 문제가 되지는 않지만, 향후 edge case를 고려하여 이 가정을 주석으로 남겨 두시면 유지보수에 도움이 될 수 있습니다.
There was a problem hiding this comment.
수정 완료: 같은 길이 업데이트는 컬럼 재배치 없이 settingAnimationRange에서 값/type 변경을 처리한다는 의도를 주석으로 남겼습니다. 이 PR의 깜빡임 원인인 길이 증감 경로는 별도 테스트로 고정했습니다. 검증: swift test.
There was a problem hiding this comment.
동의합니다. 문자 개수가 같더라도 소수점의 위치가 바뀌거나 숫자와 문자의 배열 패턴이 달라지는 특수한 환경에서는 각 칸의 속성이 맞지 않아 화면이 잘못 그려질 수 있습니다. 지금 당장은 발생하기 어려운 상황이더라도, 이 동작 방식에 대한 명확한 주석을 남기거나 추후 문자 패턴의 일치 여부까지 검사하도록 보완하는 것이 유지보수에 도움이 됩니다.
리뷰 지적을 반영해 TextType의 digit 판별 범위를 ASCII 0-9로 좁히고, formatted suffix 축소 경로를 테스트했습니다. - Character.wholeNumberValue가 로마 숫자나 아라비아-인도 숫자 등을 number로 오분류하지 않도록 ASCII digit만 animated number로 처리합니다. - placeholder 생성도 동일한 ASCII digit 기준을 사용합니다. - resizeForAnimation에서 TextType 중복 생성을 제거했습니다. - 같은 길이 업데이트는 settingAnimationRange가 처리한다는 의도를 주석으로 남겼습니다. - 306.26 ms -> 0 ms 축소 케이스와 유니코드 숫자 폴백 테스트를 추가했습니다. 검증: - swift test - DOCS_OUTPUT_PATH=.build/docc-site DOCC_ARCHIVE_PATH=.build/AnimateNumberText.doccarchive ./GeneratingDocumentationSite - git diff --check
There was a problem hiding this comment.
변경사항은 suffix가 포함된 포맷 문자열에서 숫자 컬럼과 고정 문자열 컬럼을 분리해 처리하는 방향으로 잘 정리되어 있습니다. 이전 라운드에서 제기된 유니코드 숫자 오분류, 축소 케이스 테스트, 중복 생성 지적도 최신 코드에서 대부분 반영되어 있습니다. 전체 코드 흐름을 확인했지만 병합을 막을 만한 버그, 예외 처리 누락, 데이터 손실 위험은 발견하지 못했습니다.
좋은 점
- ASCII 숫자만 애니메이션 대상으로 제한해 유니코드 숫자 오프셋 이탈 가능성을 제거한 점
- 길이 증가와 축소 모두 테스트해 오른쪽 기준 digit 보존 동작을 회귀 테스트로 고정한 점
- non-digit 즉시 반영과 digit 애니메이션 반영을 분리해 suffix 깜빡임과 timing curve 간섭을 줄인 점
💡 권장 개선 사항
- 현재 테스트는 상태 배열 단위 검증이 중심이므로, 후속으로 가능하다면 SwiftUI 렌더링 또는 스냅샷성 테스트를 추가해 실제 suffix 깜빡임 회귀까지 검증하면 더 안전합니다.
리뷰 모델: gpt-5.5
There was a problem hiding this comment.
이번 PR은 숫자 애니메이션 중 접미사가 깜빡이거나 애니메이션 효과가 제대로 표현되지 않던 레이아웃 문제를 효과적으로 해결했습니다. 비숫자 문자의 상태 변경을 비동기 처리 이전에 즉시 반영하고, 아라비아 숫자만을 안전하게 분류하여 오프셋 슬라이딩 애니메이션을 적용하는 구조적 개선이 돋보입니다. 비동기 작업 취소 시 발생할 수 있는 일시적인 상태 불일치 가능성을 조금 더 보완한다면 더욱 안정적인 라이브러리가 될 것입니다.
좋은 점
- Character 확장을 통해 asciiDigitValue를 구현하여 아라비아 숫자(0~9)만 TextType.number로 정확히 분류하게 함으로써, 특수한 유니코드 숫자 문자가 유입될 때 뷰의 레이아웃이 깨지거나 오프셋 범위를 벗어나는 예외 상황을 안전하게 방지했습니다.
- resizeForAnimation 메서드에서 기존의 숫자 컬럼(TextColumn)들을 우측 정렬 기준으로 매칭하여 재사용하도록 구현하여, SwiftUI가 불필요하게 뷰 노드를 삭제하고 재생성하는 비용을 줄이고 매끄러운 슬라이딩 애니메이션을 제공하도록 설계했습니다.
- settingAnimationRange 메서드 내에서 변경 사항이 없는 비숫자(non-digit) 문자는 업데이트 대상에서 제외하고 변경된 부분만 상태를 적용하여 불필요한 화면 흔들림을 억제했습니다.
개선할 점
- 현재 resizeForAnimation의 변경 방식은 주로 문자열의 길이 차이에 의존하고 있습니다. 문자열의 전체 길이는 같더라도 소수점의 위치가 이동하는 등 내부 구조 패턴이 바뀌는 드문 케이스에서는 재배치 로직이 누락될 수 있으므로, 향후 내부 배열 구조의 변화까지 감지하여 정합성을 맞추는 보완 작업을 제안합니다.
(주: 다음 1개 코멘트는 PR diff 범위 밖이라 본문에 모았습니다.)
드롭된 라인 지적
Sources/AnimateNumberText/Public/AnimateNumberText.swift:130— [Minor] Task.sleep이 취소되어 예외가 발생할 때, resizeAnimationRange를 통해 animationRange는 이미 새로운 크기로 변경되었으나 displayedString은 업데이트되지 않아 두 상태 간의 정합성이 일시적으로 깨질 수 있습니다. 최종적으로는 마지막 태스크가 완수되면서 정상 수렴하겠지만, 취소 발생 시의 일관성을 확실히 보장하기 위해 catch 블록 내에서 displayedString의 값을 현재 중간 상태에 맞게 동기화하거나 취소 대응을 명확히 하는 설계가 권장됩니다.
리뷰 생성 모델: Gemini 3.5 Flash (High) #1
리뷰 코멘트 추가 처리 결과수정 완료:
수정 완료: 문자열 축소 경로 테스트
수정 완료: 중복
검증:
|
There was a problem hiding this comment.
MLX 리뷰
이 PR은 TextType의 문자 처리 로직을 ASCII 숫자 값으로 업데이트하고, TextColumn의 resizeForAnimation 메서드를 재구현하여 숫자 컬럼의 안정적인 유지와 형식화된 접미사의 정렬을 보장합니다. 테스트 케이스를 추가하여 비-ASCII 숫자 및 형식화된 텍스트의 동작을 검증합니다.
라인 단위 코멘트
- 라인 단위로 남길 개선 사항은 발견되지 않았습니다.
사용된 모델: mlx-community/Qwen3-30B-A3B-Instruct-2507-8bit
stringFormatter suffix가 placeholder 상태로 보이는 시간을 줄이기 위해 기본 resizeDelay를 0으로 변경했습니다. - 기본 설정에서는 resize 후 즉시 digit rolling을 시작합니다. - 명시적으로 resizeDelay를 넘긴 경우에만 Task.sleep을 수행합니다. - 기본 animation configuration 테스트를 갱신했습니다. 검증: - swift test - DOCS_OUTPUT_PATH=.build/docc-site DOCC_ARCHIVE_PATH=.build/AnimateNumberText.doccarchive ./GeneratingDocumentationSite - git diff --check
추가 리뷰 코멘트 처리 결과수정 완료:
검증:
|
There was a problem hiding this comment.
숫자 애니메이션 중 suffix 깜빡임과 느려지는 옵션이 미반영되던 문제를 적절히 해결한 PR입니다. 자릿수 변경 시 우측 정렬 기준으로 digit 컬럼을 보존하고, non-digit 문자의 비동기 갱신 시점을 명확히 분리하여 전반적인 렌더링 성능과 애니메이션의 시각적 안정성을 크게 개선했습니다.
좋은 점
- Character 확장을 통해 asciiDigitValue를 구현함으로써 ASCII 아라비아 숫자만 정밀하게 분류하여 특수 유니코드 숫자 유입 시 발생할 수 있는 레이아웃 오동작을 예방했습니다.
- resizeForAnimation(to:) 메서드에서 기존의 숫자 컬럼(TextColumn)들을 우측 정렬 기준으로 매칭하여 재사용하도록 설계하여, 자릿수 변화 시 SwiftUI 뷰 노드의 파괴와 재생성 비용을 최소화하고 부드러운 스크롤 효과를 유지했습니다.
- settingAnimationRange 메서드 내에서 변경이 없는 비숫자(non-digit) 문자는 업데이트에서 조기 제외하고 실제 변경된 숫자 컬럼만 애니메이션 트랜잭션으로 적용해 화면 흔들림과 트랜잭션 간섭을 억제했습니다.
개선할 점
- resizeForAnimation 내에서 비숫자(non-digit) 문자를 보존하는 로직이 단순 인덱스 매칭을 사용하고 있어, 전체 문자열의 길이가 달라질 때(예: '0 ms' -> '306.26 ms') suffix인 ' ms'의 인덱스가 밀려 매칭에 실패합니다. 이로 인해 동일한 suffix 문자가 새로운 UUID를 가진 TextColumn으로 대체되면서 SwiftUI 뷰가 불필요하게 재생성될 수 있으므로, 우측 기준 또는 상대적 위치 기반 매칭 방식으로 보완하는 것을 제안합니다.
기술 단위 코멘트 3건은 각 라인에 별도 표시됩니다.
리뷰 생성 모델: Gemini 3.5 Flash (High) #1
| let targetValue = TextType(character) | ||
| guard !targetValue.isNumber else { continue } | ||
|
|
||
| if currentColumns.indices.contains(index), |
There was a problem hiding this comment.
[Minor] 현재 non-digit 문자를 보존하기 위해 currentColumns.indices.contains(index) 조건과 값 일치를 검증하고 있으나, 자릿수가 증가하여 전체 문자열의 길이가 달라지는 경우 suffix의 인덱스 위치가 달라져 매칭이 되지 않고 새로운 TextColumn(새 UUID)이 생성됩니다. digit 컬럼처럼 우측 정렬 기준으로 매칭하거나 별도 역방향 루프를 통해 매칭하여 SwiftUI가 뷰 노드를 보존할 수 있도록 구조 개선을 권장합니다.
There was a problem hiding this comment.
최신 HEAD의 resizeForAnimation은 더 이상 단순 인덱스 매칭으로 non-digit을 보존하지 않고, preservedTextColumns에서 같은 TextType을 역방향으로 찾아 재사용합니다. 추가된 테스트도 0 ms → 306.26 ms, 9 ms → 10 ms, 306.26 ms → 0 ms에서 suffix ID가 유지되는 것을 확인하므로 이 지적은 현재 코드에는 맞지 않는 것으로 보입니다.
| let resizeDelayNanoseconds = animation.resizeDelayNanoseconds | ||
| if resizeDelayNanoseconds > 0 { | ||
| do { | ||
| try await Task.sleep(nanoseconds: resizeDelayNanoseconds) |
There was a problem hiding this comment.
[Major] Task.sleep이 실행되는 도중 .task(id:) 트리거 등으로 인해 작업 취소(CancellationError)가 발생하면, resizeAnimationRange를 통해 animationRange의 자릿수만 변경된 상태에서 displayedString 및 최종 값 업데이트(settingAnimationRange)를 거치지 않고 조기 반환됩니다. 이로 인해 일시적으로 placeholder 상태(0)의 레이아웃이 화면에 노출될 수 있으므로, catch 블록에서 복구 로직을 명시하거나 displayedString과의 일관성을 유지하도록 처리를 보완하는 것이 안전합니다.
| withoutAnimation { | ||
| animationRange.set(value, index: index) | ||
| } | ||
| withAnimation(animation.digitAnimation(at: index)) { |
There was a problem hiding this comment.
[Suggestion] animation.digitAnimation(at: index)를 적용할 때 넘겨주는 index는 문자열 전체 기준의 인덱스입니다. 만약 통화 기호나 공백 등의 prefix(예: '$ 120.0')가 있는 경우, 실제 첫 번째 숫자의 인덱스가 0이 아닌 2부터 시작하여 stagger 애니메이션의 spring timing fraction 계산이 다르게 적용됩니다. 전체 인덱스 대신 실제 숫자 컬럼(digit)들 중에서 몇 번째 숫자 컬럼인지를 나타내는 인덱스를 별도로 계산하여 사용하면 일관된 stagger 딜레이 애니메이션 효과를 구현할 수 있습니다.
There was a problem hiding this comment.
suffix가 포함된 포맷 문자열에서 숫자 컬럼과 고정 문자열 컬럼을 분리해 처리하는 변경이 목적에 맞게 구현되어 있습니다. 이전 라운드에서 나온 유니코드 숫자 오분류, 중복 생성, 축소 경로 테스트 지적도 최신 코드 기준으로 반영되어 있습니다. 병합을 막을 만한 버그, 예외 처리 누락, 상태 불일치 위험은 발견하지 못했습니다.
좋은 점
- ASCII 숫자만 애니메이션 대상으로 제한해 유니코드 숫자의 잘못된 오프셋 적용 가능성을 제거한 점
- 길이 증가와 축소 케이스를 모두 테스트해 suffix 정렬 회귀를 고정한 점
- non-digit 즉시 반영과 digit 애니메이션 반영을 분리해 suffix 깜빡임과 타이밍 곡선 간섭을 줄인 점
💡 권장 개선 사항
- 후속으로 가능하다면 SwiftUI 렌더링 또는 스냅샷성 테스트를 추가해 실제 화면에서 suffix 깜빡임이 재발하지 않는지 검증하면 회귀 방지력이 더 좋아집니다.
리뷰 모델: gpt-5.5
There was a problem hiding this comment.
MLX 리뷰
이 PR은 TextType과 TextColumn의 숫자 처리 범위를 ASCII '0'~'9'로 제한하고, 문자열 길이 변화 시 컬럼 재배치 로직을 개선하여 형식화된 텍스트(예: "306.26 ms")의 애니메이션을 안정적으로 구현합니다. 또한, 중복 TextType 생성을 제거하고, 같은 길이 업데이트 시 컬럼 재배치를 생략하는 의도를 명확히 문서화했습니다.
라인 단위 코멘트
- 라인 단위로 남길 개선 사항은 발견되지 않았습니다.
사용된 모델: mlx-community/Qwen3-30B-A3B-Instruct-2507-8bit
리뷰 지적을 반영해 문자열 길이가 바뀌어도 동일한 suffix 컬럼 UUID가 재사용되도록 보강했습니다. - non-digit 컬럼도 오른쪽 기준으로 같은 문자 컬럼을 보존합니다. - 0 ms -> 306.26 ms, 9 ms -> 10 ms, 306.26 ms -> 0 ms 테스트에서 suffix UUID 보존을 검증합니다. 검증: - swift test - DOCS_OUTPUT_PATH=.build/docc-site DOCC_ARCHIVE_PATH=.build/AnimateNumberText.doccarchive ./GeneratingDocumentationSite - git diff --check
추가 리뷰 코멘트 처리 결과수정 완료: 문자열 길이 변경 시 suffix 컬럼 재사용
검증:
|
There was a problem hiding this comment.
접미사(suffix)가 포함된 숫자 애니메이션의 시각적 흔들림과 깜빡임 현상을 방지하기 위해 텍스트 컬럼의 재배치 및 업데이트 로직을 안정화했습니다. 아라비아 숫자 판별 방식을 ASCII 기준으로 제한하고, UUID 보존을 통해 SwiftUI의 불필요한 뷰 재생성 비용을 성공적으로 억제했습니다.
좋은 점
- resizeForAnimation(to:) 메서드에서 preservedTextColumns 내 동일 문자(non-digit)의 UUID를 역순으로 매칭 및 재사용하도록 구현하여 SwiftUI가 뷰 노드를 파괴하고 새로 그리는 비용을 최소화하고 레이아웃 흔들림을 개선한 점
- digitColumn(for:) 구현부에서 hidden 처리된 ZStack 내부 템플릿으로 가변 폭 폰트의 최대 치수를 먼저 확보하고 overlay 내 GeometryReader를 배치하여 부모 뷰가 무한히 커지는 현상을 방지하고 정밀한 오프셋 렌더링을 구현한 점
개선할 점
- AnimateNumberText 뷰 내부에서 직접 제어하는 Task.sleep 지연 및 animationRange 리사이징 상태 전이 흐름을 별도의 State Holder 또는 ViewModel(예: @observable 객체)로 격리하여 뷰 선언부와 비동기 비즈니스 로직을 분리하는 구조적 리팩터링을 제안합니다.
- stagger 효과(spring timing fraction) 계산 시 적용되는 인덱스가 문자열 전체 인덱스이기 때문에 prefix가 존재할 경우 타이밍이 밀리는 현상이 있습니다. digitAnimation(at:) 호출 시 전체 인덱스가 아닌 실제 animated number 컬럼들 간의 상대적 순서 인덱스를 주입하도록 로직을 정밀화할 것을 제안합니다.
기술 단위 코멘트 2건은 각 라인에 별도 표시됩니다.
리뷰 생성 모델: Gemini 3.5 Flash (High) #1
| @@ -67,6 +85,12 @@ extension Array where Element == TextColumn { | |||
| return self[index].value.isNumber && TextType(value).isNumber | |||
There was a problem hiding this comment.
[Suggestion] TextType(value).isNumber를 판정하기 위해 매번 TextType 인스턴스를 초기화하고 있습니다. value.asciiDigitValue != nil을 직접 평가하여 불필요한 인스턴스 생성을 피하는 편이 더 효율적입니다.
| func needsUpdate(to value: Character, index: Int) -> Bool { | ||
| guard indices.contains(index) else { return false } | ||
|
|
||
| return self[index].value != TextType(value) |
There was a problem hiding this comment.
[Suggestion] self[index].value != TextType(value) 비교 시에도 매번 Character를 기반으로 TextType 객체를 생성합니다. TextType 내부에 Character와의 동등성 비교 연산자(==)를 오버로딩하거나 패턴 매칭을 활용해 객체 생성을 생략할 수 있습니다.
There was a problem hiding this comment.
이번 변경은 suffix가 포함된 포맷 문자열에서 숫자 컬럼과 고정 문자열 컬럼을 분리해 처리하는 방향이 목적에 맞게 구현되어 있습니다. ASCII 숫자만 애니메이션 대상으로 제한했고, 길이 증가·축소 경로와 suffix 식별자 보존을 테스트로 고정한 점도 좋습니다. 최신 코드 기준으로 병합을 막을 만한 버그, 예외 처리 누락, 상태 불일치 위험은 발견하지 못했습니다.
좋은 점
- non-digit 즉시 반영과 digit 애니메이션 반영을 분리해 suffix 깜빡임과 타이밍 곡선 간섭을 줄인 점
- ASCII 숫자만 TextType.number로 분류해 유니코드 숫자 오프셋 오류 가능성을 제거한 점
- 문자열 증가와 축소 모두에서 suffix 컬럼 ID 보존을 테스트로 검증한 점
💡 권장 개선 사항
- 후속으로 가능하다면 SwiftUI 렌더링 또는 스냅샷성 테스트를 추가해 실제 화면에서 suffix 깜빡임이 재발하지 않는지 검증하면 회귀 방지력이 더 좋아집니다.
리뷰 모델: gpt-5.5
There was a problem hiding this comment.
MLX 리뷰
이 PR은 숫자 애니메이션 뷰의 숫자 인식 범위를 ASCII '0'~'9'로 제한하고, 자릿수 변경 시 형식 문자열(예: 'ms')의 위치를 유지하는 로직을 개선하며, resize 지연 시간 처리를 안정화했습니다. 기존의 비정상적인 유니코드 숫자 인식 문제와 resize 지연 중 취소 시 임시 상태 노출 문제를 해결했습니다.
반드시 수정할 사항
- Task.sleep 중 취소 시 animationRange의 크기만 변경되고, displayedString과 최종 값 업데이트가 수행되지 않아 placeholder 상태(0)의 레이아웃이 일시적으로 화면에 노출될 수 있습니다.
권장 개선사항
- non-digit 문자의 보존을 위해 현재는 역방향 루프로 매칭을 시도하지만, 자릿수가 증가할 때 suffix의 인덱스 위치가 달라져 기존의 TextColumn(새 UUID)이 생성되며 SwiftUI 뷰 노드 보존이 실패할 수 있습니다.
라인 단위 코멘트
- 자동 리뷰에서 2개의 라인 단위 개선 사항을 남겼습니다.
사용된 모델: mlx-community/Qwen3-30B-A3B-Instruct-2507-8bit
| let resizeDelayNanoseconds = animation.resizeDelayNanoseconds | ||
| if resizeDelayNanoseconds > 0 { | ||
| do { | ||
| try await Task.sleep(nanoseconds: resizeDelayNanoseconds) |
There was a problem hiding this comment.
[Major] Problem: Task.sleep 중 취소 시 animationRange의 크기만 변경되고, displayedString과 최종 값 업데이트가 수행되지 않아 placeholder 상태(0)의 레이아웃이 일시적으로 화면에 노출될 수 있습니다. Why it matters: 이는 사용자에게 불필요한 깜빡임이나 잘못된 숫자가 잠시 보이는 UX 문제를 유발할 수 있으며, 애니메이션의 일관성과 신뢰성을 해칩니다. Suggested fix: catch 블록에서 displayedString을 원래 값으로 복구하거나, resizeAnimationRange의 결과를 무시하지 않고 최종 상태를 일관되게 유지하도록 처리해야 합니다. Confidence: High
Confidence score: 0.95
There was a problem hiding this comment.
실제 동작에서 .task(id: stringValue)는 값이 변경될 때 기존 Task를 취소하고 새 Task를 즉시 시작합니다. 새 Task의 scheduleAnimationUpdateIfNeeded가 resizeAnimationRange를 다시 호출하므로, 취소된 Task가 남긴 중간 상태(리사이즈만 완료)는 즉시 덮어씌워집니다. MainActor 직렬 실행이므로 중간 상태가 렌더링 프레임에 노출될 가능성은 극히 낮습니다. 또한 기본 resizeDelay가 0으로 변경되어 일반 사용에서는 이 경로에 진입하지 않으므로, catch 블록에 의도 주석을 추가하는 정도면 충분하다고 판단합니다.
| var targetColumns = targetCharacters.map { TextColumn(placeholderFor: $0) } | ||
|
|
||
| for targetIndex in targetCharacters.indices.reversed() { | ||
| guard targetColumns[targetIndex].value.isNumber else { continue } |
There was a problem hiding this comment.
[Minor] Problem: non-digit 문자의 보존을 위해 현재는 역방향 루프로 매칭을 시도하지만, 자릿수가 증가할 때 suffix의 인덱스 위치가 달라져 기존의 TextColumn(새 UUID)이 생성되며 SwiftUI 뷰 노드 보존이 실패할 수 있습니다. Why it matters: 이로 인해 형식 문자열(예: 'ms')의 위치가 일관되지 않게 보일 수 있으며, 애니메이션 중 뷰가 재생성되어 깜빡임이 발생할 수 있습니다. Suggested fix: digit 컬럼처럼 우측 정렬 기준으로 매칭하거나, suffix의 위치를 고정된 인덱스 기준으로 매칭하는 방식으로 구조를 개선해야 합니다. Confidence: Medium
Confidence score: 0.85
There was a problem hiding this comment.
보류 — 최신 코드의 resizeForAnimation(to:) 메서드에서는 preservedTextColumns에서 역순(lastIndex(where:))으로 동일 문자를 검색하여 매칭하고 제거하는 방식으로 개선되었습니다. 이를 통해 자릿수가 증가하여 suffix의 상대적 인덱스 위치가 밀리더라도 기존 suffix 문자 컬럼들의 UUID가 안정적으로 유지되며, 관련 테스트(resizeForAnimationUsesStablePlaceholders 등)가 모두 통과함을 확인했습니다.
There was a problem hiding this comment.
최신 코드의 resizeForAnimation은 preservedTextColumns.lastIndex(where:)를 역순 target 루프 안에서 사용하므로 suffix의 같은 문자 컬럼을 오른쪽부터 재사용합니다. 추가 테스트도 길이 증가와 축소에서 suffix ID 보존을 확인하고 있어, 이 지적은 현재 HEAD 기준으로는 재현되지 않는 환각에 가깝습니다.
There was a problem hiding this comment.
최신 코드의 resizeForAnimation(to:)는 preservedTextColumns에서 lastIndex(where: { $0.value == targetValue })로 값 기반 매칭을 수행하므로, 자릿수가 증가하여 suffix 인덱스가 밀리더라도 동일 값의 기존 TextColumn UUID가 정확히 보존됩니다. 추가된 테스트(resizeForAnimationUsesStablePlaceholders, KeepsFormattedSuffixAligned, WhenShrinking)에서도 suffix(3).map(.id)가 resize 전후로 일치하는 것이 검증되어 있으므로, 이 지적은 현재 코드에는 해당하지 않는 것으로 보입니다.
There was a problem hiding this comment.
suffix 포함 숫자 애니메이션의 시각적 안정성을 개선하기 위해 컬럼 재배치 로직을 재설계하고, ASCII 숫자 판별을 도입하며, 즉시/애니메이션 업데이트를 분리한 변경입니다. 이전 리뷰 라운드에서 제기된 유니코드 숫자 오분류, suffix 인덱스 매칭 실패, Task.sleep 기본값 문제가 모두 최신 코드에 반영되어 있습니다. 병합을 차단할 만한 버그, 예외 처리 누락, 상태 불일치 위험은 발견하지 못했습니다.
좋은 점
- resizeForAnimation(to:)에서 digit 컬럼은 우측 정렬 popLast로, non-digit 컬럼은 값 기반 역순 매칭으로 UUID를 보존하여 SwiftUI ForEach의 뷰 재생성을 최소화한 설계가 명확합니다.
- settingAnimationRange에서 변경이 필요한 non-digit을 withoutAnimation으로 먼저 반영하고, 변경된 digit만 각각의 withAnimation으로 처리하여 suffix 깜빡임과 타이밍 곡선 간섭을 구조적으로 분리한 점이 좋습니다.
- asciiDigitValue를 unicodeScalars 기반으로 직접 구현하여 wholeNumberValue의 유니코드 숫자 오분류를 원천 차단하고, 아라비아-인도 숫자(٣)와 로마 숫자(Ⅻ) 테스트로 검증한 점이 좋습니다.
- 문자열 증가(0 ms → 306.26 ms), 1자리 증가(9 ms → 10 ms), 축소(306.26 ms → 0 ms) 세 경로 모두에서 suffix 컬럼 ID 보존을 테스트로 고정하여 회귀 방지력을 확보한 점이 좋습니다.
💡 권장 개선 사항
- resizeDelay > 0일 때 Task.sleep 취소 시 animationRange만 리사이즈된 채 반환되는 경로가 남아 있습니다. .task(id:)의 취소-재시작 시맨틱 덕분에 실질적 문제 가능성은 낮지만, 해당 경로의 의도(새 Task가 즉시 덮어쓴다)를 catch 블록 부근에 주석으로 명시해 두시면 향후 유지보수에 도움이 됩니다.
- digitAnimation(at:)에 전달되는 index가 문자열 전체 기준이므로, 통화 기호나 공백 등의 prefix가 있을 때 stagger 타이밍 fraction이 의도보다 높게 시작됩니다. 후속 PR에서 digit 전용 순서 인덱스를 계산하여 전달하면 prefix 유무와 관계없이 일관된 stagger 효과를 얻을 수 있습니다.
기술 단위 코멘트 1건은 각 라인에 별도 표시됩니다.
리뷰 모델: Claude Opus 4.6 (Thinking) #2
| let targetValue = TextType(targetCharacters[targetIndex]) | ||
| guard !targetValue.isNumber else { continue } | ||
|
|
||
| if let currentIndex = preservedTextColumns.lastIndex(where: { $0.value == targetValue }) { |
There was a problem hiding this comment.
[Suggestion] preservedTextColumns에서 동일 값의 non-digit 문자가 여러 개일 때(예: 천 단위 쉼표 "," 다수), lastIndex(where:)와 역순 순회 조합이 우측 정렬 매칭을 자연스럽게 수행합니다. 현재 동작은 정확하지만, digit 컬럼의 popLast 기반 우측 정렬과 달리 non-digit 매칭의 우측 정렬 의도가 코드만으로는 파악하기 어렵습니다. 간단한 인라인 주석(예: "// Right-to-left: reverse iteration + lastIndex preserves trailing duplicates first")을 추가해 주시면 유지보수에 도움이 됩니다.
There was a problem hiding this comment.
동의 — digit 컬럼이 popLast()를 활용하여 우측 정렬 매칭을 직관적으로 수행하는 것과 달리, non-digit 컬럼은 역순 루프와 lastIndex(where:)를 사용하여 동일한 우측 정렬 효과를 내고 있습니다. 제안해주신 대로 뒤쪽의 중복 문자부터 매칭하여 보존한다는 의도를 나타내는 설명 주석을 코드에 명시하면 향후 유지보수 시 가독성을 높이는 데 큰 도움이 될 것입니다.
외부에 노출되는 AnimateNumberTextAnimation 옵션을 smooth(duration:) 하나로 정리했습니다. - linear, easeIn, easeOut, easeInOut, interactiveSpring factory를 제거했습니다. - public DigitTiming, digitTiming, resizeDuration, resizeDelay 설정을 제거했습니다. - 기본 animation 파라미터를 .smooth()로 변경했습니다. - resize animation/delay 경로를 제거하고 length resize는 비애니메이션으로 즉시 처리합니다. - README와 DocC 예제를 smooth(duration:) 기준으로 정리했습니다. - animation 테스트를 smooth 설정 검증으로 단순화했습니다. 검증: - swift test - DOCS_OUTPUT_PATH=.build/docc-site DOCC_ARCHIVE_PATH=.build/AnimateNumberText.doccarchive ./GeneratingDocumentationSite - clean DocC 생성 후 제거된 symbol 페이지 검색 - git diff --check
추가 변경 사항수정 완료: animation API를
검증:
|
There was a problem hiding this comment.
접미사를 포함한 숫자 애니메이션의 시각적 안정성과 깜빡임 문제를 해결하기 위해 텍스트 컬럼의 재배치 및 업데이트 로직을 안정화한 작업입니다. ASCII 숫자 판별을 도입하여 유니코드 숫자 오분류 가능성을 차단하고, 문자열 길이 변경 시에도 기존 컬럼의 식별자를 역순으로 추적해 재사용하도록 하여 SwiftUI 뷰의 불필요한 재생성을 방지했습니다. 핵심적인 레이아웃 및 타이밍 간섭 문제가 해소되어 코드베이스가 한층 더 견고해졌습니다.
좋은 점
- TextType의 Character 확장으로 asciiDigitValue 연산 프로퍼티를 구현하여 기존의 wholeNumberValue 사용 시 발생하던 로마 숫자나 아라비아-인도 숫자 등의 유니코드 숫자 오분류 및 레이아웃 오프셋 오류 가능성을 차단한 점
- resizeForAnimation(to:) 메서드에서 digit 컬럼은 popLast()를 통한 우측 정렬로 매칭하고, non-digit 컬럼은 역방향 순회를 통해 동일 문자의 식별자를 보존하여 SwiftUI ForEach의 뷰 재생성 비용과 레이아웃 흔들림을 최소화한 점
개선할 점
- 현재 digitAnimation(at:)에 전달되는 인덱스가 문자열 전체 기준이므로, 통화 기호나 공백 등 prefix가 존재하는 경우 stagger 애니메이션의 spring 타이밍이 일관되지 않게 시작될 수 있습니다. 전체 인덱스가 아닌 실제 애니메이션 대상인 digit 컬럼들만의 상대적인 순서 인덱스를 전달하도록 구조를 개선하는 것이 좋습니다.
리뷰 생성 모델: Gemini 3.5 Flash (High) #1
There was a problem hiding this comment.
suffix가 포함된 포맷 문자열에서 숫자 컬럼과 고정 문자열 컬럼을 분리하고, 길이 변경 시 suffix 식별자를 보존하도록 테스트까지 추가한 점은 변경 목적에 잘 맞습니다. 다만 공개 initializer의 animation 파라미터 타입이 SwiftUI Animation에서 전용 타입으로 좁아진 것으로 보이며, 기존 사용자가 .easeOut(duration:) 같은 SwiftUI 애니메이션을 넘기던 코드가 컴파일되지 않을 수 있습니다. 이 PR이 패치/마이너 성격이라면 병합 전 공개 API 호환성 유지 방식을 먼저 정리해야 합니다.
좋은 점
- 문자열 증가와 축소 모두에서 suffix 컬럼 ID 보존을 테스트로 고정한 점
- ASCII 숫자만 rolling digit 대상으로 분류해 유니코드 숫자의 잘못된 오프셋 적용 가능성을 줄인 점
- non-digit 즉시 반영과 digit 애니메이션 반영을 분리해 suffix 깜빡임 원인을 직접 다룬 점
🔴 반드시 수정할 사항
- 공개 API의 animation 파라미터가
AnimateNumberTextAnimation으로 좁아져 기존Animation호출부가 깨질 수 있습니다. 기존 SwiftUIAnimation을 받는 initializer를 유지하거나, 호환 가능한 오버로드를 추가한 뒤 새 전용 설정 타입은 별도 API로 확장하는 방식이 안전합니다.
💡 권장 개선 사항
- 후속으로 가능하다면 실제 SwiftUI 렌더링 또는 스냅샷성 테스트를 추가해 suffix 깜빡임 회귀를 단위 로직뿐 아니라 화면 결과에서도 검증하면 좋겠습니다.
기술 단위 코멘트 1건은 각 라인에 별도 표시됩니다.
리뷰 모델: gpt-5.5
| numberFormatter: NumberFormatter? = nil, | ||
| stringFormatter: String? = nil, | ||
| animation: AnimateNumberTextAnimation = .default | ||
| animation: AnimateNumberTextAnimation = .smooth() |
There was a problem hiding this comment.
[Major] 문제 → 공개 initializer의 animation 타입이 전용 AnimateNumberTextAnimation으로 고정되어 기존 호출부에서 animation: .easeOut(duration:) 같은 SwiftUI Animation 값을 넘기던 코드가 컴파일되지 않을 가능성이 큽니다. 영향 → README와 PR 설명도 기존 SwiftUI 애니메이션 옵션을 언급하고 있어, 이 변경은 suffix 안정화 범위를 넘어서는 source-breaking 변경이 됩니다. 제안 → 기존 Animation 기반 initializer를 유지하거나 호환 오버로드를 추가하고, 새 .smooth(duration:) 설정은 별도 initializer/팩토리로 제공해 기존 사용자가 그대로 빌드되게 해 주세요.
There was a problem hiding this comment.
보류 — 이번 PR은 버전 0.7.0으로의 업데이트이므로 유의적 버전 표기법(SemVer)에 따라 0.x 버전대에서 부버전이 상승할 때 발생하는 하위 호환성 파괴가 허용되는 시점입니다. 불필요하게 복잡했던 여러 애니메이션 관련 메서드들을 단일화한 것은 올바른 설계 방향으로 판단되어 기존 애니메이션 기반 오버로드를 유지하지 않고 진행하는 것이 합당합니다.
There was a problem hiding this comment.
MLX 리뷰
이 PR은 AnimateNumberText의 기본 애니메이션을 기존의 easeOut에서 비선형 가속-감속을 갖는 .smooth(duration:)으로 변경하고, 숫자 인식 범위를 ASCII 0-9로 제한하여 비표준 숫자 문자(예: ٣, Ⅻ)를 문자로 처리하도록 개선했습니다. 또한, 문자열 길이 변경 시 형식 문자열(예: 'ms')의 위치 일관성을 보장하기 위해 suffix 컬럼 재사용 로직을 개선하고, Task.sleep 취소 시 잠재적 상태 불일치를 방지하기 위해 기본 resizeDelay를 0으로 설정했습니다.
라인 단위 코멘트
- 라인 단위로 남길 개선 사항은 발견되지 않았습니다.
사용된 모델: mlx-community/Qwen3-30B-A3B-Instruct-2507-8bit
There was a problem hiding this comment.
suffix 포함 숫자 애니메이션의 안정성을 개선한 PR로, 이전 라운드에서 제기된 Task.sleep 취소 문제와 non-digit 인덱스 매칭 실패가 모두 해결되었습니다. resize → settingAnimationRange 흐름을 동기적으로 단순화하고, digit/non-digit 업데이트를 명확히 분리하여 suffix 깜빡임을 구조적으로 방지했습니다. 병합을 차단할 만한 버그, 예외 처리 누락, 상태 불일치 위험은 발견하지 못했습니다.
좋은 점
- Task.sleep과 resizeDelay를 제거하고 resize → settingAnimationRange 흐름을 동기적으로 단순화하여, 취소 시 중간 상태(placeholder 0) 노출 위험을 원천 제거한 점
- resizeForAnimation(to:)에서 digit 컬럼은 popLast 기반 우측 정렬로, non-digit 컬럼은 값 기반 역순 매칭(lastIndex(where:) + reversed)으로 UUID를 보존하여 SwiftUI ForEach의 뷰 재생성을 최소화한 설계
- settingAnimationRange에서 변경이 필요한 non-digit을 withoutAnimation으로 먼저 반영하고, 변경된 digit만 각각 withAnimation으로 처리하여 suffix 깜빡임과 타이밍 곡선 간섭을 구조적으로 분리한 점
- 증가(0 ms → 306.26 ms), 1자리 증가(9 ms → 10 ms), 축소(306.26 ms → 0 ms) 세 경로 모두에서 suffix 컬럼 ID 보존을 테스트로 고정하여 회귀 방지력을 확보한 점
💡 권장 개선 사항
- AnimateNumberTextAnimation의 공개 API에서 .easeOut, .linear, .easeInOut, .interactiveSpring 팩토리 메서드와 public init, DigitTiming 열거형, resizeDuration, resizeDelay 프로퍼티가 모두 제거되었습니다. 0.x 범위 내 파괴적 변경이므로 semver 상 문제는 없으나, 0.7.0 릴리스 노트에 '기존 타이밍 옵션은 .smooth(duration:)으로 대체되었습니다' 같은 마이그레이션 안내를 명시해 주시면 업그레이드 시 혼란을 줄일 수 있습니다.
리뷰 모델: Claude Opus 4.6 (Thinking) #2
각 숫자 컬럼을 Animatable 기반 ReelDigitColumn으로 렌더링하도록 추가했습니다. reel 모드는 연속 position을 보간해 0...9 트랙을 프레임마다 올바르게 표시하고, 왼쪽부터 digit ordinal 기준으로 방향을 교대합니다. 모든 숫자 컬럼은 함께 회전하며 duration과 stagger를 합산한 easeOut 타이밍으로 왼쪽부터 순차적으로 멈춥니다. 자릿수가 늘어날 때는 기존 resize placeholder와 widest digit 측정을 유지해 suffix 흔들림과 가로 늘어짐을 줄였습니다. README와 DocC 예제에 .reel(duration:revolutions:stagger:) 사용법을 추가하고 관련 Swift Testing 검증을 보강했습니다.
There was a problem hiding this comment.
suffix 안정화를 위해 digit/non-digit 컬럼 갱신을 분리하고, 문자열 길이 변경 시 기존 suffix 컬럼 식별자를 보존하도록 개선한 방향은 타당합니다. Task.sleep 기반 resize 지연 경로도 제거되어 이전 라운드의 중간 상태 노출 위험은 해소된 것으로 보입니다. 다만 공개 initializer의 animation 파라미터 타입이 기존 SwiftUI Animation 호환 형태에서 전용 타입으로 좁아진 부분은 기존 사용자 코드를 깨뜨릴 수 있어 병합 전 정책 결정 또는 호환 계층이 필요합니다.
좋은 점
- digit 컬럼은 우측 기준으로 재사용하고 non-digit 컬럼은 동일 문자 기준으로 역순 재사용해 suffix 깜빡임 원인을 직접 줄인 점
- ASCII 숫자만 애니메이션 대상으로 분류해 유니코드 숫자 문자의 잘못된 rolling 처리를 방지한 점
- 0 ms에서 306.26 ms, 9 ms에서 10 ms, 306.26 ms에서 0 ms로 바뀌는 경로를 테스트로 고정한 점
🔴 반드시 수정할 사항
- 공개 API의 animation 파라미터가 AnimateNumberTextAnimation으로 고정되어 기존에 SwiftUI Animation 값을 넘기던 호출부가 컴파일되지 않을 수 있습니다. 0.7.0에서 의도한 파괴적 변경이라면 릴리스 노트와 마이그레이션 안내를 명확히 하고, 아니라면 기존 Animation 기반 initializer 또는 호환 오버로드를 유지해야 합니다.
💡 권장 개선 사항
- 문서에는 .smooth(duration:) 중심 사용법이 정리되어 있지만, 제거된 .linear, .easeOut, .easeInOut, .interactiveSpring 계열 API에서 새 API로 옮기는 짧은 마이그레이션 예시가 있으면 기존 사용자의 업그레이드 비용을 줄일 수 있습니다.
- suffix 깜빡임은 단위 로직 테스트로 잘 고정되어 있으므로, 가능하면 후속 PR에서 실제 SwiftUI 렌더링 또는 스냅샷 성격의 회귀 테스트를 추가하면 화면 수준 안정성까지 확인할 수 있습니다.
리뷰 모델: gpt-5.5
There was a problem hiding this comment.
suffix 안정화를 위해 숫자 컬럼과 문자열 컬럼의 갱신 경로를 분리한 방향은 타당합니다. 기존에 제기된 비동기 sleep 취소 문제는 최신 코드 기준으로 더 이상 해당 경로가 없어 재현 가능성이 낮습니다. 다만 DocC 예제에서 기본 권장 API와 다른 별칭을 안내하고 있어 문서 일관성 측면의 작은 개선점이 있습니다.
좋은 점
- 숫자 컬럼은 우측 기준으로 보존하고 suffix 문자열 컬럼은 동일 문자 기준으로 재사용해 깜빡임 원인을 직접 줄인 점
- ASCII 숫자만 애니메이션 대상으로 분류해 유니코드 숫자가 잘못 롤링되는 경로를 차단한 점
- 길이 증가와 축소, suffix 정렬, reel 타이밍을 테스트로 고정한 점
💡 권장 개선 사항
- 공개 API 변경은 0.7.0 범위에서 의도된 변경으로 보이므로, 릴리스 노트에 기존 SwiftUI Animation 호출부를
.smooth(duration:)또는.reel(duration:revolutions:stagger:)로 옮기는 예시를 명확히 남기면 좋겠습니다.
기술 단위 코멘트 1건은 각 라인에 별도 표시됩니다.
리뷰 모델: gpt-5.5
| } | ||
| ``` | ||
|
|
||
| Use `.roll(_:revolutions:stagger:)` to make each digit spin through 0...9 |
There was a problem hiding this comment.
[Minor] 문제 → README와 PR 본문은 .reel(duration:revolutions:stagger:)를 사용자-facing 예제로 안내하지만, DocC는 여기서 .roll(_:revolutions:stagger:)를 먼저 소개하고 있습니다. 영향 → 두 API가 같은 동작을 하더라도 문서마다 권장 이름이 달라 사용자가 어떤 이름을 써야 하는지 헷갈릴 수 있습니다. 제안 → README와 맞춰 DocC도 .reel(duration:revolutions:stagger:)를 기본 예제로 쓰고, .roll은 별칭 또는 기존 호환 API로 짧게 언급하는 편이 좋습니다.
|
fix. roll/reel 자리별 순차 정지를 보강했습니다.\n\n원인:\n- 기존 구현은 숫자별로 다른 |
There was a problem hiding this comment.
MLX 리뷰
이 PR은 AnimateNumberText의 애니메이션 시스템을 단순화하고, 새로운 릴(reel) 스타일 애니메이션을 도입하여 숫자의 연속적인 회전 효과를 제공합니다. 기존의 다양한 SwiftUI Animation 옵션을 제거하고, .smooth(duration:)와 .reel(duration:revolutions:stagger:)로 대체하며, 자릿수 변화 시 형식 문자열의 일관성과 뷰 재생성 최소화를 개선했습니다.
반드시 수정할 사항
- 공개 initializer의
animation매개변수 타입이 기존의Animation기반 값에서AnimateNumberTextAnimation으로 고정되어, 기존 코드에서animation: .easeOut(duration:)와 같은 SwiftUI 기본 애니메이션을 사용하던 호출부가 컴파일 오류를 발생시킬 수 있습니다. scheduleAnimationUpdateIfNeeded내부에서Task.sleep(nanoseconds: animation.resizeDelayNanoseconds)를 호출하는 도중.task(id:)트리거로 인해 작업이 취소되면,animationRange의 크기만 변경되고displayedString과 최종 값 업데이트가 수행되지 않아 placeholder 상태(0)의 레이아웃이 일시적으로 화면에 노출될 수 있습니다.
라인 단위 코멘트
- 자동 리뷰에서 2개의 라인 단위 개선 사항을 남겼습니다.
사용된 모델: mlx-community/Qwen3-30B-A3B-Instruct-2507-8bit
| numberFormatter: NumberFormatter? = nil, | ||
| stringFormatter: String? = nil, | ||
| animation: AnimateNumberTextAnimation = .default | ||
| animation: AnimateNumberTextAnimation = .smooth() |
There was a problem hiding this comment.
[Major] Problem: 공개 initializer의 animation 매개변수 타입이 기존의 Animation 기반 값에서 AnimateNumberTextAnimation으로 고정되어, 기존 코드에서 animation: .easeOut(duration:)와 같은 SwiftUI 기본 애니메이션을 사용하던 호출부가 컴파일 오류를 발생시킬 수 있습니다. Why it matters: 이 변경은 기존 사용자 코드와 문서의 호환성을 깨뜨리는 소스 브레이킹 변경이며, README와 DocC 예제가 여전히 기존 애니메이션 옵션을 언급하고 있어 사용자 혼란을 유발할 수 있습니다. Suggested fix: 기존 Animation 기반 initializer를 유지하거나, 호환 오버로드를 추가하고, 새 .smooth(duration:) 설정은 별도의 팩토리 메서드나 새로운 initializer로 제공하여 기존 사용자가 그대로 빌드되도록 해야 합니다. Confidence: High
Confidence score: 0.95
There was a problem hiding this comment.
보류 — 현재 라이브러리는 SemVer 0.x 버전대에 해당하여 하위 호환성 깨짐이 허용되는 단계입니다. 또한, 각 자릿수별로 staggered 타이밍 및 개별 릴 회전 효과를 정밀하게 연출하기 위해서는 SwiftUI의 내장 Animation 타입 대신 전용 래퍼 타입(AnimateNumberTextAnimation)을 매개변수로 요구하는 설계가 장기적인 아키텍처 관점에서 타당하다고 판단됩니다.
| } catch { | ||
| return | ||
| } | ||
| resizeAnimationRange(to: stringValue) |
There was a problem hiding this comment.
[Major] Problem: scheduleAnimationUpdateIfNeeded 내부에서 Task.sleep(nanoseconds: animation.resizeDelayNanoseconds)를 호출하는 도중 .task(id:) 트리거로 인해 작업이 취소되면, animationRange의 크기만 변경되고 displayedString과 최종 값 업데이트가 수행되지 않아 placeholder 상태(0)의 레이아웃이 일시적으로 화면에 노출될 수 있습니다. Why it matters: 이는 사용자에게 불필요한 깜빡임이나 잘못된 숫자가 잠시 보이는 UX 문제를 유발하며, 애니메이션의 일관성과 신뢰성을 해칩니다. Suggested fix: catch 블록에서 displayedString을 원래 값으로 복구하거나, resizeAnimationRange의 결과를 무시하지 않고 최종 상태를 일관되게 유지하도록 처리해야 합니다. Confidence: High
Confidence score: 0.92
There was a problem hiding this comment.
보류 — 최신 구현 기준으로 scheduleAnimationUpdateIfNeeded 메서드 내부에는 Task.sleep을 활용한 지연 시간 대기 경로가 전혀 존재하지 않으며, 모든 갱신 동작이 동일한 MainActor 상에서 동기적으로 완료됩니다. 따라서 Task 취소로 인해 placeholder 상태가 화면에 일시적으로 노출될 위험은 현재 구조적으로 발생하지 않습니다.
There was a problem hiding this comment.
보류 — 최신 HEAD의 scheduleAnimationUpdateIfNeeded에는 Task.sleep이나 resizeDelayNanoseconds 대기 경로가 없습니다. resizeAnimationRange, settingAnimationRange, displayedString 갱신이 같은 MainActor 흐름에서 이어지므로 이 취소 시나리오는 현재 코드 기준 환각 지적으로 보는 것이 맞습니다.
자리별 정지 간격을 사용자가 조정할 수 있는 옵션 이름을 stagger에서 settleInterval로 변경했습니다. 라이브러리는 기본값 0.25초와 자리별 duration 계산을 책임지고, 호출자는 필요할 때 settleInterval만 조정하도록 공개 API 의미를 명확히 했습니다. README, DocC, Swift Testing 호출부를 새 이름에 맞춰 갱신했습니다.
There was a problem hiding this comment.
이번 PR은 자릿수 변경 시의 뷰 재사용 구조를 보완하여 숫자 및 suffix의 애니메이션 안정성을 성공적으로 개선하고, 새로운 릴 스타일 연출을 깔끔하게 추가했습니다. README와 DocC 문서 간의 API 표기 불일치가 일부 남아있고, 불필요한 async 키워드 등의 사소한 개선 여지가 있습니다. 전반적으로 코드 품질과 기능 완성도가 우수하여 제안 사항 반영 후 머지하기에 적절합니다.
좋은 점
- 자릿수 변경 시 digit 컬럼을 우측 정렬 기준으로 보존하는 resizeForAnimation(to:) 구현을 통해, SwiftUI ForEach 내 뷰 인스턴스의 정체성(UUID)을 유지하여 레이아웃의 흔들림과 깜빡임을 최소화한 점
- 애니메이션 대상을 ASCII 숫자(0~9)로만 제한하는 Character.asciiDigitValue를 도입하여, 아랍 문자나 로마 숫자 등 유니코드 숫자에 의해 발생할 수 있는 원치 않는 롤링 오작동을 근본적으로 방지한 점
개선할 점
- README.md(line 143 등)와 DocC(AnimateNumberText.md:161) 사이에서 예시 API 명칭의 불일치가 존재합니다. README에서는 여전히 .reel(...)을 기본 예시로 사용하고 있으나 DocC 및 AnimateNumberTextAnimation.swift 파일의 가이드에서는 .roll(...) 사용을 권장하고 있으므로, 이를 하나로 정리하고 README의 사용 예시도 권장형인 .roll(...)로 정렬하는 것이 문서의 일관성을 위해 좋습니다.
(주: 다음 1개 코멘트는 PR diff 범위 밖이라 본문에 모았습니다.)
드롭된 라인 지적
Sources/AnimateNumberText/Public/AnimateNumberText.swift:127— [Minor] scheduleAnimationUpdateIfNeeded(for:) 메서드는 내부에서 await를 통한 비동기 대기 지점(suspend point)이 없음에도 async로 선언되어 있습니다. 이로 인해 불필요한 비동기 컨텍스트 홉 오버헤드가 발생할 수 있으므로, 메서드 시그니처에서 async를 제거하고 동기 함수로 전환하여 실행 흐름을 보다 단순하게 최적화하는 것을 권장합니다.
리뷰 생성 모델: Gemini 3.5 Flash (High) #1
|
fix. 자리별 정지 간격 API 이름을 |
There was a problem hiding this comment.
suffix가 포함된 숫자 문자열에서 digit 컬럼과 non-digit 컬럼을 분리해 갱신하는 방향은 실제 깜빡임 원인을 잘 겨냥하고 있습니다. 다만 reel 전용 상태인 reelPositions가 smooth 모드 변경과 동기화되지 않아, 런타임에 애니메이션 스타일을 바꾸는 경우 표시 숫자가 실제 값과 달라질 수 있습니다. 이 상태 불일치는 사용자에게 잘못된 숫자를 보여줄 수 있으므로 머지 전 보완이 필요합니다.
좋은 점
- 숫자 컬럼은 우측 기준으로 보존하고 suffix 컬럼은 동일 문자 기준으로 재사용해 SwiftUI 뷰 정체성 손실을 줄인 점이 좋습니다.
- ASCII 숫자만 애니메이션 대상으로 분류해 유니코드 숫자에서 의도치 않은 롤링이 발생하지 않도록 한 점이 명확합니다.
0 ms -> 306.26 ms,9 ms -> 10 ms, 축소 케이스까지 테스트로 고정해 이번 회귀 범위를 잘 커버했습니다.
🔴 반드시 수정할 사항
.reel과.smooth를 런타임에 전환할 때reelPositions가 현재animationRange와 동기화되지 않아 실제 값과 다른 숫자가 표시될 수 있습니다.
💡 권장 개선 사항
- 애니메이션 스타일 전환 시나리오를 테스트에 추가하면
reelPositions와animationRange의 상태 정합성을 유지하는 회귀 방지 효과가 큽니다.
기술 단위 코멘트 1건은 각 라인에 별도 표시됩니다.
리뷰 모델: gpt-5.5
| let digitOrdinal = animationRange.digitOrdinal(at: index) ?? index | ||
|
|
||
| withAnimation(animation.digitAnimation(at: digitOrdinal)) { | ||
| animationRange.set(value, index: index) |
There was a problem hiding this comment.
[Major] 문제 → smooth 경로에서 digit 값을 바꿀 때 animationRange만 갱신하고 같은 column id의 reelPositions는 갱신하지 않습니다. 영향 → .reel로 표시하다가 .smooth로 전환된 상태에서 값이 바뀐 뒤 다시 .reel로 돌아오면, line 179에서 오래된 reelPositions[column.id]가 현재 digit보다 우선되어 실제 value와 다른 숫자가 렌더링될 수 있습니다. 제안 → animation 변경 시 현재 animationRange 기준으로 reelPositions를 재동기화하거나, smooth 경로에서도 digit 변경 시 해당 column id의 reel position을 함께 갱신하세요.
There was a problem hiding this comment.
동의합니다. .smooth 모드에서 값을 업데이트할 때 reelPositions가 갱신되지 않으면, 런타임에 애니메이션 스타일을 .reel로 동적 전환했을 때 마지막에 업데이트된 숫자와 불일치하여 잘못된 숫자가 표시될 수 있습니다. smooth 경로에서도 reelPositions를 동기화하거나, style 변경 감지 시 synchronizeReelPositions를 명시적으로 호출해 상태 일관성을 유지해야 합니다.
There was a problem hiding this comment.
MLX 리뷰
이 PR은 AnimateNumberText의 애니메이션 API를 단순화하여 .smooth(duration:) 하나로 통합하고, 새로운 .reel(duration:revolutions:settleInterval:) 애니메이션을 도입했습니다. 기존의 다양한 SwiftUI Animation 옵션은 제거되었으며, 문서와 예제는 새 API 기준으로 업데이트되었습니다. 모든 테스트와 문서 생성이 성공적으로 통과했습니다.
반드시 수정할 사항
- 공개 initializer의
animation매개변수 타입이 기존의Animation기반 값에서AnimateNumberTextAnimation으로 고정되어, 기존 코드에서animation: .easeOut(duration:)와 같은 SwiftUI 기본 애니메이션을 사용하던 호출부가 컴파일 오류를 발생시킬 수 있습니다.
라인 단위 코멘트
- 자동 리뷰에서 1개의 라인 단위 개선 사항을 남겼습니다.
사용된 모델: mlx-community/Qwen3-30B-A3B-Instruct-2507-8bit
| numberFormatter: NumberFormatter? = nil, | ||
| stringFormatter: String? = nil, | ||
| animation: AnimateNumberTextAnimation = .default | ||
| animation: AnimateNumberTextAnimation = .smooth() |
There was a problem hiding this comment.
[Major] Problem: 공개 initializer의 animation 매개변수 타입이 기존의 Animation 기반 값에서 AnimateNumberTextAnimation으로 고정되어, 기존 코드에서 animation: .easeOut(duration:)와 같은 SwiftUI 기본 애니메이션을 사용하던 호출부가 컴파일 오류를 발생시킬 수 있습니다. Why it matters: 이 변경은 기존 사용자 코드와 문서의 호환성을 깨뜨리는 소스 브레이킹 변경이며, README와 DocC 예제가 여전히 기존 애니메이션 옵션을 언급하고 있어 사용자 혼란을 유발할 수 있습니다. Suggested fix: 기존 Animation 기반 initializer를 유지하거나, 호환 오버로드를 추가하고, 새 .smooth(duration:) 설정은 별도의 팩토리 메서드나 새로운 initializer로 제공하여 기존 사용자가 그대로 빌드되도록 해야 합니다. Confidence: High
Confidence score: 0.95
There was a problem hiding this comment.
보류합니다. 이 프로젝트는 현재 SemVer 0.x 버전대로 하위 호환성 보장보다 아키텍처의 단순화 및 정확성이 강조되는 단계입니다. 자릿수별 개별 릴 애니메이션 및 엇갈린(staggered) 타이밍 연출을 온전히 구현하기 위해서는 SwiftUI 기본 Animation 타입 대신 전용 래퍼 타입(AnimateNumberTextAnimation)을 매개변수로 요구하는 설계가 장기적으로 보다 타당합니다.
roll alias를 제거하고 자리별 릴 애니메이션 공개 API를 reel 하나로 정리했습니다. DocC 예제와 Swift Testing 검증도 reel(duration:revolutions:settleInterval:) 기준으로 갱신했습니다. 불필요한 중복 API를 줄여 호출자가 선택해야 하는 애니메이션 이름을 명확히 했습니다.
There was a problem hiding this comment.
suffix와 숫자 컬럼을 분리해 갱신하는 방향은 타당하고, README와 DocC도 최신 API 기준으로 정리되어 있습니다. 다만 이전 라운드에서 지적된 .reel/.smooth 전환 시 reelPositions 동기화 문제가 현재 코드에도 남아 있어, 런타임 스타일 전환 후 실제 값과 다른 숫자가 표시될 수 있습니다. 이 상태 불일치는 사용자에게 잘못된 값을 보여주는 회귀라 머지 전에 수정이 필요합니다.
좋은 점
- 숫자 컬럼은 우측 기준으로 보존하고 suffix 컬럼은 동일 문자 기준으로 재사용해 깜빡임 원인을 직접 줄인 점이 좋습니다.
- ASCII 숫자만 애니메이션 대상으로 분류해 유니코드 숫자가 의도치 않게 롤링되는 경로를 차단했습니다.
- README와 DocC의
.reel(duration:revolutions:settleInterval:)예제가 현재 공개 API와 맞게 정리되었습니다.
🔴 반드시 수정할 사항
.reel로 표시한 뒤.smooth상태에서 값이 바뀌고 다시.reel로 전환하면, 기존 column id에 매달린 오래된reelPositions가 현재animationRange보다 우선되어 실제 값과 다른 숫자가 렌더링될 수 있습니다. smooth 경로에서도 digit 변경 시 해당 column id의 reel position을 현재 digit으로 동기화하거나, 애니메이션 스타일 변경 시reelPositions를 현재animationRange기준으로 재생성해야 합니다.
💡 권장 개선 사항
- 애니메이션 스타일을
.reel → .smooth → .reel로 전환하면서 값도 변경하는 회귀 테스트를 추가하면 이번 상태 정합성 문제를 안정적으로 막을 수 있습니다. scheduleAnimationUpdateIfNeeded는 현재 내부 suspend 지점이 없으므로, 별도 비동기 대기가 다시 들어갈 계획이 없다면async제거를 후속 정리로 고려할 수 있습니다.
리뷰 모델: gpt-5.5
There was a problem hiding this comment.
이번 PR은 자릿수 변경 시 suffix와 digit 컬럼의 SwiftUI 뷰 정체성(UUID)을 유지하여 레이아웃의 안정성을 크게 높였으며, 릴(reel) 스타일 회전 애니메이션의 정지 시차(settleInterval) 연출을 성공적으로 개선했습니다. 다만, smooth 모드에서 값이 변경될 때 reel 전용 상태인 reelPositions가 갱신되지 않아 런타임에 스타일을 전환할 시 표시 숫자가 불일치할 수 있는 여지가 있습니다. 또한 불필요하게 async로 정의된 일부 내부 메서드를 단순화하는 마이너 리팩터링을 권장합니다.
좋은 점
- Character.asciiDigitValue를 도입하여 아랍 숫자나 로마 숫자와 같은 유니코드 숫자 체계가 롤링 애니메이션 중에 오작동하는 것을 근본적으로 방지하고 ASCII 숫자(0~9)만 애니메이션 대상으로 명확히 정의했습니다.
- resizeForAnimation(to:) 메서드에서 문자열의 우측 정렬을 기준으로 digit 컬럼의 UUID(정체성)를 보존하고 non-digit 컬럼(suffix, 공백 등)을 재사용함으로써 SwiftUI ForEach에서 뷰의 불필요한 재생성을 최소화하고 레이아웃 깜빡임을 방지한 설계가 우수합니다.
개선할 점
- smooth 애니메이션 모드에서 숫자 값이 업데이트될 때 reelPositions 상태가 갱신되지 않습니다. 런타임에 애니메이션 스타일이 .reel과 .smooth 사이에서 동적으로 전환될 경우, 이전에 캐싱된 reelPositions 정보가 현재 값과 어긋나 다른 숫자가 렌더링될 수 있는 상태 불일치 문제가 있습니다. smooth 업데이트 경로에서도 reelPositions를 갱신하거나 애니메이션 스타일 전환 시에 reelPositions를 동기화하는 로직이 필요합니다.
(주: 다음 1개 코멘트는 PR diff 범위 밖이라 본문에 모았습니다.)
드롭된 라인 지적
Sources/AnimateNumberText/Public/AnimateNumberText.swift:127— [Minor] scheduleAnimationUpdateIfNeeded(for:) 메서드는 내부적으로 await를 통한 비동기 대기(suspend point)를 수행하지 않음에도 async로 선언되어 있습니다. 호출부인 .task 블록 등에서 불필요하게 비동기 컨텍스트 홉을 요구하게 되므로, async 키워드를 제거하고 동기 함수로 변경하는 것이 코드 가독성과 오버헤드 측면에서 바람직합니다.
리뷰 생성 모델: Gemini 3.5 Flash (High) #1
|
fix. reel 애니메이션 API를 하나로 단일화했습니다.\n\n- |
There was a problem hiding this comment.
MLX 리뷰
이 PR은 AnimateNumberText 라이브러리에 새로운 .reel 애니메이션 스타일을 도입하고, 기존의 .easeOut 기반 애니메이션을 .smooth로 대체하여 더 정교한 숫자 전환 효과를 제공합니다. 기존 사용자 코드와 문서의 호환성을 깨뜨리는 소스 브레이킹 변경이지만, 0.x 버전대의 특성상 허용 가능합니다.
반드시 수정할 사항
- 공개 initializer의
animation매개변수 타입이 기존의Animation기반 값에서AnimateNumberTextAnimation으로 고정되어, 기존 코드에서animation: .easeOut(duration:)와 같은 SwiftUI 기본 애니메이션을 사용하던 호출부가 컴파일 오류를 발생시킬 수 있습니다.
라인 단위 코멘트
- 자동 리뷰에서 1개의 라인 단위 개선 사항을 남겼습니다.
사용된 모델: mlx-community/Qwen3-30B-A3B-Instruct-2507-8bit
| numberFormatter: NumberFormatter? = nil, | ||
| stringFormatter: String? = nil, | ||
| animation: AnimateNumberTextAnimation = .default | ||
| animation: AnimateNumberTextAnimation = .smooth() |
There was a problem hiding this comment.
[Major] Problem: 공개 initializer의 animation 매개변수 타입이 기존의 Animation 기반 값에서 AnimateNumberTextAnimation으로 고정되어, 기존 코드에서 animation: .easeOut(duration:)와 같은 SwiftUI 기본 애니메이션을 사용하던 호출부가 컴파일 오류를 발생시킬 수 있습니다. Why it matters: 이 변경은 기존 사용자 코드와 문서의 호환성을 깨뜨리는 소스 브레이킹 변경이며, README와 DocC 예제가 여전히 기존 애니메이션 옵션을 언급하고 있어 사용자 혼란을 유발할 수 있습니다. Suggested fix: 기존 Animation 기반 initializer를 유지하거나, 호환 오버로드를 추가하고, 새 .smooth(duration:) 설정은 별도의 팩토리 메서드나 새로운 initializer로 제공하여 기존 사용자가 그대로 빌드되도록 해야 합니다. Confidence: High
Confidence score: 0.95
There was a problem hiding this comment.
보류 — 이 라이브러리는 현재 SemVer 0.x 대에 머물러 있어 하위 호환성 보장보다 설계의 단순함과 정확성이 중요합니다. 자릿수별로 staggered 타이밍 및 개별 릴 회전 효과를 구현하기 위해서는 SwiftUI 기본 Animation 타입 대신 전용 래퍼 타입(AnimateNumberTextAnimation)을 매개변수로 일원화하는 설계가 아키텍처 관점에서 타당하다고 판단됩니다.
reel 애니메이션의 duration 의미를 마지막 숫자가 완전히 멈추는 전체 시간으로 수정했습니다. 기존에는 첫 번째 숫자의 정지 시간에 ordinal * settleInterval을 더해 duration보다 뒤에 마지막 숫자가 멈추는 문제가 있었습니다. digit count를 기준으로 각 자리의 duration을 duration - remainingDigits * settleInterval로 계산해 301.9에서 3, 0, 1, 9가 순차 정지하고 마지막 숫자가 지정한 duration에 멈추게 했습니다. duration이 settleInterval 총합보다 짧을 때 음수 duration이 나오지 않도록 0으로 보정하는 테스트도 추가했습니다.
There was a problem hiding this comment.
이번 PR은 suffix가 포함된 숫자 애니메이션의 안정성을 높이기 위해 문자열 크기 변경 시 digit 컬럼의 식별자를 보존하고 suffix 고정 처리를 도입하였습니다. 다만, .smooth 애니메이션 모드에서 값이 변경될 때 reelPositions 상태가 동기화되지 않아 런타임 스타일 전환 시 잘못된 숫자가 표시될 수 있는 상태 불일치 문제가 남아 있습니다. 또한, 내부 메서드의 불필요한 비동기 선언을 정리하고, SemVer 0.x 단계에 맞춰 전용 애니메이션 래퍼 타입을 단일 매개변수로 단순화한 설계는 장기적인 아키텍처 관점에서 타당해 보입니다.
좋은 점
- Character.asciiDigitValue를 도입하여 유니코드 숫자 체계의 의도치 않은 롤링을 막고 ASCII 숫자만 애니메이션 대상으로 제한한 점이 안전합니다.
- resizeForAnimation(to:)에서 문자열 우측 정렬 기준으로 digit 컬럼의 UUID를 보존하고 non-digit 컬럼을 재사용하여 SwiftUI 뷰의 불필요한 재생성과 깜빡임을 효과적으로 방지했습니다.
개선할 점
- 애니메이션 모드가 런타임에 동적으로 변경될 수 있으므로, .smooth와 .reel 스타일이 전환될 때 뷰 상태(reelPositions 및 animationRange) 간의 동기화 메커니즘을 보다 견고하게 보장해야 합니다.
기술 단위 코멘트 1건은 각 라인에 별도 표시됩니다.
(주: 다음 1개 코멘트는 PR diff 범위 밖이라 본문에 모았습니다.)
드롭된 라인 지적
Sources/AnimateNumberText/Public/AnimateNumberText.swift:128— [Minor] scheduleAnimationUpdateIfNeeded(for:) 메서드 내부에 비동기 대기(await) 지점이 전혀 존재하지 않습니다. async 키워드를 제거하고 동기 함수로 변경하여 불필요한 비동기 컨텍스트 홉을 줄이는 것이 좋습니다.
리뷰 생성 모델: Gemini 3.5 Flash (High) #1
| let digitOrdinal = animationRange.digitOrdinal(at: index) ?? index | ||
|
|
||
| withAnimation(animation.digitAnimation(at: digitOrdinal)) { | ||
| animationRange.set(value, index: index) |
There was a problem hiding this comment.
[Major] .smooth 모드에서 값을 업데이트할 때 reelPositions가 함께 갱신되지 않습니다. 이로 인해 런타임에 애니메이션 스타일이 .smooth에서 .reel로 동적으로 전환될 때, 과거에 저장된 잘못된 reelPositions 값이 사용되어 실제 숫자 값과 렌더링 결과가 불일치하는 버그가 발생할 수 있습니다.
There was a problem hiding this comment.
suffix 고정과 digit 컬럼 보존 방향은 이번 회귀 원인을 잘 겨냥하고 있으며, README와 DocC도 공개 API 기준으로 정리되어 있습니다. 다만 최신 코드에서도 .smooth 경로가 reelPositions를 현재 digit 값과 동기화하지 않아, 애니메이션 스타일을 런타임에 전환하면 실제 값과 다른 숫자가 표시될 수 있습니다. 이 문제는 이미 이전 라운드에서 같은 라인에 지적되어 있어 라인 코멘트는 중복하지 않지만, 머지 전 수정이 필요합니다.
좋은 점
- digit 컬럼은 우측 기준으로 재사용하고 suffix 컬럼은 동일 문자 기준으로 보존해 SwiftUI 뷰 정체성 손실을 줄인 점이 좋습니다.
- ASCII 숫자만 애니메이션 대상으로 분류해 유니코드 숫자에서 의도치 않은 롤링을 막은 점이 명확합니다.
0 ms -> 306.26 ms,9 ms -> 10 ms, 축소 케이스까지 테스트로 고정해 이번 회귀 범위를 잘 덮었습니다.
🔴 반드시 수정할 사항
.reel로 표시한 뒤.smooth상태에서 값이 바뀌고 다시.reel로 전환하면, 기존 column id에 남은 오래된reelPositions가 현재animationRange보다 우선되어 실제 값과 다른 숫자가 렌더링될 수 있습니다. smooth digit 업데이트 시 해당 column id의 reel position을 현재 digit으로 함께 갱신하거나, 애니메이션 스타일 전환 시 현재animationRange기준으로reelPositions를 재생성해야 합니다.
💡 권장 개선 사항
- 애니메이션 스타일을
.reel -> .smooth -> .reel로 전환하면서 값도 변경하는 회귀 테스트를 추가하면 상태 불일치 재발을 막을 수 있습니다.
리뷰 모델: gpt-5.5
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@Sources/AnimateNumberText/Public/AnimateNumberTextAnimation.swift`:
- Around line 94-105: Remove the default digitCount of 1 from
digitAnimation(at:), since callers can otherwise omit it and break reel-style
per-digit settling in AnimateNumberTextAnimation. Update the relevant call
sites, especially the AnimateNumberText usage that currently relies on the
default, to pass the actual digit count explicitly so digitDuration and reel
behavior are computed correctly.
🪄 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 Plus
Run ID: 09c4644a-8f65-4882-ad46-add453d5f237
📒 Files selected for processing (5)
README.mdSources/AnimateNumberText/AnimateNumberText.docc/AnimateNumberText.mdSources/AnimateNumberText/Public/AnimateNumberText.swiftSources/AnimateNumberText/Public/AnimateNumberTextAnimation.swiftTests/AnimateNumberTextTests/AnimateNumberTextTests.swift
✅ Files skipped from review due to trivial changes (2)
- Sources/AnimateNumberText/AnimateNumberText.docc/AnimateNumberText.md
- README.md
🚧 Files skipped from review as they are similar to previous changes (2)
- Tests/AnimateNumberTextTests/AnimateNumberTextTests.swift
- Sources/AnimateNumberText/Public/AnimateNumberText.swift
There was a problem hiding this comment.
MLX 리뷰
이 PR은 AnimateNumberText 라이브러리에 새로운 릴(reel) 스타일 애니메이션을 도입하고, 기존의 easeOut 등 SwiftUI 기본 애니메이션을 대체하는 전용 애니메이션 타입을 도입하여 애니메이션의 정교함과 일관성을 향상시켰습니다. 기존 사용자 코드와 문서의 호환성은 깨졌지만, SemVer 0.x 버전대에서 하위 호환성 파괴가 허용되는 점을 고려하면 합리적인 변경입니다.
반드시 수정할 사항
- 공개 initializer의
animation매개변수 타입이 기존의Animation기반 값에서AnimateNumberTextAnimation으로 고정되어, 기존 코드에서animation: .easeOut(duration:)와 같은 SwiftUI 기본 애니메이션을 사용하던 호출부가 컴파일 오류를 발생시킬 수 있습니다.
라인 단위 코멘트
- 자동 리뷰에서 1개의 라인 단위 개선 사항을 남겼습니다.
사용된 모델: mlx-community/Qwen3-30B-A3B-Instruct-2507-8bit
| numberFormatter: NumberFormatter? = nil, | ||
| stringFormatter: String? = nil, | ||
| animation: AnimateNumberTextAnimation = .default | ||
| animation: AnimateNumberTextAnimation = .smooth() |
There was a problem hiding this comment.
[Major] Problem: 공개 initializer의 animation 매개변수 타입이 기존의 Animation 기반 값에서 AnimateNumberTextAnimation으로 고정되어, 기존 코드에서 animation: .easeOut(duration:)와 같은 SwiftUI 기본 애니메이션을 사용하던 호출부가 컴파일 오류를 발생시킬 수 있습니다. Why it matters: 이 변경은 기존 사용자 코드와 문서의 호환성을 깨뜨리는 소스 브레이킹 변경이며, README와 DocC 예제가 여전히 기존 애니메이션 옵션을 언급하고 있어 사용자 혼란을 유발할 수 있습니다. Suggested fix: 기존 Animation 기반 initializer를 유지하거나, 호환 오버로드를 추가하고, 새 .smooth(duration:) 설정은 별도의 팩토리 메서드나 새로운 initializer로 제공하여 기존 사용자가 그대로 빌드되도록 해야 합니다. Confidence: High
Confidence score: 0.95
There was a problem hiding this comment.
[보류] 해당 라이브러리는 현재 SemVer 0.x 버전대에 머물러 있어 설계의 단순함과 일관성이 더 중요합니다. 자릿수별 지연 타이밍 및 개별 릴 회전 효과를 일관되게 지원하기 위해, 기존 SwiftUI 기본 Animation 대신 전용 래퍼 타입(AnimateNumberTextAnimation)으로 매개변수를 단일화하는 설계 변경이 타당해 보입니다.
reel 애니메이션의 공개 API를 spinningDuration, settleDuration, revolutions 라벨로 정리합니다. spinningDuration은 첫 번째 숫자 컬럼이 멈추는 시간을 의미하고, settleDuration은 다음 숫자 컬럼이 순차적으로 멈추는 간격을 의미하도록 구현했습니다. 예를 들어 301.9는 첫 자리 3이 spinningDuration 시점에 멈춘 뒤 0, 1, 9가 settleDuration 간격으로 멈춥니다. README와 DocC 예제, Swift Testing 테스트를 새 API 기준으로 갱신했습니다. 검증: - git diff --check - swift test
There was a problem hiding this comment.
suffix와 포맷 문자를 즉시 고정하고 digit 컬럼만 애니메이션 대상으로 분리한 방향은 이번 회귀 원인에 잘 맞습니다. 다만 최신 코드에서도 smooth 경로에서 값이 바뀐 뒤 reel 스타일로 전환할 때 reelPositions가 현재 숫자와 동기화되지 않는 상태 불일치가 남아 있습니다. 이미 같은 라인에 여러 봇이 지적했으므로 라인 코멘트는 중복하지 않지만, 머지 전 수정이 필요합니다.
좋은 점
- 숫자 컬럼은 우측 기준으로 보존하고 suffix 컬럼은 동일 문자 기준으로 재사용해 문자열 길이 변경 시 SwiftUI 뷰 정체성 손실을 줄였습니다.
- ASCII 숫자만 애니메이션 대상으로 제한해 유니코드 숫자가 의도치 않게 롤링되는 경로를 막았습니다.
- suffix 정렬, 자리 증가, 자리 축소 케이스를 테스트로 고정해 이번 회귀 범위를 직접 검증했습니다.
🔴 반드시 수정할 사항
- .reel로 표시한 뒤 .smooth 상태에서 값이 변경되고 다시 .reel로 전환되면, 기존 column id에 남아 있는 오래된 reelPositions가 현재 animationRange보다 우선되어 실제 값과 다른 숫자가 렌더링될 수 있습니다. smooth digit 업데이트 시 해당 column id의 reel position도 현재 digit으로 갱신하거나, animation 스타일 변경 시 현재 animationRange 기준으로 reelPositions를 재생성해야 합니다.
💡 권장 개선 사항
- .reel → .smooth → 값 변경 → .reel 전환 시 표시 숫자가 현재 값과 일치하는지 검증하는 회귀 테스트를 추가하면 같은 상태 불일치가 재발하는 것을 막을 수 있습니다.
리뷰 모델: gpt-5.5
There was a problem hiding this comment.
이번 PR은 접미사가 포함된 숫자 애니메이션의 시각적 흔들림 문제를 개선하고 새로운 .reel 애니메이션 스타일을 도입하고 있습니다. 자릿수가 변경될 때 우측 정렬 기준으로 기존 자릿수 컬럼의 UUID를 보존하고 비숫자 컬럼만 재사용하는 방식으로 SwiftUI 뷰의 불필요한 재생성을 방지한 설계가 우수합니다. 다만 내부 메서드의 불필요한 비동기 키워드 제거 및 런타임 내 스타일 전환 시 상태 동기화 누락을 보완하면 완성도가 더욱 높아질 것입니다.
좋은 점
- 자릿수 길이 변경 시 우측 정렬을 기준으로 기존 digit 컬럼의 UUID를 보존하고 변경이 없는 텍스트 컬럼을 재사용하여 SwiftUI 뷰의 불필요한 깜빡임을 줄이고 렌더링 성능을 개선한 점
- Character의 Private extension으로 asciiDigitValue를 정의하여 유니코드 숫자 체계의 의도치 않은 롤링을 방지하고 ASCII 숫자만 애니메이션하도록 한정한 점
개선할 점
- AnimateNumberText 뷰가 .smooth 스타일로 작동할 때 reelPositions 상태가 업데이트되지 않아, 런타임에 애니메이션 스타일이 .smooth에서 .reel로 동적으로 전환될 경우 과거에 남아 있던 잘못된 reelPositions 값으로 인해 렌더링 숫자 불일치 버그가 발생할 수 있습니다. .smooth 모드 실행 중에도 reelPositions를 동기화하거나 스타일 전환 시점에 상태를 명시적으로 리셋하는 설계 보완이 필요합니다.
(주: 다음 1개 코멘트는 PR diff 범위 밖이라 본문에 모았습니다.)
드롭된 라인 지적
Sources/AnimateNumberText/Public/AnimateNumberText.swift:128— [Minor] scheduleAnimationUpdateIfNeeded(for:) 메서드 내부에 비동기 대기(await) 지점이 존재하지 않습니다. async 키워드를 제거하고 동기 함수로 전환하여 불필요한 비동기 컨텍스트 홉을 방지하는 것이 좋습니다.
리뷰 생성 모델: Gemini 3.5 Flash (High) #1
There was a problem hiding this comment.
MLX 리뷰
이 PR은 AnimateNumberText 라이브러리에 새로운 릴(reel) 스타일 애니메이션을 도입하고, 기존 smooth 애니메이션을 유지하면서 애니메이션 컨피그를 전용 타입으로 단일화했습니다. 문서와 테스트를 업데이트하여 새로운 API를 명확히 설명하고, 자릿수별 순차 정지 및 방향 교대 기능을 정교하게 구현했습니다. 기존 사용자 코드와 호환성을 유지하기 위한 조치가 필요합니다.
반드시 수정할 사항
- 공개 initializer의
animation매개변수 타입이 기존의Animation기반 값에서AnimateNumberTextAnimation으로 고정되어, 기존 코드에서animation: .easeOut(duration:)와 같은 SwiftUI 기본 애니메이션을 사용하던 호출부가 컴파일 오류를 발생시킬 수 있습니다. - .smooth 모드에서 값을 업데이트할 때
reelPositions가 함께 갱신되지 않습니다. 이로 인해 런타임에 애니메이션 스타일이 .smooth에서 .reel로 동적으로 전환될 때, 과거에 저장된 잘못된reelPositions값이 사용되어 실제 숫자 값과 렌더링 결과가 불일치하는 버그가 발생할 수 있습니다. digitAnimation(at:ordinal:digitCount:)메서드의digitCount매개변수에 기본값1이 설정되어 있어, 호출 시 실제 자리 수를 전달하지 않으면reel스타일의 자리별 정착 간격이 적용되지 않습니다.
라인 단위 코멘트
- 자동 리뷰에서 3개의 라인 단위 개선 사항을 남겼습니다.
사용된 모델: mlx-community/Qwen3-30B-A3B-Instruct-2507-8bit
| numberFormatter: NumberFormatter? = nil, | ||
| stringFormatter: String? = nil, | ||
| animation: AnimateNumberTextAnimation = .default | ||
| animation: AnimateNumberTextAnimation = .smooth() |
There was a problem hiding this comment.
[Major] Problem: 공개 initializer의 animation 매개변수 타입이 기존의 Animation 기반 값에서 AnimateNumberTextAnimation으로 고정되어, 기존 코드에서 animation: .easeOut(duration:)와 같은 SwiftUI 기본 애니메이션을 사용하던 호출부가 컴파일 오류를 발생시킬 수 있습니다. Why it matters: 이 변경은 기존 사용자 코드와 문서의 호환성을 깨뜨리는 소스 브레이킹 변경이며, README와 DocC 예제가 여전히 기존 애니메이션 옵션을 언급하고 있어 사용자 혼란을 유발할 수 있습니다. Suggested fix: 기존 Animation 기반 initializer를 유지하거나, 호환 오버로드를 추가하고, 새 .smooth(duration:) 설정은 별도의 팩토리 메서드나 새로운 initializer로 제공하여 기존 사용자가 그대로 빌드되도록 해야 합니다. Confidence: High
Confidence score: 0.95
There was a problem hiding this comment.
동의합니다. 기존 SwiftUI의 Animation 형식을 직접 전달하던 사용자의 경우 컴파일 에러가 발생할 것입니다. 호환성을 유지하기 위한 오버로드 이니셜라이저를 추가하거나, 마이그레이션 가이드를 제공하는 편이 좋습니다.
| let digitOrdinal = animationRange.digitOrdinal(at: index) ?? index | ||
|
|
||
| withAnimation(animation.digitAnimation(at: digitOrdinal)) { | ||
| animationRange.set(value, index: index) |
There was a problem hiding this comment.
[Major] Problem: .smooth 모드에서 값을 업데이트할 때 reelPositions가 함께 갱신되지 않습니다. 이로 인해 런타임에 애니메이션 스타일이 .smooth에서 .reel로 동적으로 전환될 때, 과거에 저장된 잘못된 reelPositions 값이 사용되어 실제 숫자 값과 렌더링 결과가 불일치하는 버그가 발생할 수 있습니다. Why it matters: 이는 사용자에게 잘못된 숫자가 표시되는 심각한 UI 오류를 유발하며, 애니메이션의 신뢰성을 심각하게 해칩니다. Suggested fix: animation 변경 시 현재 animationRange 기준으로 reelPositions를 재동기화하거나, smooth 경로에서도 digit 변경 시 해당 column id의 reel position을 함께 갱신하세요. Confidence: High
Confidence score: 0.92
There was a problem hiding this comment.
수정 완료: 최신 HEAD에서 .smooth 경로가 끝날 때 현재 animationRange의 digit 값으로 reelPositions를 동기화하도록 변경했습니다. 따라서 .reel → .smooth 값 변경 → .reel 전환 시 오래된 continuous position이 현재 숫자보다 우선되는 문제가 사라집니다. 회귀 테스트로 currentDigitPositionsDropsStaleContinuousOffsets와 preservingReelPositionsKeepsExistingContinuousOffsets를 추가했고, swift test / DocC 생성 / git diff --check를 통과했습니다.
There was a problem hiding this comment.
동의합니다. smooth 업데이트 도중에도 자릿수 크기가 변경되거나 즉시 업데이트가 이루어지는 경로 등에서는 reelPositions가 누락될 가능성이 여전히 남아 있어 동적 스타일 전환 시 렌더링 값 불일치 버그의 잠재적 원인이 될 수 있습니다.
| return .easeOut(duration: digitDuration(spinningDuration, | ||
| settleDuration: settleDuration, | ||
| ordinal: ordinal, | ||
| digitCount: digitCount)) |
There was a problem hiding this comment.
[Major] Problem: digitAnimation(at:ordinal:digitCount:) 메서드의 digitCount 매개변수에 기본값 1이 설정되어 있어, 호출 시 실제 자리 수를 전달하지 않으면 reel 스타일의 자리별 정착 간격이 적용되지 않습니다. Why it matters: 이로 인해 301.9와 같은 값이 3, 0, 1, 9 순서로 정지하는 대신, 모든 자릿수가 동시에 멈추는 잘못된 애니메이션 효과가 발생할 수 있습니다. Suggested fix: 기본값을 제거하고, 호출부에서 반드시 실제 자리 수를 전달하도록 변경하거나, digitCount를 digitOrdinal 기반으로 동적으로 계산하는 방식으로 개선하세요. Confidence: High
Confidence score: 0.90
There was a problem hiding this comment.
보류가 맞습니다. 최신 코드에서는 digitAnimation(at:digitCount:) 시그니처에서 digitCount 기본값이 제거됐고, 호출부도 실제 digitCount를 명시적으로 넘기고 있어 이 지적은 현재 HEAD 기준으로는 재현되지 않습니다.
There was a problem hiding this comment.
보류합니다. 현재 변경된 코드 기준으로 digitAnimation 및 digitDuration 메서드는 digitCount 매개변수에 대해 기본값을 가지고 있지 않으며, 호출부에서도 정확하게 실제 자릿수를 세어 인자로 제공하고 있으므로 이 문제는 발생하지 않습니다.
코드리뷰에서 지적된 smooth/reel 전환 시 reelPositions 상태 불일치를 수정했습니다. smooth 경로에서 값이 바뀐 뒤에도 현재 digit 값으로 reelPositions를 동기화하여 다시 reel 스타일로 전환해도 오래된 continuous position이 표시되지 않게 했습니다. digitAnimation과 digitDuration의 digitCount 기본값을 제거하고 호출부에서 실제 digit count를 명시하도록 변경했습니다. scheduleAnimationUpdateIfNeeded의 불필요한 async도 제거하고, 관련 상태 동기화 helper와 회귀 테스트를 추가했습니다.
There was a problem hiding this comment.
이번 변경은 suffix/구분자 컬럼과 digit 컬럼을 분리해 기존 깜빡임 회귀를 직접 겨냥하고 있고, 이전 라운드에서 지적된 reel 상태 동기화와 digitCount 기본값 문제도 최신 코드에서는 처리되어 있습니다. 전체적으로 병합을 막을 만한 버그나 데이터 불일치 위험은 확인되지 않았습니다. 다만 reel 스타일에서 값이 바뀌지 않은 숫자까지 다시 회전시키는 동작이 새 기능의 의도라면, 현재 테스트는 내부 계산만 확인하고 실제 업데이트 경로를 고정하지 못해 후속 회귀에 취약합니다.
좋은 점
- 숫자 컬럼은 오른쪽 기준으로 보존하고 비숫자 suffix 컬럼은 동일 문자 기준으로 재사용해 문자열 길이 변경 시 뷰 정체성 손실을 줄인 점이 좋습니다.
- smooth 업데이트 후 reelPositions를 현재 digit 값으로 재동기화해 이전 라운드의 스타일 전환 상태 불일치가 해소되었습니다.
- digitAnimation(at:digitCount:)에서 digitCount 기본값을 제거해 reel 정착 간격 계산 누락 가능성을 줄였습니다.
- ASCII 숫자만 애니메이션 대상으로 제한한 테스트가 있어 유니코드 숫자 표시가 의도치 않게 롤링되는 경로를 막고 있습니다.
💡 권장 개선 사항
- reel 스타일에서 값이 동일한 digit도 revolutions 설정에 따라 다시 회전해야 한다면,
settingReelAnimationRange가needsUpdate와 무관하게 해당 경로를 실행한다는 것을 뷰 상태 단위 테스트 또는 더 좁은 헬퍼 테스트로 고정하면 좋습니다. 현재는reelTargetPosition의 산술만 검증되어 실제 업데이트 루프 회귀를 놓칠 수 있습니다.
리뷰 모델: gpt-5.5
There was a problem hiding this comment.
이번 PR은 자릿수 변경 시 SwiftUI 뷰의 안정적인 정렬을 유지하고, 새로운 릴 애니메이션 스타일을 도입하여 완성도를 높였습니다. 다만 애니메이션 스타일의 동적 전환 시 상태 불일치 가능성과 공개 생성자의 호환성 이슈를 보완하면 더욱 안전한 라이브러리가 될 것입니다.
좋은 점
- TextColumn 배열의 resizeForAnimation(to:) 메서드에서 자릿수가 늘어나거나 줄어들 때 우측 정렬을 기준으로 기존 digit 컬럼의 UUID를 보존하고, suffix 컬럼은 동일 문자 기준으로 재사용하여 SwiftUI 뷰의 레이아웃 깜빡임을 방지한 점
- Character의 private extension으로 asciiDigitValue를 정의해 ASCII 범위의 숫자만 애니메이션하도록 한정함으로써 유니코드 숫자 체계로 인한 동작 오류 가능성을 차단한 점
개선할 점
- AnimateNumberText 뷰가 .smooth 스타일로 동작하는 도중에 자릿수가 변경되거나 즉시 업데이트가 일어나는 경우, reelPositions 상태가 최신 자릿수 데이터와 동기화되지 않아 추후 .reel 스타일로 전환될 때 숫자 표시 오류가 발생할 수 있습니다. 모든 업데이트 경로에서 reelPositions가 최신 상태를 반영하도록 구조적 보완이 권장됩니다.
기술 단위 코멘트 1건은 각 라인에 별도 표시됩니다.
리뷰 생성 모델: Gemini 3.5 Flash (High) #1
| private func resizeAnimationRange(to stringValue: String) { | ||
| withoutAnimation { | ||
| animationRange.resizeForAnimation(to: stringValue) | ||
| synchronizeReelPositions() |
There was a problem hiding this comment.
[Major] .smooth 애니메이션 모드에서 자릿수가 변경되는 경우, synchronizeReelPositions() 내부의 guard let 조건으로 인해 reelPositions가 갱신되지 못하고 건너뛰게 됩니다. 이로 인해 추후 동적으로 스타일을 변경할 때 상태 불일치 버그가 생길 수 있으므로, smooth 모드에서도 자릿수 변경 시 해당 컬럼 위치를 동기화해주어야 합니다.
코드리뷰 검토 결과\n\n수정 완료: smooth/reel 전환 시 reelPositions 상태 불일치\n- 실제 수정 필요로 판단했습니다.\n- .smooth 경로에서 값이 바뀐 뒤 현재 digit 값으로 reelPositions를 동기화하도록 변경했습니다.\n- .reel → .smooth 값 변경 → .reel 전환 시 오래된 continuous position이 현재 숫자보다 우선되는 문제를 막았습니다.\n- 관련 회귀 테스트를 추가했습니다.\n\n수정 완료: digitCount 기본값\n- 실제 수정 필요로 판단했습니다.\n- digitAnimation(at:digitCount:)와 digitDuration(at:digitCount:)에서 digitCount 기본값을 제거했습니다.\n- 호출부가 실제 digit count를 명시하게 하여 reel 자리별 정지 계산 누락을 컴파일 단계에서 방지합니다.\n\n수정 완료: 불필요한 async\n- scheduleAnimationUpdateIfNeeded(for:) 내부에 suspend point가 없어 async를 제거했습니다.\n\n보류: animation 파라미터 타입 변경 호환성\n- 0.7.0에서 의도한 공개 API 변경으로 판단했습니다.\n- reel의 자리별 방향/정지 시간 계산은 SwiftUI 기본 Animation만으로 표현하기 어렵기 때문에 AnimateNumberTextAnimation 단일 API를 유지합니다.\n\n검증:\n- swift test 통과: 31 tests\n- git diff --check 통과\n- DOCS_OUTPUT_PATH=.build/docc-site DOCC_ARCHIVE_PATH=.build/AnimateNumberText.doccarchive ./GeneratingDocumentationSite 통과\n- GitHub Actions build / CodeRabbit 통과\n\n수정 커밋을 push했으므로 규칙에 따라 머지는 하지 않고 대기합니다. |
There was a problem hiding this comment.
MLX 리뷰
이 PR은 AnimateNumberText의 기본 애니메이션을 .smooth(duration:)로 변경하고, .reel 스타일을 단일화하여 사용자 정의 애니메이션 API를 개선했습니다. 기존의 .easeOut 등 SwiftUI 기본 애니메이션은 더 이상 사용되지 않으며, 애니메이션 스타일 전환 시 상태 일관성을 보장하기 위해 reelPositions 동기화 로직이 강화되었습니다.
반드시 수정할 사항
- 공개 initializer의
animation매개변수 타입이 기존의Animation기반 값에서AnimateNumberTextAnimation으로 고정되어, 기존 코드에서animation: .easeOut(duration:)와 같은 SwiftUI 기본 애니메이션을 사용하던 호출부가 컴파일 오류를 발생시킬 수 있습니다. - .smooth 모드에서 값을 업데이트할 때
reelPositions가 함께 갱신되지 않습니다. 이로 인해 런타임에 애니메이션 스타일이 .smooth에서 .reel로 동적으로 전환될 때, 과거에 저장된 잘못된reelPositions값이 사용되어 실제 숫자 값과 렌더링 결과가 불일치하는 버그가 발생할 수 있습니다. digitAnimation(at:ordinal:digitCount:)메서드의digitCount매개변수에 기본값1이 설정되어 있어, 호출 시 실제 자리 수를 전달하지 않으면reel스타일의 자리별 정착 간격이 적용되지 않습니다.
라인 단위 코멘트
- 자동 리뷰에서 3개의 라인 단위 개선 사항을 남겼습니다.
사용된 모델: mlx-community/Qwen3-30B-A3B-Instruct-2507-8bit
| numberFormatter: NumberFormatter? = nil, | ||
| stringFormatter: String? = nil, | ||
| animation: AnimateNumberTextAnimation = .default | ||
| animation: AnimateNumberTextAnimation = .smooth() |
There was a problem hiding this comment.
[Major] Problem: 공개 initializer의 animation 매개변수 타입이 기존의 Animation 기반 값에서 AnimateNumberTextAnimation으로 고정되어, 기존 코드에서 animation: .easeOut(duration:)와 같은 SwiftUI 기본 애니메이션을 사용하던 호출부가 컴파일 오류를 발생시킬 수 있습니다. Why it matters: 이 변경은 기존 사용자 코드와 문서의 호환성을 깨뜨리는 소스 브레이킹 변경이며, README와 DocC 예제가 여전히 기존 애니메이션 옵션을 언급하고 있어 사용자 혼란을 유발할 수 있습니다. Suggested fix: 기존 Animation 기반 initializer를 유지하거나, 호환 오버로드를 추가하고, 새 .smooth(duration:) 설정은 별도의 팩토리 메서드나 새로운 initializer로 제공하여 기존 사용자가 그대로 빌드되도록 해야 합니다. Confidence: High
Confidence score: 0.95
|
|
||
| withAnimation(animation.digitAnimation(at: digitOrdinal, | ||
| digitCount: digitCount)) { | ||
| animationRange.set(value, index: index) |
There was a problem hiding this comment.
[Major] Problem: .smooth 모드에서 값을 업데이트할 때 reelPositions가 함께 갱신되지 않습니다. 이로 인해 런타임에 애니메이션 스타일이 .smooth에서 .reel로 동적으로 전환될 때, 과거에 저장된 잘못된 reelPositions 값이 사용되어 실제 숫자 값과 렌더링 결과가 불일치하는 버그가 발생할 수 있습니다. Why it matters: 이는 사용자에게 잘못된 숫자가 표시되는 심각한 UI 오류를 유발하며, 애니메이션의 신뢰성을 심각하게 해칩니다. Suggested fix: animation 변경 시 현재 animationRange 기준으로 reelPositions를 재동기화하거나, smooth 경로에서도 digit 변경 시 해당 column id의 reel position을 함께 갱신하세요. Confidence: High
Confidence score: 0.92
| return .easeOut(duration: digitDuration(spinningDuration, | ||
| settleDuration: settleDuration, | ||
| ordinal: ordinal, | ||
| digitCount: digitCount)) |
There was a problem hiding this comment.
[Major] Problem: digitAnimation(at:ordinal:digitCount:) 메서드의 digitCount 매개변수에 기본값 1이 설정되어 있어, 호출 시 실제 자리 수를 전달하지 않으면 reel 스타일의 자리별 정착 간격이 적용되지 않습니다. Why it matters: 이로 인해 301.9와 같은 값이 3, 0, 1, 9 순서로 정지하는 대신, 모든 자릿수가 동시에 멈추는 잘못된 애니메이션 효과가 발생할 수 있습니다. Suggested fix: 기본값을 제거하고, 호출부에서 반드시 실제 자리 수를 전달하도록 변경하거나, digitCount를 digitOrdinal 기반으로 동적으로 계산하는 방식으로 개선하세요. Confidence: High
Confidence score: 0.90
1 participant
작업 내용
stringFormatter: "%@ ms"처럼 suffix를 붙인 숫자 애니메이션에서ms가 깜빡이고.easeOut(duration:)같은 느려지는 옵션이 눈에 잘 보이지 않는 문제를 보정했습니다.0placeholder로 채웁니다.ms가 흔들리지 않도록 했습니다..easeOut(duration:)같은 느려지는 옵션이 숫자 컬럼에 반영되도록 했습니다.0 ms -> 306.26 ms,9 ms -> 10 ms형태의 길이 변경 회귀 테스트를 추가했습니다.reel공개 API를.reel(spinningDuration:settleDuration:revolutions:)로 정리했습니다.spinningDuration은 첫 자리 숫자가 멈추는 시간,settleDuration은 다음 자리 숫자가 순차 정지하는 간격으로 동작합니다.301.9는3이 먼저 멈춘 뒤0,1,9가settleDuration간격으로 멈춥니다.reelAPI 기준으로 갱신했습니다.릴리즈 운영
0.7.0GitHub Release와 원격 태그는 삭제했습니다.0.7.0릴리즈를 다시 생성하면 됩니다.7.0.0release/tag는 존재하지 않아 삭제 대상이 아니었습니다.검증
swift testDOCS_OUTPUT_PATH=.build/docc-site DOCC_ARCHIVE_PATH=.build/AnimateNumberText.doccarchive ./GeneratingDocumentationSitegit diff --checkSummary by CodeRabbit
smooth와reel방식으로 설정할 수 있게 개선했고, 기본값을smooth로 변경했습니다.reel스타일용 회전 효과를 추가했습니다.AnimateNumberTextAnimation설정 방식이DigitTiming에서Style(smooth/reel)로 전환됩니다.smooth/reel중심으로 업데이트했습니다.