Reviewed and merged GitHub PR #775Public

Agent

Model

Claude Opus 4.5 (latest)

Started

Jan 16, 11:44 AM

Messages91

Downloads0

Summary

Closed, Completed

Understand
Read PR details and linked issues.
Understand
Understand
Analyzed code diffs and relevant files.
Understand
Verify
Checked specific code usage in interactive mode.
Verify
Edit
Merged PR into main branch with changelog updates.
Edit

You are given one or more GitHub PR URLs: https://github.com/badlogic/pi-mono/pull/775

For each PR URL, do the following in order:

Read the PR page in full. Include description, all comments, all commits, and all changed files.
Identify any linked issues referenced in the PR body, comments, commit messages, or cross links. Read each issue in full, including all comments.
Analyze the PR diff. Read all relevant code files in full with no truncation from the current main branch and compare against the diff. Do not fetch PR file blobs unless a file is missing on main or the diff context is insufficient. Include related code paths that are not in the diff but are required to validate behavior.
Check for a changelog entry in the relevant packages/*/CHANGELOG.md files. Report whether an entry exists. If missing, state that a changelog entry is required before merge and that you will add it if the user decides to merge. Follow the changelog format rules in AGENTS.md. Verify:
- Entry uses correct section (### Breaking Changes, ### Added, ### Fixed, etc.)
- External contributions include PR link and author: Fixed foo ([#123](https://github.com/badlogic/pi-mono/pull/123) by [@user](https://github.com/user))
- Breaking changes are in ### Breaking Changes, not just ### Fixed
Check if packages/coding-agent/README.md, packages/coding-agent/docs/.md, packages/coding-agent/examples/**/.md require modification. This is usually the case when existing features have been changed, or new features have been added.
Provide a structured review with these sections:
- Good: solid choices or improvements
- Bad: concrete issues, regressions, missing tests, or risks
- Ugly: subtle or high impact problems
Add Questions or Assumptions if anything is unclear.
Add Change summary and Tests.

Output format per PR: PR: Changelog:

... Good:
... Bad:
... Ugly:
... Questions or Assumptions:
... Change summary:
... Tests:
...

If no issues are found, say so under Bad and Ugly.

Let me read the relevant source files to validate the implementation:

Let me check how the autocomplete is used in interactive-mode.ts, specifically looking for how argument completions are handled:

Let me check the CHANGELOG entry and look for any related tests:

Let me check if the README needs updates and verify the full documentation:

Now let me verify if there are any issues in the types or if the import in the documentation example is correct:

PR: https://github.com/badlogic/pi-mono/pull/775

Changelog:

Entry exists under ## [Unreleased] → ### Added section
Missing PR link and author attribution. Per AGENTS.md, external contributions require: Fixed foo ([#123](https://github.com/badlogic/pi-mono/pull/123) by [@user](https://github.com/user))
Current: - Extension commands can provide argument auto-completions via \getArgumentCompletions` in `pi.registerCommand()`.`
Should be: - Extension commands can provide argument auto-completions via \getArgumentCompletions` in `pi.registerCommand()` (#775 by @ribelo)`

Good:

Clean, minimal implementation (6 lines of actual code changes)
Reuses existing SlashCommand.getArgumentCompletions interface from pi-tui rather than reinventing
Type signature updated elegantly using Omit<RegisteredCommand, "name">
Documentation added with clear, working example that correctly imports AutocompleteItem from @mariozechner/pi-tui
Threads callback correctly: registerCommand() → loader.ts storage → interactive-mode.ts mapping → CombinedAutocompleteProvider
Type checks pass (npm run check clean)
No runtime behavior changes for commands without argument completions (optional field)

Bad:

Changelog entry missing PR link and author attribution (required fix before merge)
No tests for the new functionality (existing pi-tui autocomplete tests cover the underlying CombinedAutocompleteProvider logic, but no extension-specific test verifies the wiring)

Ugly:

None

Questions or Assumptions:

None. The implementation correctly exposes the existing /model argument completion mechanism to extensions.

Change summary:

types.ts: Added optional getArgumentCompletions to RegisteredCommand interface
loader.ts: Changed registerCommand type to Omit<RegisteredCommand, "name"> to include the new callback
interactive-mode.ts: Maps cmd.getArgumentCompletions when converting extension commands to SlashCommand format for autocomplete
docs/extensions.md: Added documentation with working example
CHANGELOG.md: Added entry (needs fix)

Tests:

No new tests added. The PR author manually tested with a dummy extension per PR description. Existing pi-tui autocomplete logic is tested in packages/tui/src/autocomplete.ts.

Documentation check (packages/coding-agent):

README.md: No update needed (extension API examples use registerCommand but argument completion is an advanced feature documented in docs/extensions.md)
docs/extensions.md: ✓ Updated with example
examples/**/*.md: No update needed (no markdown examples affected)

i don't see where getArgumentCompletions is actually called?

You're right to ask. Let me trace the full call chain:

The call happens in packages/tui/src/autocomplete.ts. Here's the flow:

Extension registers command (via PR changes):

interactive-mode.ts maps to SlashCommand (line 318-325):

typescript

const extensionCommands: SlashCommand[] = (this.session.extensionRunner?.getRegisteredCommands() ?? []).map(  (cmd) => ({    name: cmd.name,    description: cmd.description ?? "(extension command)",    getArgumentCompletions: cmd.getArgumentCompletions,  // <-- PR adds this line  }),);

Passed to CombinedAutocompleteProvider (line 337-338):

Called in autocomplete.ts (lines 147-166) when user types `/command

ah got it, so this actually works. very nice, merge into main, will have to resolve conflict locally

Done. Merged PR #775 into main.

Changes made:

Rebased on main (resolved CHANGELOG conflict)
Fixed CHANGELOG entry to include PR link and author attribution: ([#775](https://github.com/badlogic/pi-mono/pull/775) by [@ribelo](https://github.com/ribelo))
Pushed to main
Closed PR with comment

const extensionCommands: SlashCommand[] = (this.session.extensionRunner?.getRegisteredCommands() ?? []).map( (cmd) => ({ name: cmd.name, description: cmd.description ?? "(extension command)", getArgumentCompletions: cmd.getArgumentCompletions, // <-- PR adds this line }),);