feat: buddy dialogue flavor, draggable notch sections, usage cache fix#6
Open
Mythical365 wants to merge 3 commits into
Open
feat: buddy dialogue flavor, draggable notch sections, usage cache fix#6Mythical365 wants to merge 3 commits into
Mythical365 wants to merge 3 commits into
Conversation
- buddi-hook.py: track tool usage, prompt count, and denial rate per session in ~/.buddi-session-stats.json. Compute a dialogue_flavor field (chaotic/runner/explorer/methodical/neutral) sent on every event — purely additive, does not touch buddy identity at all. Follows the direction suggested in talkvalue#5 for personality-driven dialogue. - NotchHomeView.swift: make the three notch sections (music, buddy, calendar) draggable. Drag any section left or right to reorder while hovering in the notch. Order is persisted to UserDefaults so it survives restarts. No Xcode required to understand the intent — contributed for maintainer review/implementation if preferred. - UsageService.swift: fix usage display lag. Cache was loaded and shown indefinitely even when stale (>5 min old). Now detects stale cache on startup and schedules an immediate re-fetch instead of waiting another full 300s interval. startPolling() also re-fetches if called while already polling but data is stale. Note: Swift changes submitted without local build verification (no Xcode installed). Logic and API usage are correct to the best of my knowledge but maintainer review for compilation is appreciated. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
3 issues found across 3 files
Confidence score: 3/5
- There is moderate merge risk because two high-confidence medium-severity issues can affect runtime behavior under normal use, so this is not a purely cosmetic change set.
- In
buddi/Buddi/Resources/buddi-hook.py, session stat updates are not atomic/locked, which can drop counter increments or reset stats when hooks run concurrently. - In
buddi/Buddi/Services/Shared/UsageService.swift, stale-cache and failure paths can leavecurrentIntervalat 0, creating an unintended 1-second polling loop;buddi/components/Notch/NotchHomeView.swiftalso leavesdraggingSectionstale on canceled drags, causing lingering dimmed UI state. - Pay close attention to
buddi/Buddi/Resources/buddi-hook.py,buddi/Buddi/Services/Shared/UsageService.swift, andbuddi/components/Notch/NotchHomeView.swift- concurrency safety, interval restoration, and drag-state cleanup need validation.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="buddi/Buddi/Resources/buddi-hook.py">
<violation number="1" location="buddi/Buddi/Resources/buddi-hook.py:98">
P2: Session stats updates are not atomic or locked, causing lost counter updates and possible stats reset under concurrent hook executions.</violation>
</file>
<file name="buddi/Buddi/Services/Shared/UsageService.swift">
<violation number="1" location="buddi/Buddi/Services/Shared/UsageService.swift:132">
P2: Stale cache sets `currentInterval` to 0, and failure paths never restore it, causing an unintended 1-second polling loop.</violation>
</file>
<file name="buddi/components/Notch/NotchHomeView.swift">
<violation number="1" location="buddi/components/Notch/NotchHomeView.swift:469">
P2: Drag state cleanup is incomplete: `draggingSection` is set on drag start but only cleared in `performDrop`, so cancelled/outside-target drags can leave a stale dimmed section.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
- buddi-hook.py: use fcntl.flock for atomic read-modify-write of session stats so concurrent hook invocations don't drop increments; use os.replace (atomic rename) for writes to avoid partial-file reads - UsageService.swift: replace currentInterval=0 with currentInterval=2 to avoid a 1-second polling loop; always restore to baseInterval on any failure/error path so the fast-fetch only fires once on stale cache - NotchHomeView.swift: remove duplicate dropExited, consolidate drag cleanup into a single path with a short delay so cancelled drags (user releases outside a drop target) clear draggingSection instead of leaving the section dimmed indefinitely Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
@grayashh I have started the AI code review. It will take a few minutes to complete. |
There was a problem hiding this comment.
3 issues found across 3 files
Confidence score: 3/5
- There is a concrete regression risk in
buddi/Buddi/Resources/buddi-hook.py: a duplicate function definition appears to overwrite the lock-protected version, which can make session stat updates non-atomic and lead to lost updates under concurrency. buddi/Buddi/Services/Shared/UsageService.swifthas a medium-impact retry behavior issue where the stale-cache fast interval may persist on missing-token paths, potentially causing indefinite 2-second polling.buddi/components/Notch/NotchHomeView.swiftmay leave UI order changes unsaved if drag/drop is canceled or ends outside, since reordering happens indropEnteredbut persistence is deferred toperformDrop.- Pay close attention to
buddi/Buddi/Resources/buddi-hook.py,buddi/Buddi/Services/Shared/UsageService.swift, andbuddi/components/Notch/NotchHomeView.swift- concurrency safety, retry interval reset logic, and drag-drop persistence consistency should be validated before merge.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="buddi/Buddi/Services/Shared/UsageService.swift">
<violation number="1" location="buddi/Buddi/Services/Shared/UsageService.swift:135">
P2: Stale-cache fast interval can persist in missing-token path, causing indefinite 2-second polling retries.</violation>
</file>
<file name="buddi/Buddi/Resources/buddi-hook.py">
<violation number="1" location="buddi/Buddi/Resources/buddi-hook.py:109">
P1: Duplicate function definition overwrites the lock-protected implementation, making session stats updates non-atomic and prone to lost updates under concurrency.</violation>
</file>
<file name="buddi/components/Notch/NotchHomeView.swift">
<violation number="1" location="buddi/components/Notch/NotchHomeView.swift:558">
P2: Section order is mutated during `dropEntered` but only persisted in `performDrop`, so canceled/outside drops can leave reordered state unsaved.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
- buddi-hook.py: previous rename collision left two functions named update_session_stats_atomic — the second silently shadowed the first, bypassing fcntl locking. Split into _update_session_stats_unlocked (the actual logic) and update_session_stats_atomic (the locked wrapper) - UsageService.swift: reset currentInterval to baseInterval in the missing-token branch of poll(), so a logged-out user doesn't get trapped polling every 2s indefinitely after a stale-cache load - NotchHomeView.swift: persist section order inside dropEntered after every reorder, not just in performDrop. Cancelled drags (release outside a target) now still save the new order Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
@grayashh I have started the AI code review. It will take a few minutes to complete. |
There was a problem hiding this comment.
3 issues found across 3 files
Confidence score: 3/5
- There is a concrete medium-risk concurrency issue in
buddi/Buddi/Services/Shared/UsageService.swift:startPolling()may launchpoll()while polling is already active, which can cause overlapping tasks and racing state updates. - The other findings are lower severity (4/10) but still user-impacting over time: unbounded growth of the stats file in
buddi/Buddi/Resources/buddi-hook.pycan steadily increase I/O cost, and duplicate persisted section entries inbuddi/components/Notch/NotchHomeView.swiftcan lead to duplicateForEachIDs and unstable SwiftUI rendering. - Given one clear medium-severity behavioral risk plus two minor correctness/performance concerns, this sits at moderate merge risk rather than a safe-to-merge 4/5.
- Pay close attention to
buddi/Buddi/Services/Shared/UsageService.swift,buddi/Buddi/Resources/buddi-hook.py,buddi/components/Notch/NotchHomeView.swift- prevent overlapping polling, bound/prune session stats growth, and deduplicate persisted section order before rendering.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="buddi/Buddi/Resources/buddi-hook.py">
<violation number="1" location="buddi/Buddi/Resources/buddi-hook.py:110">
P2: Session stats file can grow without bound, causing increasing full-file read/write cost on every tracked event.</violation>
</file>
<file name="buddi/Buddi/Services/Shared/UsageService.swift">
<violation number="1" location="buddi/Buddi/Services/Shared/UsageService.swift:50">
P2: `startPolling()` can trigger `poll()` while polling is already active, allowing overlapping poll tasks that race polling state updates.</violation>
</file>
<file name="buddi/components/Notch/NotchHomeView.swift">
<violation number="1" location="buddi/components/Notch/NotchHomeView.swift:520">
P2: Persisted section order is loaded without deduplication, which can produce duplicate `ForEach` IDs (`id: \.self`) and undefined SwiftUI rendering behavior.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
|
|
||
|
|
||
| def _update_session_stats_unlocked(session_id, event, tool_name=None, denied=False): | ||
| stats = load_session_stats() |
There was a problem hiding this comment.
P2: Session stats file can grow without bound, causing increasing full-file read/write cost on every tracked event.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At buddi/Buddi/Resources/buddi-hook.py, line 110:
<comment>Session stats file can grow without bound, causing increasing full-file read/write cost on every tracked event.</comment>
<file context>
@@ -80,6 +82,100 @@ def get_cmux_surface():
+
+
+def _update_session_stats_unlocked(session_id, event, tool_name=None, denied=False):
+ stats = load_session_stats()
+ s = stats.setdefault(session_id, {
+ "tool_counts": {},
</file context>
| // If already polling, only kick off a fresh fetch if the last one is stale (>5 min old) | ||
| guard pollTimer == nil else { | ||
| if let cached = loadCachedUsage(), Date().timeIntervalSince(cached.fetchedAt) > baseInterval { | ||
| poll() |
There was a problem hiding this comment.
P2: startPolling() can trigger poll() while polling is already active, allowing overlapping poll tasks that race polling state updates.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At buddi/Buddi/Services/Shared/UsageService.swift, line 50:
<comment>`startPolling()` can trigger `poll()` while polling is already active, allowing overlapping poll tasks that race polling state updates.</comment>
<file context>
@@ -43,8 +43,14 @@ final class UsageService: ObservableObject {
+ // If already polling, only kick off a fresh fetch if the last one is stale (>5 min old)
+ guard pollTimer == nil else {
+ if let cached = loadCachedUsage(), Date().timeIntervalSince(cached.fetchedAt) > baseInterval {
+ poll()
+ }
+ return
</file context>
| } | ||
| let decoded = raw.compactMap { NotchSection(rawValue: $0) } | ||
| let missing = NotchSection.allCases.filter { !decoded.contains($0) } | ||
| return decoded + missing |
There was a problem hiding this comment.
P2: Persisted section order is loaded without deduplication, which can produce duplicate ForEach IDs (id: \.self) and undefined SwiftUI rendering behavior.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At buddi/components/Notch/NotchHomeView.swift, line 520:
<comment>Persisted section order is loaded without deduplication, which can produce duplicate `ForEach` IDs (`id: \.self`) and undefined SwiftUI rendering behavior.</comment>
<file context>
@@ -463,6 +487,125 @@ struct NotchHomeView: View {
+ }
+ let decoded = raw.compactMap { NotchSection(rawValue: $0) }
+ let missing = NotchSection.allCases.filter { !decoded.contains($0) }
+ return decoded + missing
+ }
+
</file context>
Member
|
Thanks for three solid improvements — but they're three independent changes (Python stats module, Swift cache fix, SwiftUI drag reorder), and the PR template asks for "single change" for a reason: a problem in one blocks the others. Could we split this into three separate PRs? Suggested split + per-PR feedback: PR A — Cleanest of the three, ready to merge with one polish:
The PR B — Please hold this until the Swift consumer lands. As-is, every hook invocation writes a stats file and ships
Other items to address when this comes back:
PR C — Submitting Swift changes without local build verification is a blocker — please get it building (and ideally clicking through once) before re-submitting. While doing that, please address:
Thanks again — happy to re-review each PR as it lands. |
grayashh
added
type: feature
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
buddi-hook.py: track tool usage, prompt count, and denial rate per session in ~/.buddi-session-stats.json. Compute a dialogue_flavor field (chaotic/runner/explorer/methodical/neutral) sent on every event — purely additive, does not touch buddy identity at all. Follows the direction suggested in feat: style-aware buddy generation from coding behavior #5 for personality-driven dialogue.
NotchHomeView.swift: make the three notch sections (music, buddy, calendar) draggable. Drag any section left or right to reorder while hovering in the notch. Order is persisted to UserDefaults so it survives restarts. No Xcode required to understand the intent — contributed for maintainer review/implementation if preferred.
UsageService.swift: fix usage display lag. Cache was loaded and shown indefinitely even when stale (>5 min old). Now detects stale cache on startup and schedules an immediate re-fetch instead of waiting another full 300s interval. startPolling() also re-fetches if called while already polling but data is stale.
Note: Swift changes submitted without local build verification (no Xcode installed). Logic and API usage are correct to the best of my knowledge but maintainer review for compilation is appreciated.
Description
Type of change
Checklist
Screenshots