feat(ios): render $skill mentions as tappable pills with detail sheet - #320
Closed
kingbootoshi wants to merge 4 commits into
Closed
kingbootoshi wants to merge 4 commits into
kingbootoshi wants to merge 4 commits into
Conversation
$skill mentions autocomplete in the composer but land in the transcript as plain text. This renders them as accent pills in user bubbles, mirroring the existing plugin-ref pills in FormattedText, and tapping a pill opens a sheet with the skill's display name, scope, description, default prompt, and path. - SkillMentionCatalog: per-conversation @observable registry injected via environment; loads listSkills once, and only when a rendered message actually contains a $mention token, so mention-free transcripts never pay for the RPC - tokenizer mirrors the composer's mention byte rules exactly ($ + [A-Za-z0-9_-]+, rejected mid-word); kept self-contained to avoid colliding with in-flight branches that touch the composer helpers - only names present in the loaded catalog render as pills, so dollar amounts like $200 stay plain text - catalog rebuilt per thread from the thread's cwd - unit tests for the tokenizer; UI test drives the display harness, taps the pill, and asserts the detail sheet Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This was referenced Sep 13, 2026
Owner
|
Superseded by #341 — the feature commits were cherry-picked onto current main as a rebranch. The CI failures here came from stale |
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
3 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.
Summary
Render recognized $skill mentions as tappable pills in iOS user messages. A detail sheet displays skill metadata; catalog loading is demand-driven and recovers when connection or conversation context changes. Android presentation remains a documented follow-up.
Review and verification — 2026-09-12
Reviewed skill pills, tokenizer behavior, catalog lifecycle and detail content. Fixed three observable gaps: an offline/failed initial catalog request could permanently suppress pills; a hydrated or changed cwd did not rebuild the catalog; and editing plain text to add the first mention did not trigger loading. The catalog now follows thread/cwd/transport availability, allows retry after failures, and text changes trigger demand loading. In-flight requests are owned by the catalog so recycling a text row does not cancel them. The detail sheet now displays the full description with a short-description fallback.
Added async regression coverage for failed-load retry, successful-load caching and configuring an initially unavailable loader; extended the UI assertion for the full description. Added an explicit Android parity entry: this PR ships SwiftUI presentation; Android pill/detail behavior and shared Rust token/catalog resolution remain a follow-up, not verified parity.
Checks: 14 skill tokenizer/catalog XCTest cases passed in a focused macOS Swift harness extracted from the actual source. The harness substitutes only a minimal SkillMetadata record to avoid the native Rust/UniFFI link; it does not prove the full app builds. All five changed Swift files passed
swiftc -frontend -parse;git diff --checkpassed. New UI assertion was not executed.Still needs native device/simulator checks: offline→reconnect, cwd hydration, edited messages, reused conversations and detail-sheet sizing.
Validation limitations: a native
make ios-sim-fastattempt in the isolated #320 worktree reached Ghostty compilation and stopped because this Xcode installation lacks the Metal Toolchain (xcrun metal: "cannot execute tool 'metal' due to missing Metal Toolchain"). Therefore this review did not execute simulator/UI tests for these changes. No global Xcode installation was changed. The normal project-regeneration path also requires missing generated UniFFI bindings and filesystem resources; no generated placeholders were committed. Native CI and device checks remain required.No dependency revisions changed. No PR was merged; updates are confined to the PR branch.
Final branch update: synchronized with main at
94b1b04fand included the shared native build fix discovered in CI: Swift SavedServerRecord constructors now supply detachedTransport, and the Android SSH login dialog obtains its local Context and uses SavedServerStore correctly. This repairs the demonstrated source-level compile failures in the combined branch; it does not establish a successful full native build. After that update, all 14 changed Swift files passed syntax parsing and the branch diff passed whitespace checks. Android build verification remains pending.Original demonstration
skill-mention-pills.mp4
Original contribution by @kingbootoshi. Review fixes retain the original commit history.