feat: improve auto-mode TUI rendering and reviewflow validation - #61
Conversation
A broken tap (lost HOMEBREW_TAP_TOKEN, org migration, etc.) was blocking the promote-release polling loop indefinitely, preventing the GitHub Release from ever reaching Latest and leaving install scripts pointing at a stale prerelease. Remove homebrew_ready from the hard polling condition. Hard gates are now only required platform assets + PyPI — the channels that actually serve the install scripts and in-app updater. Add a best-effort 'Check Homebrew tap' step that runs after promotion: it checks once and emits a warning annotation + step summary note + Slack alert if the tap lags, then exits 0. The job still succeeds; the warning is visible in the run summary without holding the release hostage.
Blocks gh pr create when shipped-code paths are changed but ## Unreleased in CHANGELOG.md is empty — catches the same gate that changelog-entry-required CI enforces, before the push. Escape hatches (mirroring CI): release/* branch, chore(release) title, or [skip changelog] in the PR body. Also unblocks .claude/settings.json and .claude/hooks/ from .gitignore (changed .claude to .claude/* so negation patterns work) so project-scoped hook config is tracked by the team.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughAdds a CodeRabbit pre-merge gate and a resilient release dispatch; refactors review prompts into manifest-backed bundles with manifest-aware validation and rate-limiting; centralizes security review knowledge; introduces destructive-action deliberation and unattended-denial behavior; refreshes TUI/theme, tool headers, markdown table rendering, and tests. ChangesMerge Gate and Release Workflows
Reviewflow Validation and Manifests
Approval and Auto Mode
Shell UI Theme and Rendering
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
|
Two suites drifted from intentional changes already on this branch: - dispatch-pythinker-home-sync.yml was redesigned to degrade gracefully (best-effort sync backstopped by pythinker-home's daily cron) instead of failing loud. Update the site-dispatch test to the new contract: still require no PAT, App-based auth, a single-repo + contents-only scoped token, a SHA-pinned create-github-app-token action, and a read-only job GITHUB_TOKEN, but assert the dispatch degrades (continue-on-error, no exit 1) while still surfacing failures via a step-summary warning and Slack alert. Rename to ..._degrades_gracefully. - The Agent tool description gained two orchestration bullets (durable finding IDs; stateful review loop). Regenerate the two stale inline snapshots so they match the shipped description.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/core/test_runtime_auto_state.py`:
- Around line 99-132: The test function
test_unattended_runtime_in_default_safe_mode_denies_without_waiting has the
pytest.mark.asyncio decorator applied twice; remove the duplicate decorator so
the function is annotated only once (keep a single `@pytest.mark.asyncio` above
the function definition) to avoid redundant decoration.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: db23cf76-7947-4839-94bc-4dc95f5a58b5
📒 Files selected for processing (65)
.claude/hooks/coderabbit-merge-gate.sh.github/workflows/dispatch-pythinker-home-sync.ymlpackages/pythinker-review/README.mdpackages/pythinker-review/docs/blackbox-parity.mdpackages/pythinker-review/src/pythinker_review/cli/review.pypackages/pythinker-review/src/pythinker_review/reviewflow/provider.pypackages/pythinker-review/src/pythinker_review/reviewflow/workflow.pypackages/pythinker-review/src/pythinker_review/security_scan/knowledge.pypackages/pythinker-review/src/pythinker_review/security_scan/processor.pypackages/pythinker-review/src/pythinker_review/security_scan/prompt.pypackages/pythinker-review/src/pythinker_review/security_scan/tech.pypackages/pythinker-review/src/pythinker_review/signals/advisor.pypackages/pythinker-review/tests/e2e/test_reviewflow_workflow.pypackages/pythinker-review/tests/unit/test_reviewflow.pypackages/pythinker-review/tests/unit/test_security_scan.pypackages/pythinker-review/tests/unit/test_signals.pysrc/pythinker_code/agents/default/code_reviewer.yamlsrc/pythinker_code/agents/default/security_reviewer.yamlsrc/pythinker_code/cli/__init__.pysrc/pythinker_code/config.pysrc/pythinker_code/session_recap.pysrc/pythinker_code/soul/agent.pysrc/pythinker_code/soul/approval.pysrc/pythinker_code/soul/dynamic_injections/auto_mode.pysrc/pythinker_code/tools/__init__.pysrc/pythinker_code/tools/agent/description.mdsrc/pythinker_code/ui/shell/echo.pysrc/pythinker_code/ui/shell/motion.pysrc/pythinker_code/ui/shell/prompt.pysrc/pythinker_code/ui/shell/selectors/settings.pysrc/pythinker_code/ui/shell/slash.pysrc/pythinker_code/ui/shell/tool_renderers/__init__.pysrc/pythinker_code/ui/shell/tool_renderers/_render_utils.pysrc/pythinker_code/ui/shell/tool_renderers/agent.pysrc/pythinker_code/ui/shell/tool_renderers/skill.pysrc/pythinker_code/ui/shell/tool_renderers/todo.pysrc/pythinker_code/ui/shell/visualize/_live_view.pysrc/pythinker_code/ui/shell/visualize/_worklog.pysrc/pythinker_code/ui/theme.pysrc/pythinker_code/utils/rich/markdown.pytests/core/test_approval_auto.pytests/core/test_auto_injection.pytests/core/test_config.pytests/core/test_runtime_auto_state.pytests/e2e/test_shell_modal_e2e.pytests/e2e/test_shell_pty_e2e.pytests/test_session_recap.pytests/ui/test_shell_markdown.pytests/ui_and_conv/README_contract_registry.mdtests/ui_and_conv/test_live_view_notifications.pytests/ui_and_conv/test_live_view_todos.pytests/ui_and_conv/test_md_table_contract.pytests/ui_and_conv/test_prompt_tips.pytests/ui_and_conv/test_settings_selector.pytests/ui_and_conv/test_shell_design_system.pytests/ui_and_conv/test_shell_motion.pytests/ui_and_conv/test_shell_motion_shimmer.pytests/ui_and_conv/test_shell_prompt_echo.pytests/ui_and_conv/test_shell_slash_commands.pytests/ui_and_conv/test_thinking_cycle.pytests/ui_and_conv/test_tui_blocks_integration.pytests/ui_and_conv/test_tui_card_tool_renderers.pytests/ui_and_conv/test_tui_theme_tokens.pytests/ui_and_conv/test_visualize_running_prompt.pytests/utils/test_rich_markdown.py
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests/core/test_default_agent.py (1)
550-637: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick winAdd focused assertions for the new prompt bullets instead of extending the monolithic snapshot.
These two new requirements are only covered by a huge prompt snapshot, which makes the test brittle and hard to review. Add small invariant checks for the new guidance text and keep the snapshot for broader regressions if you still want it.
As per coding guidelines,
tests/**/*agent*.py: "When changing prompts/specs, update or add focused tests; avoid brittle tests that assert large prompt snapshots; prefer behavior, required sections, and exact small invariants".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/core/test_default_agent.py` around lines 550 - 637, Replace the giant snapshot assertion on agent.toolset.tools[0].description with targeted invariant checks for the two new prompt bullets: add small assertions that specific key phrases exist in agent.toolset.tools[0].description (e.g., "Explore Agent — Preferred for Codebase Research" and the short list items like "quick", "medium", "thorough" or the "When Not To Use Agent" short guidance) and keep the full snapshot assertion only as a separate broader-regression test if desired; update tests in test_default_agent.py to assert these exact substrings rather than relying solely on the monolithic snapshot.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/test_release_update_pipeline.py`:
- Around line 92-93: The two assertions are too global—narrow them to the
app-token/degradation step block in tests/test_release_update_pipeline.py so
unrelated steps don't affect the check; locate where the test builds the
workflow string (variable workflow) and instead of asserting across the whole
workflow, find the substring for the step named "app-token" or "degradation"
(e.g., match the step header "name: app-token" or "name: degradation" and
extract its block) and then assert "continue-on-error: true" is present and
"exit 1" is not present within that extracted block only.
---
Outside diff comments:
In `@tests/core/test_default_agent.py`:
- Around line 550-637: Replace the giant snapshot assertion on
agent.toolset.tools[0].description with targeted invariant checks for the two
new prompt bullets: add small assertions that specific key phrases exist in
agent.toolset.tools[0].description (e.g., "Explore Agent — Preferred for
Codebase Research" and the short list items like "quick", "medium", "thorough"
or the "When Not To Use Agent" short guidance) and keep the full snapshot
assertion only as a separate broader-regression test if desired; update tests in
test_default_agent.py to assert these exact substrings rather than relying
solely on the monolithic snapshot.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: ff5d2494-16bf-4660-be06-299f79df9f55
📒 Files selected for processing (3)
tests/core/test_default_agent.pytests/test_release_update_pipeline.pytests/tools/test_tool_descriptions.py
- CHANGELOG.md: add `## Unreleased` entries for the TUI rendering, auto-mode approval, and reviewflow changes so the changelog-entry-required gate passes. - test_shell_markdown: assert the wrapped "Suggested improvement" header fragment via "improvement"[:-1] instead of a misspelled string literal, so the typos spell-check passes while keeping the same coverage. - test_runtime_auto_state: drop a duplicate @pytest.mark.asyncio decorator flagged by CodeRabbit. The PR title was also updated to a Conventional Commit prefix.
Address CodeRabbit: the continue-on-error / no-exit-1 checks scanned the whole workflow file, so an unrelated step could satisfy or break them. Add a _step_block helper and assert continue-on-error on the token-mint step and the absence of exit 1 (plus presence of exit 0) within the dispatch step.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/test_release_update_pipeline.py`:
- Around line 17-34: The helper _step_block currently lets next(...) raise
StopIteration with no context if name_fragment isn't found; change it to handle
that case by capturing the result (e.g., use next(..., None) or wrap in
try/except) and raise a clear assertion or ValueError that includes the missing
name_fragment (and optionally the workflow snippet) so test failures show which
step was missing; modify the logic around the start variable in _step_block to
check for None or catch StopIteration and raise the descriptive error before
continuing.
In `@tests/ui/test_shell_markdown.py`:
- Line 156: The compound assertion at the end of test_shell_markdown.py uses a
single `assert "Suggested" in output and "improvement"[:-1] in output`; split
this into two separate assertions so failures indicate which substring is
missing: one `assert "Suggested" in output` and another `assert
"improvement"[:-1] in output` (keep the existing slicing) located where the
original compound assert is.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: d2f27b10-4250-4652-ba35-51690cc008a4
📒 Files selected for processing (4)
CHANGELOG.mdtests/core/test_runtime_auto_state.pytests/test_release_update_pipeline.pytests/ui/test_shell_markdown.py
💤 Files with no reviewable changes (1)
- tests/core/test_runtime_auto_state.py
Summary
Verification
Summary by CodeRabbit