Srihari Sriraman shared in

Public

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.

Now let me read the relevant source files to fully understand the changes and validate the implementation. I need to read:

The full types.ts file to understand the RegisteredCommand interface
The loader.ts file to see how registerCommand works
The interactive-mode.ts file to understand how autocomplete works

Let me also check for any linked issues (none mentioned in the PR body) and read the full context.

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

Readtypes.ts

Readloader.ts

Readinteractive-mode.ts

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

Readautocomplete.ts

Readextensions.md80 lines

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

ReadCHANGELOG.md50 lines

Let me check if there are any tests related to autocomplete in pi-tui and if they should test getArgumentCompletions. Also, let me check the RegisteredCommand interface again to make sure the change is complete.

Looking at the PR:

types.ts - Added getArgumentCompletions?: (argumentPrefix: string) => AutocompleteItem[] | null; to RegisteredCommand interface and updated registerCommand signature
loader.ts - Changed the type annotation from inline to Omit<RegisteredCommand, "name">
interactive-mode.ts - Added getArgumentCompletions: cmd.getArgumentCompletions when mapping extension commands to SlashCommand format
docs/extensions.md - Added documentation for the new feature
CHANGELOG.md - Added a changelog entry but it's in the wrong place

Wait, looking at the CHANGELOG, I don't see the entry from the PR. Let me re-check the diff. The diff shows:

But this entry is placed after the input event entry, which is correct. It's in the Added section under [Unreleased]. Let me verify this is the correct placement.

Actually, looking at the diff more closely:

The entry is added AFTER the input event line and AFTER the extension example line
This is correctly placed in the ### Added section under ## [Unreleased]

Now let me check if the CHANGELOG entry follows the proper format:

External contributions should include PR link and author: Fixed foo ([#123](https://github.com/badlogic/pi-mono/pull/123) by [@user](https://github.com/user))

The entry is: - Extension commands can provide argument auto-completions via \getArgumentCompletions` in `pi.registerCommand()`.`

The CHANGELOG entry is missing the PR link and author attribution. Since this is from @ribelo, it should include ([#775](https://github.com/badlogic/pi-mono/pull/775) by [@ribelo](https://github.com/ribelo)). I should also check if the README.md needs updates for this new extension API feature.

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

The AutocompleteItem type is exported from pi-tui, so extensions can import it. The documentation example in extensions.md correctly shows importing it:

Now let me summarize what I found:

Good:

The implementation is clean and minimal - just 6 lines of code changes
Uses the existing SlashCommand interface from pi-tui which already has getArgumentCompletions
The type signature is properly updated using Omit<RegisteredCommand, "name">
Documentation is added with a clear example
The AutocompleteItem type is correctly imported from @mariozechner/pi-tui in the example
No new dependencies
Type checks pass

Bad:

CHANGELOG entry is missing PR link and author attribution (required for external contributions per AGENTS.md)
No tests for the new functionality (though the existing autocomplete tests in pi-tui cover the underlying mechanism)

Ugly:

Nothing ugly found

The implementation correctly threads the getArgumentCompletions callback through:

Extension registers command with getArgumentCompletions via registerCommand()
loader.ts stores it in extension.commands
interactive-mode.ts retrieves commands and maps them to SlashCommand format with getArgumentCompletions
The existing CombinedAutocompleteProvider in pi-tui handles the rest

This is a well-designed minimal change that exposes existing functionality to extensions.

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

Readindex.ts

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?

The user is asking where getArgumentCompletions is actually called. Let me trace through the code again.

Extension registers a command with getArgumentCompletions via pi.registerCommand()
loader.ts stores it in extension.commands
interactive-mode.ts retrieves commands via getRegisteredCommands() and maps them to SlashCommand format, including getArgumentCompletions
These are passed to CombinedAutocompleteProvider in setupAutocomplete()

The actual call to getArgumentCompletions happens in CombinedAutocompleteProvider.getSuggestions() in packages/tui/src/autocomplete.ts at line 159-163:

typescript

if (!command || !("getArgumentCompletions" in command) || !command.getArgumentCompletions) {    return null; // No argument completion for this command}
const argumentSuggestions = command.getArgumentCompletions(argumentText);

So the call chain is:

User types /deploy (with space)
Editor triggers autocomplete
CombinedAutocompleteProvider.getSuggestions() is called
It detects we're in command argument context (line has /command with space)
It finds the command and calls command.getArgumentCompletions(argumentText)
Extension's callback returns suggestions

The key is that CombinedAutocompleteProvider already has the logic to call getArgumentCompletions - it's used by the built-in /model command. The PR just wires extension commands into the same system.

Let me verify this by looking at how the built-in /model command is set up and how extension commands are passed to the autocomplete provider.

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

OK, so the extension commands are mapped and passed to CombinedAutocompleteProvider at line 338. Now let me look at where getArgumentCompletions is actually called in the autocomplete provider. I already read it - it's at line 159-163 in packages/tui/src/autocomplete.ts:

typescript

const command = this.commands.find((cmd) => {    const name = "name" in cmd ? cmd.name : cmd.value;    return name === commandName;});if (!command || !("getArgumentCompletions" in command) || !command.getArgumentCompletions) {    return null; // No argument completion for this command}
const argumentSuggestions = command.getArgumentCompletions(argumentText);

So the call path is:

User types in editor
Editor triggers autocomplete via getSuggestions()
CombinedAutocompleteProvider.getSuggestions() in packages/tui/src/autocomplete.ts
When user types /command (with space after command), lines 147-166 handle argument completion:
- Extract command name and argument text
- Find the command in this.commands (which includes extension commands)
- Call command.getArgumentCompletions(argumentText) if it exists

So getArgumentCompletions is called in packages/tui/src/autocomplete.ts line 163. The PR correctly wires extension commands into this existing system by:

Adding getArgumentCompletions to the RegisteredCommand type
Passing it through when mapping extension commands to SlashCommand format

Let me confirm this is working by showing the relevant code path more clearly.

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

ReadCHANGELOG.md30 lines

EditedCHANGELOG.md

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 command = this.commands.find((cmd) => { const name = "name" in cmd ? cmd.name : cmd.value; return name === commandName;});if (!command || !("getArgumentCompletions" in command) || !command.getArgumentCompletions) { return null; // No argument completion for this command} const argumentSuggestions = command.getArgumentCompletions(argumentText);

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