Skip to content

fix: update poly review --delete to interactive select-and-delete flow#47

Merged
oeisenberg merged 13 commits intomainfrom
oliver/fix/change-review-delete-bh
Mar 30, 2026
Merged

fix: update poly review --delete to interactive select-and-delete flow#47
oeisenberg merged 13 commits intomainfrom
oliver/fix/change-review-delete-bh

Conversation

@oeisenberg
Copy link
Copy Markdown
Contributor

@oeisenberg oeisenberg commented Mar 26, 2026

Summary

  • Updates gist description format from Diff for account/project to Poly ADK: local → remote / project: before → after
  • Replaces poly review --delete bulk-delete with a questionary.checkbox prompt so users can select individual gists to delete
  • Adds poly review --list to interactively select and open a gist in the browser
  • Adds list_diff_gists() and delete_gist() helpers to GitHubAPIHandler; refactors delete_diff_gists() to use them
  • Exits gracefully with a warning if no gists exist or none are selected
  • Adds --json flag for outputting results using JSON
  • Adds unit tests for gist commands in tests/review_test.py
  • Converts poly review list and poly review delete from a positional action argument to proper argparse subparsers, so each subcommand owns only its relevant flags (--id now only appears under poly review delete --help)
  • Applies the same refactor to poly branch (list, create, switch, current), so --force, --format, --from-projection etc. only appear under poly branch switch --help for standardisation

Test plan

  • Run poly review to create one or more review gists
  • Run poly review --delete and verify the checkbox menu appears listing gists by description
  • Select a subset and confirm only those are deleted
  • Run again with no gists present and confirm "No review gists found." message
  • Press Ctrl-C / select nothing and confirm "No gists selected. Exiting." warning
  • Run poly review --list and confirm a select menu appears; selecting a gist opens it in the browser
image image image image image

🤖 Generated with Claude Code

Replace bulk-delete behaviour with a questionary.checkbox prompt so users
can select individual review gists to delete rather than removing all at once.
Adds list_diff_gists() and delete_gist() helpers to GitHubAPIHandler.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@oeisenberg oeisenberg marked this pull request as draft March 26, 2026 17:19
@github-actions

This comment has been minimized.

oeisenberg and others added 2 commits March 26, 2026 22:33
- Format gist choices as: date  short-hash  description
- Use 7-char gist ID (git commit style) instead of first3...last3
- Include account/project name in branch-comparison gist descriptions
- Add --list flag to interactively select and open a gist in the browser
- Extract _format_gist_choice as module-level helper

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@github-actions

This comment has been minimized.

@oeisenberg oeisenberg marked this pull request as ready for review March 27, 2026 09:52
@oeisenberg oeisenberg requested a review from Ruari-Phipps March 27, 2026 09:52
@github-actions

This comment has been minimized.

@oeisenberg oeisenberg requested a review from Ruari-Phipps March 27, 2026 11:57
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

…rs (#49)

## Summary
- Converts `poly review list` and `poly review delete` from a positional
`action` argument to proper argparse subparsers, so each subcommand owns
only its relevant flags (`--id` now only appears under `poly review
delete --help`)
- Applies the same refactor to `poly branch` (`list`, `create`,
`switch`, `current`), so `--force`, `--format`, `--from-projection` etc.
only appear under `poly branch switch --help` for standardisation

## Test plan
- [x] `poly review --help` no longer shows `--id`
- [x] `poly review delete --help` shows `--id`
- [x] `poly branch switch --help` shows `--force`, `--format`; `poly
branch list --help` does not
- [x] All existing commands still work end-to-end
- [x] 391 tests pass

<img width="742" height="205" alt="image"
src="https://github.com/user-attachments/assets/c6f4e123-b779-443f-9842-53e53671ae59"
/>
<img width="942" height="512" alt="image"
src="https://github.com/user-attachments/assets/5a9c07e0-7448-4153-9619-eae293071fa8"
/>


🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-authored-by: Ruari Phipps <ruari@poly-ai.com>
@github-actions

This comment has been minimized.

oeisenberg and others added 4 commits March 27, 2026 14:13
The second match clause `gist_id.startswith(g["id"][:7])` was backwards:
it matched any user-supplied ID that *starts with* a stored gist's 7-char
prefix, meaning a non-existent ID like "abc1234xyz" would silently delete
gist "abc1234yyy". The first clause `g["id"].startswith(gist_id)` already
handles both full IDs and short 7-char prefixes correctly on its own.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
When output_json=True, HTTP and OS errors were being printed via error()
instead of json_print(), breaking the machine-readable contract. Now
consistent with delete_gists and review which both emit
{"success": false, "message": "..."} on error.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The subparser refactor moved --json off the top-level branches_parser,
breaking the previously valid `poly branch --json list` form. Restoring
json_parent to branches_parser and removing it from each subparser
fixes the regression: --json is now parsed at the top level so both
`poly branch --json list` and `poly branch list` work correctly.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The CLI now calls list_diff_gists() and delete_gist() directly, leaving
delete_diff_gists() with no callers. Removing the dead code.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@github-actions

This comment has been minimized.

Comment on lines +58 to +65
def _format_gist_choice(g: dict) -> str:
"""Format a gist dict as a human-readable choice label."""
id_hint = g["id"][:7]
date = g.get("created_at", "")[:10] # YYYY-MM-DD
parts = [p for p in [date, id_hint, g["description"]] if p]
return " ".join(parts)


Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Could maybe put in output/console

Co-authored-by: Ruari Phipps <ruari@poly-ai.com>
@github-actions
Copy link
Copy Markdown
Contributor

Coverage Report

Base (main) PR Change
70.6% 70.8% +0.2% ✅

Changed file coverage

File Coverage Change
poly/cli.py 37.5% +6.9% ✅
poly/handlers/github_api_handler.py 53.7% +11.8% ✅

@oeisenberg oeisenberg merged commit 9ea826a into main Mar 30, 2026
3 checks passed
@oeisenberg oeisenberg deleted the oliver/fix/change-review-delete-bh branch March 30, 2026 10:16
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.

fix: poly review --delete should prompt for confirmation before removing all gists

2 participants