Skip to content

fix: return after peer color cache hit - #1640

Open
Chessing234 wants to merge 1 commit into
permissionlesstech:mainfrom
Chessing234:fix-peer-color-cache
Open

fix: return after peer color cache hit#1640
Chessing234 wants to merge 1 commit into
permissionlesstech:mainfrom
Chessing234:fix-peer-color-cache

Conversation

@Chessing234

Copy link
Copy Markdown
Contributor

peer color cache checked for a hit but never returned, so every call recomputed the color and rewrote the same cache entry. added a debug-only counter so a test can actually assert the cache short-circuits.

Color(peerSeed:isDark:) checked the cache but never returned on a hit,
so every call recomputed the hue/saturation/brightness math and
overwrote the cache entry with an identical value. The cache never
actually short-circuited anything.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 16e0a3d090

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

import SwiftUI
@testable import bitchat

struct ColorPeerTests {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Serialize tests using the shared color counter

Because this Swift Testing suite is not marked serialized, its @Test methods can run concurrently under the repo's swift test --parallel CI path, but they all assert deltas against the same process-global Color._peerColorComputeCountForTesting. If differentAppearanceForSameSeedIsNotCachedTogether increments the counter between this test's reads, afterSecond == afterFirst can fail even when the cache behavior is correct; make this suite/test serialized or avoid assertions based on a shared global counter.

Useful? React with 👍 / 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant