Skip to content

docs(skill): pi package integration - #52

Merged
umputun merged 3 commits into
umputun:masterfrom
melonamin:docs/pi-package-review-fixes
Apr 7, 2026
Merged

umputun merged 3 commits into
umputun:masterfrom
melonamin:docs/pi-package-review-fixes

Conversation

@melonamin

@melonamin melonamin commented Apr 7, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Add pi package with extensions for launching revdiff from pi, capturing annotations, and displaying them in a widget and side panel
  • Add pi documentation to README, docs.html, and index.html
  • Add accessibility label and pi project link to index.html feature card

Add pi package section to README, docs, and landing page. Clarify
command descriptions, add missing /revdiff-reminders to docs.html,
and improve accessibility on feature icon.
@melonamin
melonamin requested a review from umputun as a code owner April 7, 2026 12:14
@melonamin

melonamin commented Apr 7, 2026

Copy link
Copy Markdown
Contributor Author

@umputun I'm not sure about this one, I need your input.

If you merge this, pi users would be able to run pi install https://github.com/umputun/revdiff to install the extension, with two minor downsides:

  • You'll have to have package.json at the repo root.
  • pi install clones the full repo, most of which is not the extension.

An alternative would be to publish it separately, let's say at https://github.com/melonamin/pi-revdiff, but it relies on .claude-plugin/skills/revdiff/scripts/launch-revdiff.sh, so we would have a sync problem.

What is your philosophy for this kind of issue/features?

@melonamin melonamin changed the title Add pi package integration docs(skill): pi package integration Apr 7, 2026
@umputun
umputun requested a review from Copilot April 7, 2026 15:46

@umputun umputun left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thx, I'm fine with keeping this in-repo under plugins/pi/. the extension code is well-structured and reuses existing infra nicely.

few things to address before merge:

  1. node_modules/ in .gitignore - with package.json at root, anyone running npm install will create node_modules/. pls add it to .gitignore

  2. version in package.json - currently 0.11.0, project is at v0.13.0. should either track the project version or start at 0.1.0 if it has its own versioning. what's the intent? also, after any pi plugin file change pls bump the version in package.json (same convention we have for .claude-plugin/)

  3. detectSmartRefFallback missing no-commits check (revdiff.ts:971-992) - the shell script detect-ref.sh checks for has_commits (line 48-49) before running git rev-parse. the TS fallback skips this, so fresh git init repos will behave differently. would be nice to add the same guard

  4. wildcard peerDependencies - "@mariozechner/pi-coding-agent": "*" and "@mariozechner/pi-tui": "*" - should pin at least a major version to avoid silent breakage on pi API changes

@umputun

umputun commented Apr 7, 2026

Copy link
Copy Markdown
Owner

also, pls add a rule to CLAUDE.md about bumping version in package.json after any pi plugin file change. similar to the existing rule we have for .claude-plugin/

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds a pi harness integration for revdiff, alongside documentation and site updates to advertise the new distribution and workflow.

Changes:

  • Introduces a pi package (root package.json) plus pi extensions for launching revdiff, persisting captured annotations, and rendering them in a widget/right-side panel.
  • Adds an optional post-edit reminder extension and a pi skill/README describing commands and recommended workflow.
  • Updates site/index.html, site/docs.html, and README.md to document and promote pi installation and usage.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
site/index.html Updates SEO/social metadata and adds a “Pi integration” feature card + install option.
site/docs.html Adds a “Pi package” section and navigation link describing pi commands and setup.
README.md Documents pi package installation, commands, and notes about modes and paths.
plugins/pi/skills/revdiff/SKILL.md Adds a pi skill definition describing when/how to use the revdiff pi integration.
plugins/pi/README.md Documents the pi integration directory structure and installation.
plugins/pi/extensions/revdiff.ts Implements the main pi extension: launch modes, annotation capture/parsing, widget/panel UI, and commands.
plugins/pi/extensions/revdiff-post-edit.ts Implements optional post-edit reminders based on agent edit/write tool calls.
package.json Adds pi package manifest exposing extensions and skills to pi.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +53 to +56
if (!enabled || editCallsInRun === 0 || !ctx.hasUI) {
ctx.ui.setStatus(STATUS_KEY, undefined);
return;
}

