fix(desktop): support IME composition and reduce task noise#7537
fix(desktop): support IME composition and reduce task noise#7537waffensam wants to merge 2 commits into
Conversation
Greptile SummaryThis PR fixes IME (Input Method Editor) composition handling in the macOS desktop chat editor and reduces noisy task extraction from public/group channels. The IME fix prevents placeholder text from flashing during composition and stops
Confidence Score: 4/5Safe to merge; the IME tracking logic is sound, text binding is guarded during composition, and the new prompt rules are additive. All five changed files are low-risk: the IME state machine correctly deduplicates callbacks, the updateNSView guard prevents string clobbering during composition, and the placeholder gating in both input views is straightforward. The only notable gap is the test file, which checks string presence rather than exercising any extraction behavior, leaving prompt regressions that preserve the exact words but change their context undetected. desktop/Desktop/Tests/TaskAssistantPromptTests.swift — the test assertions only verify substring presence and would not catch structural rearrangements or semantic regressions in the prompt. Important Files Changed
Sequence DiagramsequenceDiagram
participant User
participant OmiNSTextView
participant Coordinator
participant SwiftUI
User->>OmiNSTextView: starts IME (e.g. Japanese input)
OmiNSTextView->>OmiNSTextView: setMarkedText(_:selectedRange:replacementRange:)
OmiNSTextView->>Coordinator: onMarkedTextStatusChange(true)
Coordinator->>Coordinator: "updateMarkedTextState(true), lastMarkedTextState = true"
Coordinator-->>SwiftUI: DispatchQueue.main.async → onMarkedTextChange(true)
SwiftUI-->>SwiftUI: "hasMarkedText = true → placeholder hidden"
User->>OmiNSTextView: presses Enter (during composition)
OmiNSTextView->>Coordinator: doCommandBy(insertNewline:)
Coordinator-->>OmiNSTextView: "hasMarkedText() == true → return false (IME handles it)"
User->>OmiNSTextView: confirms IME selection
OmiNSTextView->>OmiNSTextView: insertText(_:replacementRange:)
OmiNSTextView->>Coordinator: onMarkedTextStatusChange(false)
Coordinator->>Coordinator: "updateMarkedTextState(false), lastMarkedTextState = false"
Coordinator-->>SwiftUI: DispatchQueue.main.async → onMarkedTextChange(false)
SwiftUI-->>SwiftUI: "hasMarkedText = false → placeholder shown (if text empty)"
Note over SwiftUI,OmiNSTextView: updateNSView guards text assignment via !textView.hasMarkedText()
Reviews (1): Last reviewed commit: "fix(desktop): reduce group-channel task ..." | Re-trigger Greptile |
| XCTAssertTrue(prompt.contains("CRITICAL FOR PUBLIC/GROUP CHANNELS")) | ||
| XCTAssertTrue(prompt.contains("visible evidence shows the user is directly involved")) | ||
| XCTAssertTrue(prompt.contains("call no_task_found")) | ||
| XCTAssertTrue(prompt.contains("questions posted to the community at large")) | ||
| } |
There was a problem hiding this comment.
Shallow string-presence assertions
The test only verifies that four literal substrings exist inside defaultAnalysisPrompt. It acts as a deletion guard but cannot catch prompt regressions where the wording is reordered, split across lines, or rephrased while losing semantic meaning — all of those would still pass. If the intent is a prompt-regression test, consider also asserting the key logical conditions in the right order (e.g. that the @mention bullet appears before the DM bullet, or that no_task_found follows the public-channel guard block), which would catch structural rearrangements that break the LLM's interpretation.
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!
|
Hey @waffensam 👋 Thank you so much for taking the time to contribute to Omi! We truly appreciate you putting in the effort to submit this pull request. After careful review, we've decided not to merge this particular PR. Please don't take this personally — we genuinely try to merge as many contributions as possible, but sometimes we have to make tough calls based on:
Your contribution is still valuable to us, and we'd love to see you contribute again in the future! If you'd like feedback on how to improve this PR or want to discuss alternative approaches, please don't hesitate to reach out. Thank you for being part of the Omi community! 💜 |
Summary:
Tests:
Notes: