fix: 0.7.1 reel 업데이트 경로 보강#12
Conversation
PR #11 리뷰에서 후속 후보로 남긴 reel 업데이트 경로 검증을 보강합니다. - digit ordinal 매핑을 한 번에 계산하는 digitOrdinalsByIndex를 추가했습니다. - reel target position 계산을 TextColumn 배열 헬퍼로 분리해 테스트 가능한 단위로 만들었습니다. - settingSmoothAnimationRange와 settingReelAnimationRange에서 반복 digitOrdinal 계산을 줄였습니다. - 값이 동일한 digit도 reel revolutions 설정에 따라 다시 회전하는 경로를 테스트로 고정했습니다. - README와 DocC 설치 예시 버전을 0.7.1로 갱신했습니다. 검증: - 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 35 minutes and 15 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 (2)
Walkthrough문서의 설치 예시와 배지 버전 표기가 0.7.1로 갱신되고, 숫자 컬럼 ordinal 계산과 reel 위치 갱신 경로가 사전 계산 기반으로 바뀌었습니다. 관련 테스트가 추가되었습니다. Changes숫자 ordinal 기반 애니메이션 갱신
버전 표기 갱신
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~30 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 |
![codex-review-bot[bot]](https://avatars.githubusercontent.com/in/3409825?s=60&v=4){kind=small}
There was a problem hiding this comment.
변경사항은 reel 애니메이션의 연속 위치 계산을 별도 헬퍼로 분리하고, 동일 digit도 업데이트 경로에서 회전하도록 테스트로 고정해 회귀 방지 범위를 잘 좁혔습니다. README와 DocC 버전 갱신도 실제 공개 API 예제와 맞게 반영되어 있습니다. 전체 변경을 검토했지만 병합을 막을 만한 버그, 예외 처리 누락, 상태 불일치 위험은 확인되지 않았습니다.
좋은 점
- reel position 계산을
TextColumn배열 헬퍼로 분리해 실제 렌더링 경로와 테스트가 더 직접적으로 연결된 점 - 동일 digit 회전, suffix 정렬, ASCII digit 처리처럼 회귀 가능성이 있던 경계를 테스트로 고정한 점
- 문서의 설치 버전을 코드 변경 범위와 함께 0.7.1로 일관되게 갱신한 점
리뷰 모델: gpt-5.5
![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은 0.7.1 릴리즈 후속 조치로서, 문자 업데이트 경로의 Reel 애니메이션 보강 및 연산 성능 최적화를 안전하게 수행했습니다. 문자열 길이 변경 시의 UUID 식별자 유지 정책과 O(N^2) 연산 경로의 O(N) 개선을 통해 SwiftUI 렌더링 성능과 안정성을 향상시켰습니다.
좋은 점
- TextColumn.resizeForAnimation(to:) 메서드에서 숫자와 텍스트 열의 보존 처리를 분리하고 역순으로 순회하며 잔여 요소를 맵핑함으로써, 문자열 길이 변경 시 기존 컬럼의 UUID 식별자를 안정적으로 유지하여 레이아웃이 튀는 현상을 방지한 점
- renderedColumns 연산 프로퍼티 내에서 1회 순회로 digitOrdinal과 digitCount를 O(N)으로 수집하도록 최적화하여, 기존의 불필요한 중복 필터링 루프를 제거한 점
개선할 점
- 구조적 개선을 거치며 더 이상 본문 코드에서 사용되지 않는 digitOrdinal(at:) 헬퍼 메서드가 남아 있으므로, 코드 유지보수성을 위해 이를 완전히 정리하거나 테스트 타겟 전용 유틸리티로 분류할 것을 권장합니다.
기술 단위 코멘트 1건은 각 라인에 별도 표시됩니다.
(주: 다음 1개 코멘트는 PR diff 범위 밖이라 본문에 모았습니다.)
드롭된 라인 지적
Sources/AnimateNumberText/Private/TextType.swift:83— [Suggestion] preservedTextColumns.remove(at: currentIndex)는 배열 내 요소를 제거하는 O(N) 작업이며, 루프 내부에서 지속적으로 수행되어 최악의 경우 O(N^2) 복잡도를 가집니다. 일반적인 숫자 포맷 문자열은 길이가 짧아 성능 영향은 미미하지만, 중복 구분자가 많은 대규모 텍스트 처리 시 성능 저하를 방지하기 위해 맵이나 연결 리스트 기반으로 스캔 구조를 대체하는 방안을 고려해 볼 수 있습니다.
리뷰 생성 모델: Gemini 3.5 Flash (High) #1
| return self[index].value != value | ||
| } | ||
|
|
||
| func digitOrdinal(at index: Int) -> Int? { |
There was a problem hiding this comment.
[Minor] digitOrdinalsByIndex() 도입으로 digitOrdinal(at:) 메서드는 프로덕션 코드 내에서 사용처가 없어졌습니다. 테스트 코드 외에 실제 앱 런타임에는 기여하지 않는 죽은 코드(Dead Code)이므로 정리하는 것을 권장합니다.
There was a problem hiding this comment.
수정 완료: 최신 커밋에서 프로덕션 코드와 테스트 코드 모두 digitOrdinal(at:) 의존을 제거하고, digitOrdinalsByIndex() 기준 테스트로 정리했습니다. 검증: swift test, DocC 생성, git diff --check.
There was a problem hiding this comment.
이번 PR의 diff를 확인하면 digitOrdinal(at:) 메서드는 실제로 삭제되었고, 테스트도 digitOrdinalsByIndex()를 사용하도록 변경되었습니다. dead code 문제는 해당 사항이 없는 것으로 보입니다.
![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은 AnimateNumberText 라이브러리의 버전을 0.7.0에서 0.7.1로 업데이트하고, reel 애니메이션에서 숫자 자리수 순서를 정확히 추적하는 기능을 추가하여, 숫자 변경 시 각 자리수의 순서를 기반으로 애니메이션 타이밍을 정확히 계산하도록 개선했습니다. 이로 인해 숫자의 순서가 중요한 애니메이션 시나리오에서 더 자연스러운 동작이 가능해졌습니다.
라인 단위 코멘트
- 라인 단위로 남길 개선 사항은 발견되지 않았습니다.
사용된 모델: mlx-community/Qwen3-30B-A3B-Instruct-2507-8bit
![claudecode-pr-review-bot[bot]](https://avatars.githubusercontent.com/in/3920205?s=60&v=4){kind=small}
There was a problem hiding this comment.
O(N²) digitOrdinal(at:) 반복 호출을 digitOrdinalsByIndex()로 O(N) 사전 계산으로 개선하고, reel 위치 계산 로직을 View 바깥의 테스트 가능한 헬퍼로 분리한 깔끔한 리팩터링입니다. 동작 변경 없이 내부 구조를 정리했으며, 새 테스트로 회귀 방지력이 보강되었습니다. critical/major 수준의 문제는 발견되지 않았고, dead code 정리 등 소소한 개선점만 남아 있습니다.
좋은 점
- 루프 내
digitOrdinal(at:)반복 호출(O(N²))을digitOrdinalsByIndex()단일 O(N) 사전 계산으로 대체하여, smooth/reel 양쪽 경로의 불필요한 재순회를 제거한 점 - reel 위치 계산을
[TextColumn].reelPositions(updatingTo:animation:currentPositions:)로 분리하여,@MainActor·@State의존 없이 순수 함수로 단위 테스트가 가능해진 점 reelPositionsSpinUnchangedDigitsThroughUpdatePath테스트로 동일 값 digit도revolutions설정에 따라 회전하는 reel 동작을 회귀 방지 테스트로 고정한 점canAnimateDigitChange(to:TextType)·needsUpdate(to:TextType)overload를 도입하여 같은Character에 대한TextType변환 중복을 줄인 점
💡 권장 개선 사항
digitOrdinal(at:)메서드가 이번 PR 이후 프로덕션 코드(settingSmoothAnimationRange,settingReelAnimationRange)에서 더 이상 호출되지 않는 것으로 보입니다. 테스트 전용으로 유지하려면 그 의도를 주석으로 명시하거나, 불필요하다면 제거를 검토해 주세요. 남겨두면 dead code로 오해받거나 향후 정리 시 혼동이 생길 수 있습니다.digitOrdinalsByIndex()내부에서reserveCapacity(digitCount)호출 시digitCount프로퍼티가filter(\.value.isNumber).count로 배열을 한 번 순회하고, 바로 아래 본문 루프에서 다시 순회합니다. 실질적 성능 영향은 없지만,reserveCapacity를 제거하거나 루프 안에서 capacity를 함께 추적하면 순회가 한 번으로 줄어 의도가 더 명확해집니다.
기술 단위 코멘트 2건은 각 라인에 별도 표시됩니다.
리뷰 모델: Claude Opus 4.6 (Thinking) #2
| return self[index].value != value | ||
| } | ||
|
|
||
| func digitOrdinal(at index: Int) -> Int? { |
There was a problem hiding this comment.
[Minor] digitOrdinal(at:) 메서드는 이번 PR에서 settingSmoothAnimationRange과 settingReelAnimationRange 양쪽 호출부가 digitOrdinalsByIndex()로 대체되어, 프로덕션 코드에서 더 이상 호출되지 않는 것으로 보입니다. 테스트(digitOrdinalSkipsFormattedCharacters)에서만 사용 중입니다.
테스트 전용으로 유지하려면 // Used only by tests to verify individual ordinal lookups. 같은 주석을 남겨 의도를 명시하거나, digitOrdinalsByIndex()의 테스트만으로 충분하다면 제거를 검토해 주세요.
There was a problem hiding this comment.
수정 완료: 테스트 전용 주석으로 남기는 대신 dead code 오해가 없도록 digitOrdinal(at:)를 완전히 제거했습니다. 테스트는 digitOrdinalsByIndexSkipsFormattedCharacters로 변경했습니다. 검증: swift test, DocC 생성, git diff --check.
There was a problem hiding this comment.
최신 헤드 기준으로 digitOrdinal(at:) 메서드는 더 이상 남아 있지 않아 이 지적은 이미 처리된 것으로 보입니다. 현재는 digitOrdinalsByIndex() 테스트가 해당 경로를 검증하고 있습니다.
|
|
||
| func digitOrdinalsByIndex() -> [Int: Int] { | ||
| var ordinalsByIndex: [Int: Int] = [:] | ||
| ordinalsByIndex.reserveCapacity(digitCount) |
There was a problem hiding this comment.
[Suggestion] reserveCapacity(digitCount)에서 digitCount가 filter(\.value.isNumber).count로 배열을 한 번 순회한 뒤, 바로 아래 for index in indices where ...에서 다시 순회합니다. 숫자 자릿수가 작아 실질적 차이는 없지만, reserveCapacity 호출을 생략하면 순회가 한 번으로 줄고 코드도 간결해집니다. Dictionary는 내부적으로 동적 확장하므로 소규모 데이터에서 사전 할당의 이점이 거의 없습니다.
There was a problem hiding this comment.
수정 완료: reserveCapacity(digitCount)가 사전 계산을 위해 한 번 더 순회한다는 지적이 맞아 제거했습니다. 이제 digitOrdinalsByIndex()는 단일 순회로 ordinal map을 구성합니다. 검증: swift test, DocC 생성, git diff --check.
PR #12 리뷰에서 지적된 소소한 정리 사항을 반영합니다. - 프로덕션 코드에서 더 이상 사용하지 않는 digitOrdinal(at:) 헬퍼를 제거했습니다. - digitOrdinalsByIndex()에서 reserveCapacity(digitCount)를 제거해 불필요한 추가 순회를 없앴습니다. - 관련 테스트명을 digitOrdinalsByIndex 기준으로 정리했습니다. 검증: - swift test - DOCS_OUTPUT_PATH=.build/docc-site DOCC_ARCHIVE_PATH=.build/AnimateNumberText.doccarchive ./GeneratingDocumentationSite - git diff --check
코드리뷰 검토 결과수정 완료:
수정 완료:
보류:
보류: CodeRabbit docstring coverage warning
검증:
수정 커밋을 push했으므로 저장소 규칙에 따라 머지는 하지 않고 대기합니다. |
There was a problem hiding this comment.
이번 풀 리퀘스트는 자릿수 변경 시 애니메이션 처리를 위해 컬럼의 고유 식별자를 안정적으로 보존하고, 자릿수 순서 계산을 일괄 처리하도록 최적화하여 렌더링 성능과 애니메이션 일관성을 개선했습니다. 릴 스타일 애니메이션에서 값이 동일한 자릿수도 설정에 따라 회전하도록 보완하였으며 관련 회귀 방지 테스트를 적절히 추가했습니다. 전반적인 아키텍처와 코드가 깔끔하게 정리되었고 동작을 검증하기 위한 단위 테스트가 잘 보강되었습니다.
좋은 점
- TextColumn 배열의 resizeForAnimation 메서드에서 애니메이션 자릿수 변경 시 역순 순회를 활용해 기존 컬럼의 고유 식별자를 안정적으로 보존하여 SwiftUI의 불필요한 뷰 재생성을 방지한 점
- 동일한 값의 자릿수에 대해서도 revolutions 설정에 따라 회전 효과를 일관되게 적용하도록 reelTargetPosition 산식을 개선하고 이를 검증하는 테스트 코드를 상세하게 작성한 점
개선할 점
- SwiftUI 뷰의 body가 렌더링될 때마다 renderedColumns 계산 프로퍼티에서 자릿수 개수를 세기 위해 filter 연산이 반복적으로 수행되므로, 한 번의 맵 순회 과정에서 자릿수를 함께 추적하도록 변경하여 연산 횟수를 최소화할 수 있습니다.
(주: 다음 2개 코멘트는 PR diff 범위 밖이라 본문에 모았습니다.)
드롭된 라인 지적
Sources/AnimateNumberText/Public/AnimateNumberText.swift:122— [Suggestion] task 수식어는 비동기 태스크를 생성하므로 상태 업데이트가 다음 런루프 틱으로 지연될 수 있습니다. 사용자 입력의 즉각적인 반영을 위해 동기적으로 갱신 처리를 감시하는 onChange 수식어의 사용을 고려해 보시기 바랍니다.Sources/AnimateNumberText/Public/AnimateNumberText.swift:161— [Minor] renderedColumns 계산 프로퍼티 내부에서 digitCount를 조회할 때마다 배열 전체를 매번 필터링하여 순회합니다. 뷰가 렌더링될 때마다 O(N) 연산이 중복으로 발생하므로, 맵 메서드 순회 중에 자릿수를 함께 계산하는 방식으로 효율화할 수 있습니다.
리뷰 생성 모델: Gemini 3.5 Flash (High) #1
There was a problem hiding this comment.
변경사항은 reel 애니메이션의 위치 계산 경로를 테스트 가능한 헬퍼로 분리하고, 동일 숫자 자리도 업데이트 시 회전하도록 회귀 테스트를 추가한 점이 핵심입니다. 전체 코드 흐름과 테스트를 함께 확인했지만 병합을 막을 버그, 예외 처리 누락, 상태 불일치 위험은 보이지 않습니다. 문서 버전 갱신도 README와 DocC에 일관되게 반영되어 있습니다.
좋은 점
- reel 위치 계산을 순수 헬퍼로 분리해 SwiftUI 상태 없이 단위 테스트할 수 있게 한 점
- 동일 digit 회전, 포맷 문자 건너뛰기, ASCII digit 처리 같은 경계 조건을 테스트로 고정한 점
- README와 DocC 설치 예시 버전을 0.7.1로 함께 갱신한 점
리뷰 모델: gpt-5.5
최신 승인 리뷰 추가 검토보류:
보류:
|
There was a problem hiding this comment.
reel 위치 계산 로직을 테스트 가능한 순수 함수로 분리하고, O(N²) digitOrdinal(at:) 반복 호출을 O(N) 사전 계산으로 대체한 깔끔한 리팩터링입니다. 동작 변경 없이 내부 구조를 개선했으며, 동일 digit 회전 경로를 테스트로 고정해 회귀 방지력을 보강했습니다. 병합을 차단할 만한 버그, 예외 처리 누락, 상태 불일치 위험은 발견되지 않았습니다.
좋은 점
- 루프 내
digitOrdinal(at:)O(N²) 반복 호출을digitOrdinalsByIndex()단일 O(N) 사전 계산으로 대체하여, smooth·reel 양쪽 경로의 불필요한 재순회를 제거한 점 reelPositions(updatingTo:animation:currentPositions:)헬퍼로 reel 위치 계산을@MainActor·@State의존 없이 순수 함수로 분리하여 단위 테스트 가능성을 확보한 점- 동일 값 digit도
revolutions설정에 따라 회전하는 reel 동작을reelPositionsSpinUnchangedDigitsThroughUpdatePath테스트로 고정하여 회귀 방지력을 높인 점 canAnimateDigitChange(to:TextType)·needsUpdate(to:TextType)오버로드를 도입해 동일Character에 대한TextType변환 중복을 줄인 점
💡 권장 개선 사항
settingReelAnimationRange에서synchronizeReelPositions()가 내부적으로preservingReelPositions를 호출한 뒤,reelPositions(updatingTo:...)가 다시preservingReelPositions를 호출하여 동일 연산이 두 번 수행됩니다. 숫자 자릿수가 짧아 실질적 영향은 없지만, 향후 리팩터링 시 중복 호출을 정리하면 의도가 더 명확해질 수 있습니다.
리뷰 모델: Claude Opus 4.6 (Thinking) #2
There was a problem hiding this comment.
MLX 리뷰
이 PR은 AnimateNumberText 라이브러리의 버전을 0.7.0에서 0.7.1로 업데이트하고, 숫자 자릿수 순서를 관리하는 digitOrdinalsByIndex() 메서드를 도입하여 .reel 애니메이션의 정확성을 개선합니다. 기존의 digitOrdinal(at:) 메서드는 더 이상 사용되지 않으며, 테스트 코드에서만 남아 있습니다.
반드시 수정할 사항
digitOrdinal(at:)메서드는 프로덕션 코드에서 더 이상 사용되지 않으며, 테스트 코드 외에는 전혀 사용되지 않습니다. 이는 죽은 코드(Dead Code)로, 유지보수 부담과 불필요한 복잡성을 증가시킵니다.
권장 개선사항
reelPositions(updatingTo:animation:currentPositions:)메서드 내에서digitOrdinalsByIndex()를 호출한 후,guard문에서let digitOrdinal = digitOrdinals[index]를 사용하지만,digitOrdinals는digitOrdinalsByIndex()의 결과로index가 존재하지 않으면nil을 반환합니다. 이는digitOrdinal이nil일 수 있음을 의미하며,animation.reelTargetPosition에 전달될 때nil이 전달될 수 있습니다.
라인 단위 코멘트
- 자동 리뷰에서 2개의 라인 단위 개선 사항을 남겼습니다.
사용된 모델: mlx-community/Qwen3-30B-A3B-Instruct-2507-8bit
| @@ -93,6 +93,18 @@ extension Array where Element == TextColumn { | |||
| filter(\.value.isNumber).count | |||
There was a problem hiding this comment.
[Major] Problem: digitOrdinal(at:) 메서드는 프로덕션 코드에서 더 이상 사용되지 않으며, 테스트 코드 외에는 전혀 사용되지 않습니다. 이는 죽은 코드(Dead Code)로, 유지보수 부담과 불필요한 복잡성을 증가시킵니다. Why it matters: 죽은 코드는 코드베이스의 품질을 저하시키고, 다른 개발자가 이를 오해하거나 의도적으로 사용할 위험이 있습니다. Suggested fix: digitOrdinal(at:) 메서드를 완전히 제거하고, 테스트 코드에서 digitOrdinalsByIndex()를 사용하도록 수정하세요. Confidence: High
Confidence score: 0.95
| let digitOrdinals = digitOrdinalsByIndex() | ||
| var nextPositions = preservingReelPositions(currentPositions) | ||
|
|
||
| for (index, character) in characters.enumerated() { |
There was a problem hiding this comment.
[Minor] Problem: reelPositions(updatingTo:animation:currentPositions:) 메서드 내에서 digitOrdinalsByIndex()를 호출한 후, guard 문에서 let digitOrdinal = digitOrdinals[index]를 사용하지만, digitOrdinals는 digitOrdinalsByIndex()의 결과로 index가 존재하지 않으면 nil을 반환합니다. 이는 digitOrdinal이 nil일 수 있음을 의미하며, animation.reelTargetPosition에 전달될 때 nil이 전달될 수 있습니다. Why it matters: reelTargetPosition의 ordinal 파라미터는 Int 타입이므로 nil이 전달되면 컴파일 오류가 발생합니다. Suggested fix: digitOrdinal이 nil일 경우 continue로 처리하는 guard 문을 추가하여 nil 처리를 명시적으로 보장하세요. Confidence: Medium
Confidence score: 0.88
1 participant
작업 내용
PR #11 리뷰 댓글 중 후속 보강 가치가 있는 항목을 검토해 0.7.1 후속 작업으로 반영했습니다.
수정 완료:
revolutions설정에 따라 다시 회전하는 업데이트 경로를 테스트로 고정했습니다.reelTargetPosition산술 검증에만 기대지 않도록,TextColumn배열의 실제 reel position 계산 경로를 내부 헬퍼로 분리했습니다.digitOrdinal(at:)반복 호출로 생기던 이론상 O(N^2) 계산을 줄이기 위해 digit ordinal 매핑을 한 번에 계산하도록 정리했습니다.TextType기반 비교 overload를 추가해 같은 target 문자를 여러 번 enum으로 변환하는 경로를 줄였습니다.보류:
Animation호환 initializer 복원은 0.7.0에서 의도한 공개 API 단순화와 충돌하므로 이번 0.7.1 후속 범위에서 제외했습니다.검증
swift testDOCS_OUTPUT_PATH=.build/docc-site DOCC_ARCHIVE_PATH=.build/AnimateNumberText.doccarchive ./GeneratingDocumentationSitegit diff --checkSummary by CodeRabbit
New Features
Bug Fixes
Tests