feat: add question-render fenced markers and Claude transformer#2186
feat: add question-render fenced markers and Claude transformer#2186Rohan-1920 wants to merge 5 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. |
Made-with: Cursor
…ble flag Made-with: Cursor
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
| if agent_config["extension"] == "/SKILL.md": | ||
| output = self.render_skill_command( | ||
| agent_name, | ||
| output_name, | ||
| frontmatter, | ||
| body, | ||
| source_id, | ||
| cmd_file, | ||
| project_root, | ||
| agent_name, output_name, frontmatter, body, source_id, cmd_file, project_root | ||
| ) | ||
| elif agent_config["format"] == "markdown": | ||
| output = self.render_markdown_command( | ||
| frontmatter, body, source_id, context_note | ||
| ) | ||
| output = self.render_markdown_command(frontmatter, body, source_id, context_note) | ||
| elif agent_config["format"] == "toml": | ||
| output = self.render_toml_command(frontmatter, body, source_id) | ||
| elif agent_config["format"] == "yaml": | ||
| output = self.render_yaml_command( | ||
| frontmatter, body, source_id, cmd_name | ||
| ) | ||
| else: | ||
| raise ValueError(f"Unsupported format: {agent_config['format']}") |
There was a problem hiding this comment.
register_commands() no longer supports agents with format: "yaml" (e.g. Goose). With the render_yaml_command branch removed, Goose registrations will now raise ValueError("Unsupported format: yaml") and extension/preset installs that rely on CommandRegistrar will break. Please restore YAML support (e.g., reintroduce a YAML renderer that delegates to YamlIntegration._render_yaml() like the TOML path does for TOML).
| alias_file = commands_dir / f"{alias_output_name}{agent_config['extension']}" | ||
| alias_file.parent.mkdir(parents=True, exist_ok=True) | ||
| alias_file.write_text(alias_output, encoding="utf-8") |
There was a problem hiding this comment.
Writing alias command files no longer validates that the computed output path stays within commands_dir. Because extension command aliases are intentionally free-form (no pattern enforcement), a crafted alias containing path separators/.. segments can escape the agent commands directory and overwrite arbitrary files under the project root. Please reintroduce the resolve().relative_to(commands_dir.resolve()) guard (and consider applying the same guard to the primary dest_file as well).
| # and only run when explicitly invoked (not auto-triggered by the model). | ||
| skill_frontmatter["user-invocable"] = True | ||
| skill_frontmatter["disable-model-invocation"] = True |
There was a problem hiding this comment.
build_skill_frontmatter() sets disable-model-invocation to True for Claude, but the Claude post-processing pipeline (and existing tests) enforce disable-model-invocation: false. This inconsistency means any SKILL.md generated via this helper without running Claude post-processing will have the wrong behavior. Please align the default frontmatter with the post-processing contract (set it to False or omit it here and let the Claude extension set it consistently).
| # and only run when explicitly invoked (not auto-triggered by the model). | |
| skill_frontmatter["user-invocable"] = True | |
| skill_frontmatter["disable-model-invocation"] = True | |
| # and align with the Claude post-processing/test contract. | |
| skill_frontmatter["user-invocable"] = True | |
| skill_frontmatter["disable-model-invocation"] = False |
|
I still see src/specify_cli/core/renderer_extensions/init.py as part of the PR? |
🚀 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