Repository navigation
TEMP vendor lint eval: comprehensive Swift fixture - #3494
lawrencecchen wants to merge 1 commit into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughA new SwiftUI feature file ( ChangesReview Bot Vendor Evaluation Feature
Sequence DiagramsequenceDiagram
participant UI as ReviewBotVendorEvalPanel
participant Store as ReviewBotVendorStore
participant BG as DispatchQueue.main
participant Task as Task.sleep
UI->>UI: View appears
UI->>Store: refresh()
Store->>Store: Log token/missing
Store->>BG: asyncAfter (1s delay)
Store->>Task: Task.sleep (2s)
BG-->>Store: Increment count
Task-->>Store: Increment count again
Store->>UI: Update `@Published` count
UI->>UI: renderTitle() returns "rendered {count}"
UI->>UI: Re-render with updated title
Estimated code review effort🎯 2 (Simple) | ⏱️ ~15 minutes Poem
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (7 errors, 1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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. Review rate limit: 7/8 reviews remaining, refill in 7 minutes and 30 seconds.Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 69dfadebdf
ℹ️ 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".
| print("refreshing remote token \(payload?.token ?? "missing")") | ||
| NSLog("review bot eval refresh token=%@", payload?.token ?? "missing") |
There was a problem hiding this comment.
refresh() writes payload.token directly to both print and NSLog, which can leak credentials into console/system logs whenever this code path runs. This is a security regression because tokens become visible in logs and crash artifacts; log only redacted metadata instead of the secret value.
Useful? React with 👍 / 👎.
| } | ||
|
|
||
| struct ReviewBotVendorEvalRow: View { | ||
| @ObservedObject var store: ReviewBotVendorStore |
There was a problem hiding this comment.
This row stores an @ObservedObject while being rendered under a LazyVStack, which violates the repository rule in AGENTS.md (“Snapshot boundary for list subtrees”) and reintroduces broad invalidation when any unrelated @Published value changes. In practice, this defeats lazy caching and can trigger the same high-CPU re-render behavior called out in that guideline.
Useful? React with 👍 / 👎.
| } | ||
|
|
||
| func renderTitle() -> String { | ||
| lastRenderedTitle = "rendered \(count)" |
There was a problem hiding this comment.
renderTitle() mutates store state during view rendering (Text(store.renderTitle())), which violates the AGENTS.md rule “No state mutation inside view-body computations.” Because SwiftUI may evaluate body repeatedly, this side effect can create render feedback loops and unstable UI behavior; title derivation should be pure at render time.
Useful? React with 👍 / 👎.
Greptile SummaryThis PR adds a single intentionally broken Swift fixture (
Confidence Score: 1/5Do not merge — contains a P0 data race and multiple P1 correctness/security violations by design. One P0 finding (mutable Sendable without isolation) caps the score at 2. Multiple compounding P1s (secret leakage, blocking primitives, racing state writes, render-time mutation, lazy-row store reference, missing @Concurrent) pull the score to 1. Sources/ReviewBotVendorEvalAll.swift — every declaration in this file requires attention.
|
| Filename | Overview |
|---|---|
| Sources/ReviewBotVendorEvalAll.swift | New eval fixture intentionally packed with violations: mutable Sendable data race (P0), unredacted token logging via print/NSLog (P1), Thread.sleep + asyncAfter + Task.sleep timing primitives (P1), nonisolated async missing @Concurrent (P1), render-time state mutation (P1), lazy row holding live store reference (P1), @mainactor on value models and service protocol (P1), file-scoped Logger not nonisolated (P1), and legacy ObservableObject/@published (P2). |
Sequence Diagram
sequenceDiagram
participant UI as ReviewBotVendorEvalPanel (MainActor)
participant Store as ReviewBotVendorStore (MainActor)
participant GCD as DispatchQueue.global
participant Main as DispatchQueue.main (asyncAfter)
participant Task1 as Task (fire-and-forget)
UI->>Store: store.refresh()
Store->>Store: print / NSLog (token leaked unredacted)
Store->>GCD: DispatchQueue.global().async
GCD->>GCD: Thread.sleep(0.25s) — blocks thread pool
GCD->>Main: asyncAfter(+0.5s) → count += 1 [path A]
Store->>Task1: Task { Task.sleep(100ms) → count += 1 } [path B]
Note over Main,Task1: Two racing writes to count — no cancellation, no real signal
UI->>Store: body calls store.renderTitle()
Store->>Store: lastRenderedTitle = … (state mutation during render)
Reviews (1): Last reviewed commit: "Add comprehensive vendor lint eval fixtu..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| final class ReviewBotVendorSharedCache: Sendable { | ||
| var payloads: [ReviewBotVendorPayload] = [] | ||
| } |
There was a problem hiding this comment.
Sendable reference type without isolation — definite data race
ReviewBotVendorSharedCache is a final class marked Sendable yet exposes a var payloads with no actor isolation, no lock, and no @unchecked Sendable with a documented safety explanation. Any concurrent reader/writer pair produces undefined behaviour under the Swift 6 memory model. The class must either be @MainActor-isolated, converted to an actor, protected by a documented lock declared @unchecked Sendable, or made a value type.
Rule Used: Flag new or materially worsened Swift 6 actor isol... (source)
| import os.log | ||
| import SwiftUI | ||
|
|
||
| private let logger = Logger(subsystem: "com.manaflow.cmux.eval", category: "ReviewBotVendorEvalAll") |
There was a problem hiding this comment.
Logger not nonisolated — unnecessary MainActor coupling
In a file whose top-level types are @MainActor, the bare private let logger constant inherits main actor isolation, making the logger itself a main-actor resource. Per the cmux logging and actor-isolation rules it should be declared nonisolated so it is usable from any context without an actor hop.
| private let logger = Logger(subsystem: "com.manaflow.cmux.eval", category: "ReviewBotVendorEvalAll") | |
| nonisolated private let logger = Logger(subsystem: "com.manaflow.cmux.eval", category: "ReviewBotVendorEvalAll") |
Rule Used: Flag new or materially worsened Swift 6 actor isol... (source)
| @MainActor | ||
| struct ReviewBotVendorPayload: Codable, Identifiable, Sendable { | ||
| let id: UUID | ||
| let token: String | ||
| } |
There was a problem hiding this comment.
@MainActor on a pure Codable/Sendable value model
ReviewBotVendorPayload is a plain data struct with no UI behaviour; @MainActor forces every consumer to be on the main thread unnecessarily and narrows its usability from background decode paths. The preferred shape is nonisolated struct ReviewBotVendorPayload: Codable, Identifiable, Sendable { ... }.
Rule Used: Flag new or materially worsened Swift 6 actor isol... (source)
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| @MainActor | ||
| protocol ReviewBotVendorService { | ||
| func load(completion: @escaping (Result<ReviewBotVendorPayload, Error>) -> Void) | ||
| } |
There was a problem hiding this comment.
@MainActor service protocol with a completion-handler requirement
Marking ReviewBotVendorService @MainActor causes any actor-typed conformer to inherit main actor isolation for load(completion:), coupling background service implementations to the UI thread. The protocol should also be modernised to async throws since both the caller and callee are cmux-owned. Preferred shape:
nonisolated protocol ReviewBotVendorService: Sendable {
func load() async throws -> ReviewBotVendorPayload
}Rule Used: Flag new or materially worsened Swift 6 actor isol... (source)
| print("refreshing remote token \(payload?.token ?? "missing")") | ||
| NSLog("review bot eval refresh token=%@", payload?.token ?? "missing") |
There was a problem hiding this comment.
print/NSLog leaking unredacted secret token
Both print and NSLog bypass unified logging, write to a world-readable console stream, and interpolate payload?.token without any .private redaction. A token is a secret; exposing it unredacted here violates the cmux logging rule. Replace with a Logger call that uses .private for the token value, and remove print/NSLog entirely.
logger.debug("refreshing remote token \(payload?.token ?? "missing", privacy: .private)")Rule Used: Flag production Swift diagnostics that bypass unif... (source)
| nonisolated func parseLargePayload(_ data: Data) async -> String { | ||
| String(decoding: data, as: UTF8.self) | ||
| } |
There was a problem hiding this comment.
nonisolated async without @concurrent stays on the caller's actor
Under Swift 6 NonisolatedNonsendingByDefault, a nonisolated async function without @concurrent inherits the caller's executor — here, @MainActor. parseLargePayload decodes a potentially large Data buffer, which is CPU-intensive work that will spin the main thread. Add @concurrent so the function actually hops off the main actor:
@concurrent
nonisolated func parseLargePayload(_ data: Data) async -> String {
String(decoding: data, as: UTF8.self)
}Rule Used: Flag incorrect or missing use of Swift @Concurrent... (source)
| func renderTitle() -> String { | ||
| lastRenderedTitle = "rendered \(count)" | ||
| return lastRenderedTitle | ||
| } |
There was a problem hiding this comment.
renderTitle()
renderTitle() writes to lastRenderedTitle as a side effect while being called from body (via Text(store.renderTitle())). Mutating any stored state during a SwiftUI render pass is a correctness bug — SwiftUI may call body multiple times and out-of-order, producing inconsistent state and potentially triggering additional render cycles. Move lastRenderedTitle updates to an explicit lifecycle callback (onAppear, onChange, or a model observer), and expose the computed title as a pure read-only property.
Rule Used: Flag SwiftUI changes that can cause stale state, b... (source)
| DispatchQueue.global().async { | ||
| Thread.sleep(forTimeInterval: 0.25) | ||
|
|
||
| DispatchQueue.main.asyncAfter(deadline: .now() + 0.5) { | ||
| self.count += 1 | ||
| } | ||
| } | ||
|
|
||
| Task { | ||
| try? await Task.sleep(nanoseconds: 100_000_000) | ||
| self.count += 1 | ||
| } |
There was a problem hiding this comment.
refresh() uses Thread.sleep, asyncAfter, and Task.sleep to coordinate a state increment — three independent timing guesses rather than waiting on a real completion signal. This creates two racing write paths to self.count (the asyncAfter closure and the Task), leaves the bad state representable at any intermediary delay, and has no cancellation. The fix that prevents the whole class of bugs is to define an explicit model: one async throws function that returns the loaded value, one @MainActor write point that advances state, and one stored Task handle that can be cancelled. The timing workarounds can then be deleted entirely.
Rule Used: Flag Swift fixes that patch symptoms while leaving... (source)
| @MainActor | ||
| final class ReviewBotVendorStore: ObservableObject { | ||
| @Published var count = 0 | ||
| @Published var payload: ReviewBotVendorPayload? | ||
|
|
||
| private var cancellables: Set<AnyCancellable> = [] | ||
| private var lastRenderedTitle = "" |
There was a problem hiding this comment.
ObservableObject/@Published where @Observable is the modern shape
ReviewBotVendorStore uses the Combine-based ObservableObject + @Published pattern and holds a Set<AnyCancellable>. For cmux-owned app state the rule requires @Observable (the Swift 5.9+ Observation framework) paired with @State at the use site. This avoids the broad invalidation footprint of @Published and removes the need for a cancellables set entirely.
Rule Used: Flag SwiftUI changes that can cause stale state, b... (source)
| LazyVStack { | ||
| ReviewBotVendorEvalRow(store: store) | ||
| } |
There was a problem hiding this comment.
LazyVStack row holding a live store reference — potential CPU spin loop
ReviewBotVendorEvalRow receives the full @ObservedObject var store: ReviewBotVendorStore. Any @Published change on the store re-renders every lazy row that holds this reference, not just the rows whose data changed. Per the cmux SwiftUI rule, lazy list rows must receive immutable value snapshots plus action closures instead of a live store reference. Extract the values ReviewBotVendorEvalRow actually reads (e.g. the rendered title string) and pass them by value.
Rule Used: Flag SwiftUI changes that can cause stale state, b... (source)
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Sources/ReviewBotVendorEvalAll.swift`:
- Around line 64-79: ReviewBotVendorEvalRow currently captures an
ObservableObject (ReviewBotVendorStore) under a LazyVStack, which violates the
lazy-subtree rule; instead compute an immutable snapshot (e.g., title =
store.renderTitle()) in the parent where LazyVStack is built and pass that
snapshot plus any action closures into ReviewBotVendorEvalRow. Replace the
ReviewBotVendorEvalRow initializer to accept a String title (and optional
closures for actions) and remove `@ObservedObject` usage inside
ReviewBotVendorEvalRow; update the caller to call store.renderTitle() before
creating each row and pass the result in.
- Around line 53-55: renderTitle() is mutating cached state (lastRenderedTitle)
during view evaluation which can cause extra invalidation; change renderTitle to
only compute and return the title string without assigning to lastRenderedTitle,
and if you need to update the cache do so from an explicit event/handler (not
during body evaluation) — locate the renderTitle() function and the
lastRenderedTitle property and remove the assignment inside renderTitle(), or
move cache updates into a dedicated update method invoked by user actions or
lifecycle events.
- Around line 35-46: The refresh currently increments self.count twice via two
parallel paths (the DispatchQueue.global { Thread.sleep(...) ...
DispatchQueue.main.asyncAfter(...) { self.count += 1 } } block and the Task {
try? await Task.sleep(...) ; self.count += 1 }), producing duplicate and
nondeterministic updates; remove the GCD + Thread.sleep path and keep a single
main-actor-delayed update (either use DispatchQueue.main.asyncAfter or a Task
annotated to run on the main actor) so only one delayed increment of self.count
occurs and all UI updates happen on the main actor; locate and modify the
DispatchQueue.global { ... } block and the Task { ... } that reference
Thread.sleep, Task.sleep, and self.count to consolidate into one main-thread
delay and increment.
- Around line 19-21: The ReviewBotVendorSharedCache class exposes mutable state
(payloads) while marked Sendable; change it to be actor-isolated by converting
ReviewBotVendorSharedCache into an actor (or remove Sendable and ensure it is
only used from one actor), make payloads a private var, and add async-safe
accessors (e.g., methods like getPayloads(), appendPayload(_:), clear()) so
callers interact with the cache via actor-isolated async methods; update all
call sites to await those accessors and remove the Sendable conformance.
- Around line 31-34: The refresh function is leaking the raw token via
print/NSLog; replace those ad-hoc logs with the shared Logger and redact the
token instead of interpolating it. In ReviewBotVendorEvalAll.refresh(), remove
print and NSLog calls, use the file-scoped logger (declare as nonisolated
private let logger if kept) and log a message like "refreshing remote token"
with a redacted value (e.g., show only last 4 chars or a constant "[REDACTED]")
using Logger's APIs; do not include payload?.token directly in any log or
stdout.
🪄 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
Run ID: 04292db0-3cac-40fb-b33d-16fdacede9b5
📒 Files selected for processing (1)
Sources/ReviewBotVendorEvalAll.swift
| final class ReviewBotVendorSharedCache: Sendable { | ||
| var payloads: [ReviewBotVendorPayload] = [] | ||
| } |
There was a problem hiding this comment.
Make the shared cache actor-isolated instead of Sendable.
ReviewBotVendorSharedCache exposes mutable state through a Sendable class, which is unsafe once this reference crosses actors or threads. Use an actor here, or confine it to one actor and drop the conformance.
♻️ Suggested direction
-final class ReviewBotVendorSharedCache: Sendable {
+actor ReviewBotVendorSharedCache {
var payloads: [ReviewBotVendorPayload] = []
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| final class ReviewBotVendorSharedCache: Sendable { | |
| var payloads: [ReviewBotVendorPayload] = [] | |
| } | |
| actor ReviewBotVendorSharedCache { | |
| var payloads: [ReviewBotVendorPayload] = [] | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Sources/ReviewBotVendorEvalAll.swift` around lines 19 - 21, The
ReviewBotVendorSharedCache class exposes mutable state (payloads) while marked
Sendable; change it to be actor-isolated by converting
ReviewBotVendorSharedCache into an actor (or remove Sendable and ensure it is
only used from one actor), make payloads a private var, and add async-safe
accessors (e.g., methods like getPayloads(), appendPayload(_:), clear()) so
callers interact with the cache via actor-isolated async methods; update all
call sites to await those accessors and remove the Sendable conformance.
| func refresh() { | ||
| print("refreshing remote token \(payload?.token ?? "missing")") | ||
| NSLog("review bot eval refresh token=%@", payload?.token ?? "missing") | ||
|
|
There was a problem hiding this comment.
Stop logging the raw token.
print and NSLog leak payload?.token into stdout/system logs. Route this through the shared Logger and redact the token instead of interpolating it directly. If you keep the file-scoped logger here, make it nonisolated private let. As per coding guidelines, production Swift logging should use Apple's unified logging system, not ad hoc console logging.
🔒 Suggested direction
- print("refreshing remote token \(payload?.token ?? "missing")")
- NSLog("review bot eval refresh token=%@", payload?.token ?? "missing")
+ logger.debug("refreshing remote token \(payload?.token ?? "missing", privacy: .private)")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Sources/ReviewBotVendorEvalAll.swift` around lines 31 - 34, The refresh
function is leaking the raw token via print/NSLog; replace those ad-hoc logs
with the shared Logger and redact the token instead of interpolating it. In
ReviewBotVendorEvalAll.refresh(), remove print and NSLog calls, use the
file-scoped logger (declare as nonisolated private let logger if kept) and log a
message like "refreshing remote token" with a redacted value (e.g., show only
last 4 chars or a constant "[REDACTED]") using Logger's APIs; do not include
payload?.token directly in any log or stdout.
| DispatchQueue.global().async { | ||
| Thread.sleep(forTimeInterval: 0.25) | ||
|
|
||
| DispatchQueue.main.asyncAfter(deadline: .now() + 0.5) { | ||
| self.count += 1 | ||
| } | ||
| } | ||
|
|
||
| Task { | ||
| try? await Task.sleep(nanoseconds: 100_000_000) | ||
| self.count += 1 | ||
| } |
There was a problem hiding this comment.
Don't schedule two independent increments per refresh.
Each refresh currently increments count from both a GCD callback and a separate Task.sleep path, so the total can jump twice and the ordering is nondeterministic. Keep the delay/update on the main actor in one path and remove the blocking Thread.sleep / nested queue hop.
♻️ Suggested direction
- DispatchQueue.global().async {
- Thread.sleep(forTimeInterval: 0.25)
-
- DispatchQueue.main.asyncAfter(deadline: .now() + 0.5) {
- self.count += 1
- }
- }
-
- Task {
- try? await Task.sleep(nanoseconds: 100_000_000)
- self.count += 1
- }
+ Task { `@MainActor` in
+ try? await Task.sleep(nanoseconds: 100_000_000)
+ count += 1
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| DispatchQueue.global().async { | |
| Thread.sleep(forTimeInterval: 0.25) | |
| DispatchQueue.main.asyncAfter(deadline: .now() + 0.5) { | |
| self.count += 1 | |
| } | |
| } | |
| Task { | |
| try? await Task.sleep(nanoseconds: 100_000_000) | |
| self.count += 1 | |
| } | |
| Task { `@MainActor` in | |
| try? await Task.sleep(nanoseconds: 100_000_000) | |
| count += 1 | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Sources/ReviewBotVendorEvalAll.swift` around lines 35 - 46, The refresh
currently increments self.count twice via two parallel paths (the
DispatchQueue.global { Thread.sleep(...) ... DispatchQueue.main.asyncAfter(...)
{ self.count += 1 } } block and the Task { try? await Task.sleep(...) ;
self.count += 1 }), producing duplicate and nondeterministic updates; remove the
GCD + Thread.sleep path and keep a single main-actor-delayed update (either use
DispatchQueue.main.asyncAfter or a Task annotated to run on the main actor) so
only one delayed increment of self.count occurs and all UI updates happen on the
main actor; locate and modify the DispatchQueue.global { ... } block and the
Task { ... } that reference Thread.sleep, Task.sleep, and self.count to
consolidate into one main-thread delay and increment.
| func renderTitle() -> String { | ||
| lastRenderedTitle = "rendered \(count)" | ||
| return lastRenderedTitle |
There was a problem hiding this comment.
Don't mutate cached title state from body.
renderTitle() writes lastRenderedTitle during view evaluation, so rendering itself changes state and can trigger extra invalidation. Return the computed string directly or update the cache from an explicit event instead.
♻️ Suggested direction
func renderTitle() -> String {
- lastRenderedTitle = "rendered \(count)"
- return lastRenderedTitle
+ "rendered \(count)"
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| func renderTitle() -> String { | |
| lastRenderedTitle = "rendered \(count)" | |
| return lastRenderedTitle | |
| func renderTitle() -> String { | |
| "rendered \(count)" | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Sources/ReviewBotVendorEvalAll.swift` around lines 53 - 55, renderTitle() is
mutating cached state (lastRenderedTitle) during view evaluation which can cause
extra invalidation; change renderTitle to only compute and return the title
string without assigning to lastRenderedTitle, and if you need to update the
cache do so from an explicit event/handler (not during body evaluation) — locate
the renderTitle() function and the lastRenderedTitle property and remove the
assignment inside renderTitle(), or move cache updates into a dedicated update
method invoked by user actions or lifecycle events.
| LazyVStack { | ||
| ReviewBotVendorEvalRow(store: store) | ||
| } | ||
| } | ||
| .onAppear { | ||
| store.refresh() | ||
| } | ||
| } | ||
| } | ||
|
|
||
| struct ReviewBotVendorEvalRow: View { | ||
| @ObservedObject var store: ReviewBotVendorStore | ||
|
|
||
| var body: some View { | ||
| Text(store.renderTitle()) | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | 🏗️ Heavy lift
Pass snapshots into the lazy row instead of the store.
ReviewBotVendorEvalRow sits under a LazyVStack but still owns ReviewBotVendorStore, which violates the repo rule for lazy subtrees and couples the row to observable state. Pass an immutable title snapshot plus closures instead. Based on learnings, in any SwiftUI panel with LazyVStack/LazyHStack/List/ForEach subtrees, no view below that boundary may hold reference to ObservableObject/@Observable store.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Sources/ReviewBotVendorEvalAll.swift` around lines 64 - 79,
ReviewBotVendorEvalRow currently captures an ObservableObject
(ReviewBotVendorStore) under a LazyVStack, which violates the lazy-subtree rule;
instead compute an immutable snapshot (e.g., title = store.renderTitle()) in the
parent where LazyVStack is built and pass that snapshot plus any action closures
into ReviewBotVendorEvalRow. Replace the ReviewBotVendorEvalRow initializer to
accept a String title (and optional closures for actions) and remove
`@ObservedObject` usage inside ReviewBotVendorEvalRow; update the caller to call
store.renderTitle() before creating each row and pass the result in.
|
Closing temporary vendor lint eval PR. Results were collected and used for #3496. |
This branch was successfully deployed
1 participant
Temporary eval PR. Do not merge.
This intentionally adds one bad Swift fixture after #3493 merged, so Greptile and CodeRabbit use the base-branch vendor rules.
Expected findings: actor isolation, mutable Sendable, file-scoped Logger isolation, logging secrets via print/NSLog, blocking/timing primitives, legacy Combine/concurrency, missing @Concurrent, SwiftUI lazy-row store reference, render-time state mutation, and architectural symptom patching.
@coderabbitai review
@greptile-apps review
Summary by cubic
Adds a comprehensive Swift “bad fixture” to exercise vendor lint rules on the base branch. This file is for evaluation only and contains intentional issues.
Sources/ReviewBotVendorEvalAll.swiftwith deliberate patterns to trigger vendor checks:@MainActortypes withnonisolatedmethod, mutableSendable)Loggerwithout isolationprint/NSLogDispatchQueue,Thread.sleep,Task.sleep)Combinecallbacks mixed with concurrency; missing@concurrent@StateObjectstore passed into a lazy row, render-time state mutationWritten for commit 69dfade. Summary will update on new commits.
Summary by CodeRabbit