Copilot AI Apr 7, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updateStatus calls ctx.ui.setStatus(...) even when ctx.hasUI is false (the !ctx.hasUI case is explicitly included in the guard). If ui is only valid when hasUI is true, this can throw in headless/non-TUI contexts. Consider returning early when !ctx.hasUI (or using the last known UI context) before calling setStatus.

Suggested change
if (!enabled || editCallsInRun === 0 || !ctx.hasUI) {
ctx.ui.setStatus(STATUS_KEY, undefined);
return;
}
if (!enabled || editCallsInRun === 0) {
clearReminder();
return;
}
if (!ctx.hasUI) {
return;
}

Copilot uses AI. Check for mistakes.
Comment on lines +3 to +16
const CONFIG_TYPE = "revdiff-reminder-config";
const STATUS_KEY = "revdiff-reminder";
const LAST_LAUNCH_TYPE = "revdiff-last-launch";

interface ReminderConfig {
enabled: boolean;
}

interface LaunchMemory {
args: string[];
label: string;
mode: "direct" | "overlay";
createdAt: number;
}

Copilot AI Apr 7, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LAST_LAUNCH_TYPE and the LaunchMemory shape are duplicated here and in plugins/pi/extensions/revdiff.ts. To avoid the two drifting (e.g., adding fields in one file but not the other), consider extracting the shared constants/types into a small shared module under plugins/pi/extensions/ and importing it from both extensions.

Copilot uses AI. Check for mistakes.
Comment thread package.json Outdated
@@ -0,0 +1,29 @@
{
"name": "revdiff-pi",
"version": "0.11.0",

Copilot AI Apr 7, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new package.json sets the pi package version to 0.11.0, but the repo/site currently references revdiff v0.13.0 (e.g., CHANGELOG.md and site/index.html). If the pi package is intended to track revdiff releases, consider aligning this version to avoid confusing upgrade semantics for pi users.

Suggested change
"version": "0.11.0",
"version": "0.13.0",

Copilot uses AI. Check for mistakes.
- Add node_modules/ to .gitignore
- Set pi plugin version to 0.1.0 (independent versioning)
- Pin pi peerDependencies to ^0.60 instead of wildcard
- Add no-commits guard to detectSmartRefFallback matching detect-ref.sh
- Add Pi Plugin section to CLAUDE.md with version bump convention
updateStatus and setEnabled were calling ctx.ui methods even when
ctx.hasUI was false, which could throw in headless contexts.
@melonamin

Copy link
Copy Markdown
Contributor Author

Addressed all yours and one bug found by Copilot(good bot)

@umputun umputun left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

all 4 points addressed, thx. node_modules in gitignore, version 0.1.0, hasCommits guard in the fallback, deps pinned to ^0.60. CLAUDE.md rule added too.

the LaunchMemory duplication between the two extensions is fine, keeps them decoupled.

LGTM

@umputun
umputun merged commit 38c420e into umputun:master Apr 7, 2026
1 check passed
sanchesfree pushed a commit to sanchesfree/revdiff that referenced this pull request Apr 8, 2026
* docs: add pi package documentation and review fixes

Add pi package section to README, docs, and landing page. Clarify
command descriptions, add missing /revdiff-reminders to docs.html,
and improve accessibility on feature icon.

* fix: address pi package PR review feedback

- Add node_modules/ to .gitignore
- Set pi plugin version to 0.1.0 (independent versioning)
- Pin pi peerDependencies to ^0.60 instead of wildcard
- Add no-commits guard to detectSmartRefFallback matching detect-ref.sh
- Add Pi Plugin section to CLAUDE.md with version bump convention

* fix: guard ctx.ui calls behind hasUI check in post-edit extension

updateStatus and setEnabled were calling ctx.ui methods even when
ctx.hasUI was false, which could throw in headless contexts.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants