feat: add question-render fenced markers and Claude transformer#2186
feat: add question-render fenced markers and Claude transformer#2186Rohan-1920 wants to merge 2 commits intogithub:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Adds a Claude-specific post-processing layer that converts fenced “question table” blocks in generated SKILL.md files into structured JSON payloads intended for Claude’s native question UI, while also adjusting skill placeholder resolution logic for cross-platform script variants.
Changes:
- Added HTML comment fence markers around question tables in
clarify.mdandchecklist.mdtemplates. - Introduced
src/specify_cli/core/question_transformer.pyto detect fenced blocks, parse markdown tables, and emit a JSON payload block. - Updated Claude integration setup to run the transformer during skill installation; updated
{SCRIPT}/{AGENT_SCRIPT}placeholder resolution fallback logic.
Show a summary per file
| File | Description |
|---|---|
| tests/test_question_transformer.py | New unit tests for table parsing and fenced-block transformation behavior |
| tests/integrations/test_integration_claude.py | New integration tests asserting Claude skills contain transformed JSON and no raw markers |
| templates/commands/clarify.md | Adds begin/end markers around the clarify question table example |
| templates/commands/checklist.md | Adds begin/end markers around the checklist question table example |
| src/specify_cli/integrations/claude/init.py | Runs question block transformation when generating Claude SKILL.md files |
| src/specify_cli/core/question_transformer.py | New transformer/parser implementation for fenced question blocks |
| src/specify_cli/core/init.py | Adds core package initializer |
| src/specify_cli/agents.py | Updates script variant fallback selection for {SCRIPT} / {AGENT_SCRIPT} placeholders |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comments suppressed due to low confidence (2)
src/specify_cli/agents.py:366
fallback_orderis only built wheninit_opts['script']is not in {"sh","ps"}. If the user explicitly setsscript: ps(orsh) but that variant is missing fromscriptsoragent_scripts, both{SCRIPT}/{AGENT_SCRIPT}can fail to resolve because the later fallback loop has an emptyfallback_order. Build a fallback list whenever the chosen variant isn’t available (or always include the other available variants), so placeholder resolution works even with a mismatched/partial set of script variants.
script_variant = init_opts.get("script")
fallback_order: list[str] = []
if script_variant not in {"sh", "ps"}:
# Build fallback order: prefer the variant present in BOTH scripts and
# agent_scripts so that {SCRIPT} and {AGENT_SCRIPT} resolve consistently.
# On Windows the OS default is "ps", but if only "sh" is available in
# agent_scripts we must still resolve both placeholders from "sh".
default_variant = "ps" if platform.system().lower().startswith("win") else "sh"
secondary_variant = "sh" if default_variant == "ps" else "ps"
# Prefer a variant that satisfies both scripts AND agent_scripts.
both_variants = set(scripts) & set(agent_scripts)
if both_variants:
for v in (default_variant, secondary_variant):
if v in both_variants:
fallback_order.append(v)
for v in sorted(both_variants):
if v not in fallback_order:
fallback_order.append(v)
# Then add remaining variants from scripts / agent_scripts.
for v in (default_variant, secondary_variant):
if v not in fallback_order and (v in scripts or v in agent_scripts):
fallback_order.append(v)
for key in list(scripts) + list(agent_scripts):
if key not in fallback_order:
fallback_order.append(key)
script_variant = fallback_order[0] if fallback_order else None
# Resolve script_command: try script_variant first, then walk fallback_order.
# This ensures sh-only extensions work on Windows (where default is "ps").
script_command = scripts.get(script_variant) if script_variant else None
if not script_command:
for _variant in fallback_order:
candidate = scripts.get(_variant)
if candidate:
script_command = candidate
break
if script_command:
script_command = script_command.replace("{ARGS}", "$ARGUMENTS")
body = body.replace("{SCRIPT}", script_command)
# Resolve agent_script_command: same cross-platform fallback.
agent_script_command = agent_scripts.get(script_variant) if script_variant else None
if not agent_script_command:
for _variant in fallback_order:
candidate = agent_scripts.get(_variant)
if candidate:
agent_script_command = candidate
break
if agent_script_command:
agent_script_command = agent_script_command.replace("{ARGS}", "$ARGUMENTS")
body = body.replace("{AGENT_SCRIPT}", agent_script_command)
src/specify_cli/core/question_transformer.py:135
- Checklist vs clarify detection uses a simple substring check for
| Candidate |inside the fenced block. This can mis-detect if the word “Candidate” appears in a clarify table description, and it’s sensitive to header capitalization/spacing. Consider detecting schema from the parsed header cell count/names (e.g., first non-empty table row) rather than a raw substring search.
def _replace(match: re.Match) -> str:
block = match.group(1)
is_checklist = "| Candidate |" in block or "|Candidate|" in block
options = parse_checklist(block) if is_checklist else parse_clarify(block)
return _build_payload(options)
- Files reviewed: 8/8 changed files
- Comments generated: 7
mnriem
left a comment
mnriem
left a comment
There was a problem hiding this comment.
Please deliver this as a community preset. See https://github.com/github/spec-kit/blob/main/presets/README.md?tab=readme-ov-file#presets--customize-existing-workflows
Rohan-1920
left a comment
Rohan-1920
left a comment
There was a problem hiding this comment.
Nice work on this feature 👏
- The fenced marker implementation looks clean and readable.
- Claude transformer integration is well-structured.
- Good to see test coverage included for the transformer logic.
Minor suggestion:
- Consider adding a brief docstring or comment explaining the regex (FENCE_RE) for future readability.
Overall, solid implementation 👍
mnriem
left a comment
mnriem
left a comment
There was a problem hiding this comment.
Please deliver this as an extension
Thanks for the feedback! Got it I’ll refactor this into an extension instead of integrating it directly into the core. I’ll update the PR once that’s done. |
Hi I’ve implemented the extension-based skill renderer refactor (registry + RendererExtension, question-render + Claude transforms in separate modules) and pushed it to feat/question-render-transformer on fork:
While rebasing I also restored a broken agents.py from main and added set_frontmatter_key so disable-model-invocation: Happy to open a PR against this repo if you want let me know the preferred base branch. |
|
Please see https://github.com/github/spec-kit/tree/main/extensions on how to host your own Spec Kit extension. Note that if you need to override what the templates do you are probably also looking for a preset. Note that anything specific to a AI coding agent integration should be solely delivered by it and not require any core changes |
Thanks for the guidance. You're right this feature is currently implemented in a way that affects core behavior. I'll refactor it so that the question rendering transformation is handled entirely within the Claude integration layer instead of the core system. If needed, I’ll also explore moving this into an extension/preset structure to better align with Spec Kit’s design principles. Will push an update shortly. |
|
Thanks for the feedback. I've moved the question rendering logic into the Claude integration layer and removed core-level changes. Please take another look. |
|
Still seeing numerous python files that touch the core of Spec Kit? |
|
There was a problem hiding this comment.
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 11/11 changed files
- Comments generated: 3
|
I still see src/specify_cli/core/renderer_extensions/init.py as part of the PR? |
|
Still seeing several python files that are not in the claude specific package and I see changes to the core templates that are Claude specific? |
Move question-render transformation and Claude skill post-processing into the Claude integration package so core/shared rendering stays generic while preserving current behavior. Made-with: Cursor
Rohan-1920
force-pushed
the
feat/question-render-transformer
branch
from
f7bac09 to
60181f0
Compare
|
Made-with: Cursor
There was a problem hiding this comment.
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 4/4 changed files
- Comments generated: 4
|
|
||
| def test_no_markers_returns_identical(self): | ||
| text = "hello world, no markers here" | ||
| assert transform_question_block(text) is not text or transform_question_block(text) == text |
There was a problem hiding this comment.
This assertion is effectively always true (A is not B or A == B) and also calls transform_question_block() twice, so it doesn’t meaningfully test the no-marker behavior. Consider replacing it with a single explicit assertion about the expected output (e.g., exact equality; and only add an identity assertion if the function is intended to return the original object).
| assert transform_question_block(text) is not text or transform_question_block(text) == text | |
| result = transform_question_block(text) | |
| assert result == text |
| entry = {"label": label, "description": description} | ||
| if _RECOMMENDED_RE.search(description): | ||
| if recommended is None: | ||
| recommended = entry |
There was a problem hiding this comment.
parse_clarify() drops any additional rows whose description matches the Recommended pattern after the first one (they're neither used as recommended nor appended to options). This can silently remove valid choices if more than one row is marked recommended. Consider appending subsequent recommended rows to options once recommended is already set (or supporting multiple recommended rows explicitly).
| recommended = entry | |
| recommended = entry | |
| else: | |
| options.append(entry) |
| """Question block transformer for Claude Code integration.""" | ||
|
|
||
| from __future__ import annotations |
There was a problem hiding this comment.
PR description calls out a new core module at src/specify_cli/core/question_transformer.py, but the implementation is currently located under src/specify_cli/integrations/claude/question_transformer.py. Either update the PR description (and any docs) or move/alias the module to match, to avoid confusion and broken links in follow-up work.
| from .skill_postprocess import ( | ||
| ARGUMENT_HINTS, | ||
| apply_claude_skill_postprocess, | ||
| inject_argument_hint, | ||
| inject_frontmatter_flag, | ||
| inject_hook_command_note, | ||
| set_frontmatter_key, | ||
| ) |
There was a problem hiding this comment.
ARGUMENT_HINTS is imported but not used in this module anymore (hint resolution moved into skill_postprocess.apply_claude_skill_postprocess). Consider dropping the unused import to keep the module tidy.
mnriem
left a comment
mnriem
left a comment
There was a problem hiding this comment.
Please address Copilot feedback. If not applicable, please explain why
mnriem
left a comment
mnriem
left a comment
There was a problem hiding this comment.
I do not see the Copilot feedback addressed?
🚀 PR: Add Claude-native AskUserQuestion Rendering Pipeline
Overview
AskUserQuestionpayloadsChanges
Templates
Added fenced markers to isolate question-rendering blocks in:
templates/commands/clarify.mdtemplates/commands/checklist.mdMarkers used:
<!-- speckit:question-render:begin --><!-- speckit:question-render:end -->No changes to existing logic or structure
Core Module
New file:
src/specify_cli/core/question_transformer.pyDetects fenced question blocks
Parses:
Option | Description(clarify)Option | Candidate | Why It Matters(checklist)Converts into
AskUserQuestionpayloadExtra behavior:
Claude Integration
src/specify_cli/integrations/claude/__init__.pyAgents Fix
src/specify_cli/agents.py{SCRIPT}/{AGENT_SCRIPT}resolution logicTests
Unit Tests
tests/test_question_transformer.pyIntegration Tests
tests/integrations/test_integration_claude.pyBackward Compatibility
Result
AskUserQuestionUI with structured selection and fallback option