feat(history): unified history & recommendations window with embedded settings#12
Conversation
… settings Adds a dedicated NSWindow that opens on Spotlight launch (or app reopen) with a sidebar split between "Дані" (clipboard history, replacement history, recommendations) and "Налаштування" (existing General/Hotkeys/AutoFix/ Analytics/About tabs, now reused as sidebar sections instead of a TabView). The standalone Settings scene is removed; both menu-bar entries open the new window with a pre-selected section. New persistence layer: ReplacementHistoryStore (singleton, JSONL in Application Support, retention 1w/1m/3m/forever) records manual switches, AutoFix applications, and AutoFix undos. RecommendationEngine analyzes that history at Fibonacci thresholds (3, 5, 8, 13, 21) and surfaces three kinds of suggestions: add a word to the allowlist (frequently undone autofix), create a custom auto-replace rule (frequently performed manual switch), or add an app to the blocklist (many undos in one bundle). Accepted suggestions mutate the existing allowlist/blocklist or the new customAutoReplaceRules list, which AutoFixController consults before the language scorer so personal rules actually fire during typing. Stored text is truncated to 80 characters to limit accidental capture of passwords or long secrets. The whole feature is gated by a new "Зберігати історію замін" toggle in General settings. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
🔖 On merge this PR will release v1.4.0 ( |
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, 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 have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (7)
📝 WalkthroughWalkthroughThis PR introduces a replacement history and recommendations feature that tracks text replacements (auto-fixes and manual switches), persists them to disk with configurable retention, generates actionable recommendations based on usage patterns, and exposes all this through a new unified history window. Custom auto-replace rules are added and integrated into the auto-fix flow. ChangesReplacement History and Recommendations
Sequence DiagramssequenceDiagram
participant User
participant ReplacementHistoryStore
participant RecommendationEngine
participant HistoryWindowView
User->>HistoryWindowView: Open History Window
HistoryWindowView->>ReplacementHistoryStore: fetch entries
HistoryWindowView->>RecommendationEngine: compute(history, allowlist, rules, dismissed)
RecommendationEngine->>RecommendationEngine: aggregate patterns, apply thresholds
RecommendationEngine->>HistoryWindowView: return sorted recommendations
HistoryWindowView->>User: Display history + recommendations
User->>HistoryWindowView: Accept recommendation
HistoryWindowView->>Defaults: append to allowlist/blocklist/rules
HistoryWindowView->>Defaults: mark recommendation as dismissed
sequenceDiagram
participant AutoFixController
participant ReplacementHistoryStore
participant lastFix
participant Undo
AutoFixController->>AutoFixController: evaluateAndMaybeFix(word)
alt matches custom rule
AutoFixController->>AutoFixController: applyCustomRule(rule)
else standard path
AutoFixController->>AutoFixController: applyFix(replacement)
end
AutoFixController->>ReplacementHistoryStore: record(autoFixApplied entry)
AutoFixController->>lastFix: store replacement + layout IDs
User->>Undo: trigger undo
AutoFixController->>ReplacementHistoryStore: record(autoFixUndone entry with bundleID)
AutoFixController->>lastFix: restore original
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ 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 |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 630c49591e
ℹ️ 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".
| undoCount: count, | ||
| tier: tier(for: count) | ||
| ) | ||
| if dismissedSet.contains(rec.dedupKey) && count < fibonacciThresholds.last! { continue } |
There was a problem hiding this comment.
Suppress dismissed tier-5 recommendations
When a recommendation reaches the last threshold (21 or more repeats), dismissing it no longer has any effect because this check only skips dismissed items while count < 21; the same logic is repeated for rule and blocklist recommendations. In that scenario the user can click “Відхилити”, the dedup key is saved, but the card is recomputed and shown again immediately/forever because there is no higher tier to wait for.
Useful? React with 👍 / 👎.
| guard !customAutoReplaceRules.contains(where: { | ||
| $0.source.lowercased() == src.lowercased() | ||
| && $0.target.lowercased() == tgt.lowercased() | ||
| }) else { return } |
There was a problem hiding this comment.
Prevent conflicting custom rules with the same source
This duplicate check only rejects an identical source/target pair, so users can add another rule with the same source but a different target. At runtime AutoFixController applies Defaults[.customAutoReplaceRules].first(where: { $0.matches(word) }), so any later rule with the same source is saved and displayed but can never fire unless the earlier rule is deleted.
Useful? React with 👍 / 👎.
| if Defaults[.openHistoryOnAppLaunch] { | ||
| AppLogger.action(logger, "Opening history window on launch") | ||
| HistoryWindowController.shared.showHistory() |
There was a problem hiding this comment.
Defer launch history until managers are configured
For returning users with openHistoryOnAppLaunch enabled (the new default), this opens the history window from applicationDidFinishLaunching, but the delegate's layoutManager and clipboardHistoryManager are only populated by PapugaApp.configureIfNeeded() from the MenuBarExtra .onAppear. If this path runs before that appearance callback, historyRootView constructs HistoryWindowView without the required LayoutManager/ClipboardHistoryManager environment values, and the initial clipboard section can hit SwiftUI's missing-environment runtime crash on launch.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (1)
papuga/Views/Settings/AutoFixTab.swift (1)
125-131: ⚡ Quick winPrevent conflicting rules with the same source text.
Current duplicate filtering blocks only identical
(source,target)pairs, so one source can still map to multiple targets. That makes runtime behavior order-dependent and confusing.Proposed fix
- guard !customAutoReplaceRules.contains(where: { - $0.source.lowercased() == src.lowercased() - && $0.target.lowercased() == tgt.lowercased() - }) else { return } + guard !customAutoReplaceRules.contains(where: { + $0.source.lowercased() == src.lowercased() + }) else { return }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@papuga/Views/Settings/AutoFixTab.swift` around lines 125 - 131, The current guard only prevents identical (source,target) pairs but allows the same source to map to multiple targets; update the uniqueness check on customAutoReplaceRules so it rejects any existing rule whose source.lowercased() == src.lowercased() (regardless of target) before appending a new CustomAutoReplaceRule(source: src, target: tgt); modify the guard that currently uses contains(where:) to return early when any rule's source matches the new src (use the same $0.source.lowercased() and src.lowercased() comparisons to locate the code).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@papuga/Core/HistoryWindowController.swift`:
- Around line 124-131: The current fallback branch instantiates
HistoryWindowView without required environment dependencies (layoutManager and
clipboardHistoryManager) which can crash; change the logic so HistoryWindowView
is only created when both AppDelegate.shared?.layoutManager and
AppDelegate.shared?.clipboardHistoryManager are non-nil (the existing if-let
branch), and remove the else that builds the view without environments; instead
return a safe fallback (e.g., an EmptyView or a small error/placeholder view)
and/or log the missing dependencies via AppDelegate or a logger so the missing
layoutManager/clipboardHistoryManager are detected without crashing (refer to
HistoryWindowView, AppDelegate.shared, layoutManager, clipboardHistoryManager,
initialSection).
In `@papuga/Core/RecommendationEngine.swift`:
- Around line 75-77: The aggregation uses raw bundleID as the key
(undoneByBundle[bundleID,...]) while recommendation identity uses lowercased
keys, causing split counts; normalize bundleID (e.g., let normalized =
bundleID.lowercased()) before using it as a dictionary key in the undoneByBundle
aggregation and in the other aggregation block around lines 121-130, and ensure
the same normalization is applied wherever dedupKey/id are formed so keys remain
consistent across counting and identity.
In `@papuga/Core/ReplacementHistoryStore.swift`:
- Around line 184-185: The assignments to keptLines and keptEntries use
trimmed.reversed() and trimmedEntries.reversed(), but reversed() returns a
ReversedCollection, not an Array; convert those results to arrays before
assigning (e.g., wrap reversed() calls with Array(...)) so keptLines ([String])
and keptEntries ([ReplacementHistoryEntry]) get proper Array values; update the
assignments that reference keptLines, keptEntries, trimmed and trimmedEntries
accordingly.
In `@papuga/Models/ReplacementHistoryEntry.swift`:
- Around line 63-66: The truncated(_:) logic in ReplacementHistoryEntry
currently preserves maxStoredCharCount characters then appends an ellipsis,
resulting in persisted length of maxStoredCharCount+1; change truncated(_ text:
String) so when text.count > maxStoredCharCount it takes
String(text.prefix(maxStoredCharCount - 1)) and appends "…" (ensure
maxStoredCharCount > 0 to avoid negative prefix length) so the total stored
length, including the ellipsis, equals maxStoredCharCount.
In `@papuga/Views/History/RecommendationsSectionView.swift`:
- Around line 80-83: The .addAppToBlocklist branch can append duplicates
differing only by case; update the check that uses
autoFixBlocklist.contains(bundleID) to perform a case-insensitive dedup by
comparing normalized forms (e.g., lowercased) of bundleID against the existing
entries in autoFixBlocklist, and only append when no case-insensitive match
exists (or alternatively normalize and store bundle IDs in a canonical case when
adding); target the case .addAppToBlocklist branch and the autoFixBlocklist
handling to implement this change.
---
Nitpick comments:
In `@papuga/Views/Settings/AutoFixTab.swift`:
- Around line 125-131: The current guard only prevents identical (source,target)
pairs but allows the same source to map to multiple targets; update the
uniqueness check on customAutoReplaceRules so it rejects any existing rule whose
source.lowercased() == src.lowercased() (regardless of target) before appending
a new CustomAutoReplaceRule(source: src, target: tgt); modify the guard that
currently uses contains(where:) to return early when any rule's source matches
the new src (use the same $0.source.lowercased() and src.lowercased()
comparisons to locate the code).
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 40175433-01d6-4c32-9437-ddd6c830a55a
📒 Files selected for processing (19)
papuga/AppDelegate.swiftpapuga/Core/AutoFixController.swiftpapuga/Core/HistoryWindowController.swiftpapuga/Core/RecommendationEngine.swiftpapuga/Core/ReplacementHistoryStore.swiftpapuga/Core/TextSwitchEngine.swiftpapuga/Models/AppSettings.swiftpapuga/Models/CustomAutoReplaceRule.swiftpapuga/Models/ReplacementHistoryEntry.swiftpapuga/Models/ReplacementHistoryRetention.swiftpapuga/Views/History/ClipboardHistorySectionView.swiftpapuga/Views/History/HistoryWindowView.swiftpapuga/Views/History/RecommendationsSectionView.swiftpapuga/Views/History/ReplacementsHistorySectionView.swiftpapuga/Views/MenuBarView.swiftpapuga/Views/Settings/AutoFixTab.swiftpapuga/Views/Settings/GeneralTab.swiftpapuga/Views/Settings/SettingsView.swiftpapuga/papugaApp.swift
💤 Files with no reviewable changes (2)
- papuga/Views/Settings/SettingsView.swift
- papuga/papugaApp.swift
| keptLines = trimmed.reversed() | ||
| keptEntries = trimmedEntries.reversed() |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In Swift, can a variable of type [String]be assigned directly fromsomeArray.reversed()without wrapping inArray(...)?
💡 Result:
No, a variable of type [String] cannot be assigned directly from the result of someArray.reversed without wrapping it in Array(...) or using another conversion method [1]. In Swift, the reversed method on an Array returns a ReversedCollection, not an Array [2][1]. A ReversedCollection is a lightweight wrapper that provides a view of the original array in reverse order without allocating new memory for the elements [2]. Because the types do not match—ReversedCollection<[String]> is not the same as [String]—the compiler will throw an error if you attempt a direct assignment [1]. To store the result as an Array, you must explicitly convert the collection [2][1]: let someArray = ["a", "b", "c"] let reversedArray = Array(someArray.reversed) // Correct: explicitly creates a new [String] Alternatively, you can use other initializers or methods that accept a sequence, such as: let reversedArray = String This conversion is necessary because it performs the actual allocation and copying of elements into a new array instance [2][1].
Citations:
- 1: https://stackoverflow.com/questions/68332664/is-swift-array-reversedn-efficient-or-not
- 2: https://github.com/apple/swift/blob/main/stdlib/public/core/Reverse.swift
🏁 Script executed:
# Find and verify the file exists
fd -type f -name "ReplacementHistoryStore.swift"Repository: rmarinsky/papuga
Length of output: 231
🏁 Script executed:
# If found, check the specific lines (184-185)
if [ -f "papuga/Core/ReplacementHistoryStore.swift" ]; then
sed -n '180,190p' "papuga/Core/ReplacementHistoryStore.swift" | cat -n
fiRepository: rmarinsky/papuga
Length of output: 575
reversed() assignments will fail to compile.
Lines 184 and 185 assign ReversedCollection directly to [String] and [ReplacementHistoryEntry]. In Swift, reversed() returns a ReversedCollection, not an Array. Wrap with Array(...) to convert:
💡 Proposed fix
- keptLines = trimmed.reversed()
- keptEntries = trimmedEntries.reversed()
+ keptLines = Array(trimmed.reversed())
+ keptEntries = Array(trimmedEntries.reversed())📝 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.
| keptLines = trimmed.reversed() | |
| keptEntries = trimmedEntries.reversed() | |
| keptLines = Array(trimmed.reversed()) | |
| keptEntries = Array(trimmedEntries.reversed()) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@papuga/Core/ReplacementHistoryStore.swift` around lines 184 - 185, The
assignments to keptLines and keptEntries use trimmed.reversed() and
trimmedEntries.reversed(), but reversed() returns a ReversedCollection, not an
Array; convert those results to arrays before assigning (e.g., wrap reversed()
calls with Array(...)) so keptLines ([String]) and keptEntries
([ReplacementHistoryEntry]) get proper Array values; update the assignments that
reference keptLines, keptEntries, trimmed and trimmedEntries accordingly.
…e update flow PR #12 adds a unified history & settings window with Fibonacci-based recommendations and custom auto-replace rules. The README's feature list predates those changes; this commit brings it up to date and adds a short note that the Homebrew cask may lag behind because Sparkle handles updates inside the app. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…tained The Homebrew cask in rmarinsky/homebrew-tap is stuck at v1.0.1 while the latest release is v1.3.2, and there is no intent to keep it in sync. Point users directly at GitHub Releases instead; Sparkle handles updates inside the app from then on. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
- ReplacementHistoryEntry.truncated: persisted text was 81 chars (80 + "…"). Reserve a slot for the ellipsis so total length matches maxStoredCharCount. - RecommendationEngine: dropped the `count < lastThreshold` escape hatch on dismissal. At tier 5 (count ≥ 21) the gate was always false, so dismissed recommendations resurfaced forever. Dismissal is now sticky across all tiers. - RecommendationEngine: normalize bundleID with lowercased() before keying `undoneByBundle`, so mixed-case identifiers do not split the count or collide with the lowercased dedupKey. - RecommendationsSectionView.accept(.addAppToBlocklist): case-insensitive dedup, store the normalized form to keep the blocklist canonical. - AutoFixTab manual rule add: reject any rule whose source already exists, not just exact source/target pairs. At runtime AutoFixController fires only the first match by source, so duplicate-source rules were dead weight. - AppDelegate: opening the history window on launch was racing with the SwiftUI scene that produces layoutManager / clipboardHistoryManager via MenuBarExtra.onAppear, leaving HistoryWindowView with nil environment values. Move the launch-time call into configure(...) where both managers are guaranteed to be populated. - HistoryWindowController fallback: replace silent missing-env path with a visible warning placeholder so future regressions surface instead of crashing inside subviews. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Closes #11.
Summary
Adds a dedicated history & recommendations window that opens on Spotlight launch (or any app reopen), with a
NavigationSplitViewsidebar grouping data sources (clipboard, replacements, recommendations) alongside the previously-standalone settings (General / Hotkeys / AutoFix / Analytics / About). The standalone SwiftUISettingsscene is removed — both menu-bar entries now open this window with a pre-selected section.Side-effect that fixes #11
Before this change, the only way to reach Settings was through the menu-bar icon, so unchecking «Показувати іконку в меню-барі» trapped the user with no UI. The new window opens unconditionally on Spotlight launch (and on every app reopen via
applicationShouldHandleReopen), so the user can recover by typing "Papuga" in Spotlight → window appears → sidebar → "Загальні" → re-enable the icon toggle.Rest of the feature
ReplacementHistoryStorepersists every manual switch + AutoFix apply + AutoFix undo to JSONL in Application Support, with retention presets (1 week / 1 month / 3 months / forever). Stored text is truncated to 80 chars.RecommendationEngineanalyses the history at thresholds 3 → 5 → 8 → 13 → 21 and surfaces three kinds of suggestions:autoFixAllowlist(frequently undone autofix);CustomAutoReplaceRulesource→target (frequently performed manual switch);autoFixBlocklist(many undos in one bundle).AutoFixControllerconsultscustomAutoReplaceRulesbefore the language scorer, so accepted recommendations replace during typing.GeneralTabgains a "Історія замін" block (toggle / retention / clear / open-on-launch);AutoFixTabgains a "Власні правила автозаміни" editor.Test plan
.autoFixAppliedevent in history..autoFixUndoneevent.autoFixAllowlist; next typing of it is not auto-corrected.🤖 Generated with Claude Code