Repository navigation
docs(skills): Add symptom-based troubleshooting guidance - #2469
karangattu wants to merge 27 commits into
Conversation
…n effects, and specify shiny run for verification
…ation events, and runtime verification
…nd ensure mutation examples obey session isolation
…active restrictions, and prevent verification hangs
cpsievert
left a comment
cpsievert
left a comment
There was a problem hiding this comment.
Five comments are inline on the diff. Two more below target files that are not in this PR, so they live here in the review body.
A pointer back from shiny-for-python (shiny/.agents/skills/shiny-for-python/SKILL.md, the index tables — not in this diff)
Right now nothing in shiny-for-python mentions shiny-doctor. Routing works in one direction only. What do you think about one line in the index, something like "To audit an existing app for quality, use the shiny-doctor skill"? It helps an agent that loaded the wrong skill find the right one.
agent-skills.md needs an update in this PR (.claude/references/agent-skills.md, the opening paragraph and "Layout" — not in this diff)
This doc says the package ships "a single Agent Skill" and "There is one skill directory". It also says "Adding a new topic does not mean creating a new top-level skill directory." This PR adds a second skill, which I think is reasonable: auditing an existing app is a different trigger than building one. But the doc now contradicts the repo. Can we update it here, in the same PR? A short paragraph on when a second skill is warranted, versus adding a references/<topic>.md file, will help the next contributor decide without guessing.
… streamline router
…er references test
|
I ran skilldiff using
using
|
I could use some help understanding more of the surrounding context here. Is this something that has gotten worse with our recent changes? These look like different apps than what you included in the PR description? Generally speaking, it's not totally surprising to me that using a skill would be more costly, but hopefully the result is better (potentially in a way that's hard for the LLM to actually measure)? |
I was curious why that is happening, so I had to look into the logs
|
|
If we do not do the traversal across 2 different skills using path, the results get better slightly.
|
cpsievert
left a comment
cpsievert
left a comment
There was a problem hiding this comment.
My recommendation: consolidate the three new files into a single references/troubleshooting.md, and move the antipattern examples into the "Common mistakes" sections of the existing topic references.
Concretely:
- Distribute
antipatterns.md— sections 1–4 go toreactivity.md, 5 tosession-lifecycle.md, 6 toextended-tasks.md, 7–8 to an outputs/core reference, 9 to the modules references, 10–11 toexpress.md. Each pitfall then lives where the agent is already working when it's most likely to make the mistake. - Keep one
troubleshooting.md— the symptom → cause → fix table, with rows pointing into the topic references, plus the runtime-verification workflow. One new row in SKILL.md: "An existing app misbehaves — blank outputs, stale values, cross-session leakage, frozen sessions, ID mismatches." - Drop or fold in the checklist — it's a third restatement of the same items.
Why I think this works better:
- It removes duplication. Most of
antipatterns.mdalready exists in the skill's "Common mistakes" sections — §3 is inreactivity.md, §5 insession-lifecycle.md, §6 inextended-tasks.md, §9 inmodules-core.md, §10 inexpress.md. This PR even adds the same Express shared-objects guidance to bothexpress.mdandantipatterns.md. Two homes for the same content will drift. - "Troubleshooting" is a better trigger than "Shiny Doctor." Every other reference is named for a task; a persona doesn't tell the agent when to load the file. A symptom-first row ("an existing app misbehaves") is something the agent can actually match against the user's situation.
- One file instead of three means one obvious place to look, and less to maintain.
To be clear about what I'd keep as-is: the content itself is good — the uncalled-reactive, in-place-mutation, @output(id=...), and R-syntax material is genuinely new, and the verification workflow with its "Runtime Verified / Static Diagnosis Only" reporting levels is excellent. This is purely about placement. I'd also draw one explicit boundary while you're in there: troubleshooting routes from symptoms, debugging.md is the technique for observing live state, testing verifies the fix.
karangattu
changed the title
karangattu
changed the title
|
using
|
|
When running with
|
This branch has not been deployed
2 participants
Summary
The bundled
shiny-for-pythonAgent Skill now routes misbehaving apps to symptom-based troubleshooting guidance. Agents can find likely causes for blank outputs, stale values, frozen sessions, and state leaks, read the relevant topic guide, then verify the repair through a connected browser session.Key changes
references/troubleshooting.mdwith a symptom index and a repair/verification workflow. Distinguish symptom diagnosis from observing live state and testing a fix.dynamic-ui.mdCommon mistakes section; route blank-output and duplicate-ID symptoms there.reactivity.md, state isolation insession-lifecycle.md, blocking-work guidance inextended-tasks.md, and R-to-Python syntax inexpress.md. Link to the existing module guidance for namespace issues.Earlier SkillDiff evaluation (control vs treatment)
This preliminary comparison evaluated the earlier standalone
shiny-doctorskill, before its guidance was integrated intoshiny-for-python. It usedgpt-6-solwith one control/treatment pair per task.Each arm received a fresh copy of the same broken app. The grader was outside the fixture and checked connected Shiny browser sessions after the agent finished.
These three pairs reached the same correctness scores. Treatment used fewer tokens and finished sooner in each pair. This small comparison does not establish performance gains for the integrated skill.
Validation
uv run --no-sync pytest tests/pytest/test_agent_skills_sync.py tests/pytest/test_main_skills.py tests/pytest/test_packaging.py -n 0: 15 passed, including a fresh wheel build and the reference-index contract.test_server; the expected output matched.git diff --checkpassed.