RFC: --quick mode with static renderer + fallback [DO NOT MERGE]#12
RFC: --quick mode with static renderer + fallback [DO NOT MERGE]#12mikeyobrien wants to merge 1 commit intonicobailon:mainfrom
Conversation
- add quick static renderer pipeline (base.css + render-architecture.mjs + schema docs) - document quick mode in SKILL and README while preserving default full-html behavior - wire --quick routing + fallback instructions into generate-web-diagram, diff-review, plan-review, and project-recap prompts - improve quick CSS overflow handling to reduce clipped content on dense outputs
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 407373e35b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| 2. Read `./quick/README.md` for the JSON schema and optional rich fields. | ||
| 3. Generate a **JSON spec** (not HTML) for the architecture renderer and save it to `/tmp/ve-quick-<timestamp>.json`. | ||
| 4. Render HTML using: | ||
| - `node $HOME/.pi/agent/skills/visual-explainer/quick/render-architecture.mjs /tmp/ve-quick-<timestamp>.json ~/.agent/diagrams/<descriptive-name>-quick.html` |
There was a problem hiding this comment.
Resolve quick renderer path without Pi-only hard-coding
This quick-mode command is pinned to $HOME/.pi/agent/skills/..., so --quick rendering breaks for non-Pi installs (for example the README’s Claude install path ~/.claude/skills/visual-explainer) and immediately falls back to full generation, effectively disabling the new feature in those environments. Please derive the renderer path from the loaded skill location (or handle agent-specific install roots) instead of using a Pi-specific absolute path; the same pattern is repeated in prompts/diff-review.md, prompts/plan-review.md, and prompts/project-recap.md.
Useful? React with 👍 / 👎.
|
Marking this as RFC / feedback-only for now — not intended to merge yet. I’m looking for focused feedback on:
If you can, please comment with Approve direction / Blocker / Nice-to-have so I can triage follow-ups cleanly. |
|
Love the visual-explainer skill - only painpoint is how long it takes to generate. The intent here is trade-off the visual aesthetic for faster generation time. Curious what your thoughts are. |
|
Created paired tracking issue for structured feedback + acceptance checklist: #13 |
Summary
Adds an opt-in
--quickmode to visual-explainer while preserving the original mechanism as default.--quickroutes through JSON spec + static renderer.What changed
quick/render-architecture.mjsquick/base.cssquick/README.mdSKILL.mdREADME.md--quickinstructions into prompts:prompts/generate-web-diagram.mdprompts/diff-review.mdprompts/plan-review.mdprompts/project-recap.mdBenchmark data
Measured on 5-cycle runs:
Token/output reduction vs baseline:
Parity results (v2):
E2E validation
Verified end-to-end quick mode generation for:
/generate-web-diagram --quick .../diff-review --quick .../plan-review --quick .../project-recap --quick ...Also verified full mode remains active when
--quickis omitted.Notes