test: ジオデシック strut が閉じた三価ケージであることを検証#288
Conversation
The dual edge list already had a 120-edge invariant, but a malformed subdivision could hold that count while skewing the hub topology. Count each strut endpoint's degree and assert all 80 centroids are trivalent (80 × 3 / 2 = 120), pinning the closed-manifold structure the edge-count test alone can miss.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Warning Review limit reached
More reviews will be available in 45 minutes and 16 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 We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughVersion string bumped from ChangesVersion Bump
Trivalent Cage Test
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~4 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
GeneralD
left a comment
GeneralD
left a comment
There was a problem hiding this comment.
ジオデシック幾何構造が「三価ケージ（各頂点の次数が3、合計80頂点）」であることを検証する不変条件テストが追加されました。実装はプロジェクトの規約に適合しており、品質も良好です。パフォーマンスと型安全性の観点から微細な改善提案を添えています。
※自身のPRのため、Approveの代わりにCommentとして投稿します。
| /// vertices never collide. | ||
| private func key(_ v: Vertex3D) -> [Int] { | ||
| [v.x, v.y, v.z].map { Int(($0 * 1_000_000).rounded()) } | ||
| } |
There was a problem hiding this comment.
辞書のキーとして [Int] を使用する代わりに、専用の Hashable な struct を定義することを検討してください。これにより、意図が明確になり、配列の動的確保（ヒープ割り当て）も避けられます。
private struct QuantizedVertex: Hashable {
let x, y, z: Int
}
private func key(_ v: Vertex3D) -> QuantizedVertex {
QuantizedVertex(
x: Int((v.x * 1_000_000).rounded()),
y: Int((v.y * 1_000_000).rounded()),
z: Int((v.z * 1_000_000).rounded())
)
}There was a problem hiding this comment.
ご提案どおり専用の Hashable struct QuantizedVertex に置き換えました。意図が明確になり、辺ごとの [Int] 配列確保も しています。ad0f3af
| // triangles, i.e. a vertex of the Goldberg dual. A closed manifold | ||
| // makes every triangle share all 3 of its edges, so each centroid is | ||
| // trivalent. 80 hubs × 3 / 2 = 120 struts — consistent with edgeCount. | ||
| // A broken face list or non-manifold subdivision would drop a hub or |
There was a problem hiding this comment.
flatMap を使用して中間配列を作成する代わりに、reduce で直接集計することで、不要な中間ステップを排除できます。これはプロジェクトの「No Unnecessary Intermediate Steps」という規約により適合します。
let degree = GeodesicGeometry.edges
.reduce(into: [[Int]: Int]()) { dict, edge in
dict[key(edge.0), default: 0] += 1
dict[key(edge.1), default: 0] += 1
}There was a problem hiding this comment.
flatMap の中間配列を し、両端点を1つの
reduce(into:) に直接畳み込みました。ad0f3af
Replace the [Int] dictionary key with a dedicated Hashable QuantizedVertex struct (clearer intent, no per-edge array allocation) and fold the degree count directly into one reduce, dropping the intermediate flatMap array.
#287 で抽出した
GeodesicGeometryのテストを補強する。背景
既存テストは strut が 120 本であることだけを固定していた。しかし面リストや細分化が壊れても、本数 120 を保ったまま双対のトポロジーだけが崩れるケースは検出できない。
変更
各 strut 端点（= icosphere 三角形の centroid = Goldberg 双対の頂点）の接続次数を数え、不変条件を1つ追加する。
80 × 3 / 2 = 120で既存の 120-edge テストと整合座標は値コピーで再利用されるため、
1e-6量子化キーで同一 centroid を1つの identity に畳む（最近接 centroid 間距離 ≈ 0.36 ≫ 1e-6 なので衝突しない）。Foundation 非依存。補足
双対の「12 pentagon + 30 hexagon」はケージの面の話で、strut 端点（頂点）は一様に次数3。エッジリストから面を循環抽出するのは過剰なので、頂点レベルで効く三価性で構造を固定した。
swift test --filter GeodesicGeometryTests→ 4 件すべてパス。Summary by CodeRabbit
Release Notes
Chores
Tests