fix(claude): make argument-hint injection fold-aware for long descriptions - #4045
Open
chelsealong wants to merge 2 commits into
Open
fix(claude): make argument-hint injection fold-aware for long descriptions#4045chelsealong wants to merge 2 commits into
chelsealong wants to merge 2 commits into
Conversation
…tions ClaudeIntegration.inject_argument_hint spliced argument-hint: "..." as a raw text line right after the first line starting with "description:". When a description is long enough for the YAML dumper to fold it across indented continuation lines, that splice landed inside the scalar, producing invalid YAML (plain scalar) or silently absorbing the hint into the description string (quoted scalar). This reproduces github#3991 for the case github#3996 didn't cover: bundled core commands have no argument-hint in their source frontmatter, so the structural apply_argument_hint path is a no-op and this raw-text fallback is what actually runs. Skip every continuation line of the description scalar (anything more indented than the key itself) before inserting, so the new key always lands after the whole scalar ends rather than in the middle of it. Fixes github#4044
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes Claude skill frontmatter corruption when descriptions wrap across YAML lines.
Changes:
- Skips indented description continuation lines before injecting
argument-hint. - Adds regression tests for plain and quoted folded descriptions.
Show a summary per file
| File | Description |
|---|---|
src/specify_cli/integrations/claude/__init__.py |
Makes hint injection fold-aware. |
tests/integrations/test_integration_claude.py |
Tests folded description handling. |
Review details
Tip
Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
- Files reviewed: 2/2 changed files
- Comments generated: 1
- Review effort level: Balanced
| i += 1 | ||
| # Skip past folded/quoted continuation lines of the scalar | ||
| # before inserting, so the new key lands after it ends. | ||
| while i < n and lines[i][:1] in (" ", "\t"): |
PyYAML serializes an embedded paragraph break ("\n\n") inside a quoted
description as unindented blank lines, not indented continuation
lines. inject_argument_hint only skipped indented lines, so it still
inserted argument-hint mid-scalar for multi-paragraph descriptions,
reproducing the github#4044 failure modes. Skip blank lines too, and add a
regression test for the multi-paragraph case.
Contributor
Author
|
Fixed: |
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
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
Fixes #4044 —
ClaudeIntegration.inject_argument_hintsplicedargument-hint: "..."in as a raw text line immediately after the first line starting withdescription:. That's safe for a short, single-line description, but when a description is long enough for the YAML dumper to fold it across multiple indented continuation lines, the splice landed inside that scalar instead of after it:yaml.parser.ParserError).argument-hintdoesn't exist as a key — its line is silently absorbed into thedescriptionstring.This is the case #3996 didn't cover: bundled core commands carry no
argument-hintin their source frontmatter, soCommandRegistrar.apply_argument_hint's structural inheritance path is a no-op for them, and this raw-text fallback inpost_process_skill_contentis what actually runs for every core Claude skill.Fix
inject_argument_hintnow skips past every continuation line of thedescriptionscalar (anything more indented than thedescription:key itself — this covers both plain and quoted folded styles) before insertingargument-hint:, so the new key always lands after the whole scalar ends rather than in the middle of it. No change to the injected line's format (argument-hint: "<hint>"), so short, non-folding descriptions behave exactly as before.Test plan
Added two regression tests in
tests/integrations/test_integration_claude.py::TestClaudeArgumentHintsthat build frontmatter with a ~150+ char description viayaml.safe_dump(reproducing the fold from the issue) for both the plain and forced-quoted cases, run it throughinject_argument_hint, and assert the result is valid YAML withargument-hintpresent as its own key anddescriptionunchanged.Confirmed both new tests fail against the pre-fix code with exactly the two failure modes from the issue:
With the fix:
Full suite (
pytest tests -q): 6786 passed, 9 skipped, 10 failed — all 10 failures reproduce identically on unmodifiedmain(composed-template python/shell parity and a rich-markup width assertion, unrelated to this change and to the Claude integration).ruff checkon both changed files reports the same 17 pre-existing findings as on unmodifiedmain; none introduced by this diff.AI assistance disclosure
This PR was written by an autonomous AI coding agent (Claude, via an agent harness), including the diagnosis, fix, and regression tests. It was validated by running the reproduction from the issue, the existing and new test suites, and by confirming pre-existing failures reproduce unmodified on
main.