fix: close ponytail parity gaps from upstream PR audit (#61) - #96
Merged
Conversation
Addresses items 5, 7, 11, 15, 16, 18 from the issue #61 PR audit: - Bare `/ponytail` (and `/ponytail status`) now actually reports the active mode via UserPromptSubmit additionalContext instead of being a no-op that only echoed "OK" (ponytail#99). - Detect a compression/persona plugin (e.g. caveman) wired into the agent's settings.json and append a deconfliction note so ponytail's structural rules and the peer plugin's style rules don't read as contradictory (ponytail#517). - Add a `ponytail-no-hallucination` companion skill that blocks invented APIs, deprecated methods, and undeclared variables (ponytail#504). - Tighten the simplification-marker rule so normal/trivial code no longer needs a `ponytail:` comment (ponytail#491), and add an explicit persona-boundary rule keeping ponytail/lazy framing out of user-facing replies (ponytail#521). - Both of the above were previously only written into a dead `fallback_instructions()` function that `build()` never called; moved the content into the live `skill.md` and deleted the dead function instead of leaving it to drift out of sync. - Stop silently swallowing state-persistence I/O errors in src/state.rs — now logs to stderr on failure (ponytail#303).
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThis PR adds a new ChangesNo-hallucination skill and plugin coexistence
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
crates/ponytail/src/skill-help.md (1)
34-38: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winStale count: "all six" should be "all seven".
Adding
ponytail-no-hallucinationbrings the skill table to seven entries, but line 38 still refers to "Claude Code and OpenCode use the slash-command forms above (OpenCode ships all six as slash commands)."📝 Proposed fix
-Codex uses `@ponytail`, `@ponytail-review`, and `@ponytail-help`; Claude Code -and OpenCode use the slash-command forms above (OpenCode ships all six as -slash commands). +Codex uses `@ponytail`, `@ponytail-review`, and `@ponytail-help`; Claude Code +and OpenCode use the slash-command forms above (OpenCode ships all seven as +slash commands).🤖 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 `@crates/ponytail/src/skill-help.md` around lines 34 - 38, The documentation in skill-help.md has an outdated slash-command count after adding ponytail-no-hallucination. Update the sentence near the skills table so it refers to “all seven” instead of “all six,” keeping the wording aligned with the current set of entries and the OpenCode slash-command note.
🤖 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 `@crates/ponytail/src/config.rs`:
- Around line 137-154: The env-mutating tests around detect_compression_plugins
are flaky because they change process-global variables like CLAUDE_CONFIG_DIR,
PONYTAIL_DEFAULT_MODE, and the new instructions.rs case while tests run in
parallel. Update each test that calls std::env::set_var/remove_var in config.rs
and the related instructions.rs test to use a shared serialization guard, such
as a mutex or #[serial], so only one env-changing test runs at a time. Keep the
fix scoped to the affected test functions so the existing behavior of
detect_compression_plugins and the other env-based checks remains unchanged.
In `@src/cli/ponytail.rs`:
- Around line 193-205: The Report branch in ponytail::SwitchAction::Report
incorrectly labels an "off" mode as active when building the ctx string. Update
this branch to mirror the existing
active_mode().unwrap_or_else(ponytail::default_mode) handling used in emit_hook
and the Statusline handler by special-casing mode == "off" before formatting the
output, so the message reflects an off state instead of "PONYTAIL MODE ACTIVE".
---
Outside diff comments:
In `@crates/ponytail/src/skill-help.md`:
- Around line 34-38: The documentation in skill-help.md has an outdated
slash-command count after adding ponytail-no-hallucination. Update the sentence
near the skills table so it refers to “all seven” instead of “all six,” keeping
the wording aligned with the current set of entries and the OpenCode
slash-command note.
🪄 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: CHILL
Plan: Pro Plus
Run ID: 1ca7d3ee-8179-4c06-83f9-414796b0640e
📒 Files selected for processing (9)
crates/ponytail/src/config.rscrates/ponytail/src/instructions.rscrates/ponytail/src/skill-help.mdcrates/ponytail/src/skill-no-hallucination.mdcrates/ponytail/src/skill.mdcrates/ponytail/src/sub_skills.rscrates/ponytail/src/switcher.rssrc/cli/ponytail.rssrc/state.rs
- Report a proper off-state message instead of "PONYTAIL MODE ACTIVE — level: off" when the configured default mode is off; extracted the mode->message mapping into report_message() with unit tests. - Serialize the env-var-mutating tests in config.rs/instructions.rs behind a shared ENV_TEST_LOCK mutex (mirrors PATH_LOCK in agent-registry) so parallel cargo test runs can't race on CLAUDE_CONFIG_DIR / PONYTAIL_DEFAULT_MODE. - Fix stale "all six" skill count in skill-help.md now that ponytail-no-hallucination brings it to seven.
Merged
4 tasks
Merged
3 tasks
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes the remaining gaps identified in #61 (audit of ponytail upstream PRs relevant to this Rust port). Of the 18 items audited, 10 were already done and 1 (CRLF handling in #3) doesn't apply to this port. This PR addresses the rest:
/ponytail(and/ponytail status) now actually reports the active mode viaUserPromptSubmitadditionalContextinstead of being a no-op that only echoedOK(ponytail#99).settings.jsonand append a deconfliction note so ponytail's structural rules and the peer plugin's style rules don't read as contradictory (ponytail#517).ponytail-no-hallucinationcompanion skill that blocks invented APIs, deprecated methods, and undeclared variables (ponytail#504).ponytail:comment (ponytail#491).KNOWN_ISSUES.md) — Stop silently swallowing state-persistence I/O errors insrc/state.rs; now logs to stderr on failure (ponytail#303).Note on #11/#16/#18: this wording previously existed only in a dead
fallback_instructions()function ininstructions.rsthatbuild()never called, so it never reached the live skill. Moved the content into the actually-shippedskill.mdand deleted the dead function rather than let it keep drifting out of sync.Test plan
cargo build --workspacecargo test --workspace(all 272 tests pass, including new coverage for the report/deconfliction/no-hallucination-skill behavior)cargo clippy --workspace --all-targets— no new warnings introduced by this changeSummary by CodeRabbit
New Features
/ponytail-no-hallucination.Bug Fixes
Documentation