Fix frontmatter parsing, verbose workaround matching, and doc alignment#4
Fix frontmatter parsing, verbose workaround matching, and doc alignment#4
Conversation
…c paths and patterns Co-authored-by: spazyCZ <22116740+spazyCZ@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR addresses parsing and matching issues in the AAM CLI conversion flow and aligns multiple documentation examples/diagrams with current behavior and supported conventions.
Changes:
- Fix YAML frontmatter parsing to locate the closing
---delimiter via a line scan instead of substring search. - Improve
--verboseworkaround hint matching by normalizing workaround keys to lowercase before searching warnings. - Update docs to reflect the correct Copilot
applyTofield, legacy Copilot instruction detection, and the actual git cache path.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| docs/user_docs/docs/platforms/copilot.md | Updates deployed Copilot instruction example to use applyTo instead of scope. |
| docs/user_docs/docs/mcp/index.md | Fixes MCP diagram cache path to ~/.aam/cache/git/. |
| docs/DESIGN.md | Documents legacy .github/copilot-instructions.md detection pattern. |
| apps/aam-cli/src/aam_cli/converters/frontmatter.py | Reworks frontmatter closing-delimiter detection to be line-based. |
| apps/aam-cli/src/aam_cli/commands/convert.py | Normalizes workaround keys to lowercase to improve matching in verbose mode. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| for key, workaround in VERBOSE_WORKAROUNDS.items(): | ||
| if key.replace("_", " ").replace("removed", "").strip() in warning_lower: | ||
| normalized_key = ( | ||
| key.replace("_", " ") | ||
| .replace("removed", "") | ||
| .strip() | ||
| .lower() | ||
| ) | ||
| if normalized_key and normalized_key in warning_lower: |
There was a problem hiding this comment.
Workaround matching is still brittle because it relies on the (normalized) workaround dict key being a substring of the warning text. Several current pairs don’t match (e.g., globs_lost vs "Glob-scoped instruction converted…", user_invokable_removed vs "user-invokable flag removed.", mcp_servers_removed vs "MCP server configuration…"). Consider standardizing warnings to include a stable token/id, or mapping explicit substrings/regexes instead of deriving from the key name.
| def _print_verbose_workaround(console: Console, warning: str) -> None: | ||
| """Print verbose workaround text for a warning if available.""" | ||
| warning_lower = warning.lower() | ||
|
|
||
| for key, workaround in VERBOSE_WORKAROUNDS.items(): | ||
| if key.replace("_", " ").replace("removed", "").strip() in warning_lower: | ||
| normalized_key = ( | ||
| key.replace("_", " ") | ||
| .replace("removed", "") | ||
| .strip() | ||
| .lower() | ||
| ) | ||
| if normalized_key and normalized_key in warning_lower: | ||
| console.print(f" [dim]{workaround}[/dim]") | ||
| return |
There was a problem hiding this comment.
There are CLI/unit tests for convert, but none assert that --verbose actually prints the expected workaround hint(s). A small test that triggers a known warning (e.g., alwaysApply dropped) and checks the hint output would cover this behavior.
| body = stripped[end_idx + 3:].lstrip("\n") | ||
| closing_idx: int | None = None | ||
| for i, line in enumerate(lines[1:], start=1): | ||
| if line.strip() == "---": |
There was a problem hiding this comment.
The closing frontmatter delimiter check uses line.strip() == "---", which will also treat indented --- lines (e.g., inside a YAML literal block) as a delimiter and truncate parsing. Delimiters should only match when --- starts at column 0 (optionally with trailing whitespace/newline).
| if line.strip() == "---": | |
| # Match closing delimiter only when '---' starts at column 0 | |
| # and is followed only by optional whitespace/newline. | |
| if line.startswith("---") and line[3:].strip() == "": |
| # Body is everything after the closing delimiter | ||
| body = "".join(lines[closing_idx + 1:]).lstrip("\n") |
There was a problem hiding this comment.
lstrip("\n") will leave a leading \r when parsing files with CRLF line endings, so body can start with \r\n. Consider stripping both \r and \n here (and keep behavior consistent across platforms).
| body = stripped[end_idx + 3:].lstrip("\n") | ||
| closing_idx: int | None = None | ||
| for i, line in enumerate(lines[1:], start=1): | ||
| if line.strip() == "---": |
There was a problem hiding this comment.
The new line-by-line delimiter scan changes parsing behavior but there’s no unit test covering the previously-broken case (e.g., a YAML value containing --- that should not terminate frontmatter). Adding a regression test would help prevent this from reappearing.
| if line.strip() == "---": | |
| # Only treat '---' as a closing delimiter when it appears at the start | |
| # of the line (column 0) with no other non-whitespace characters. | |
| if line.startswith("---") and line.strip() == "---": |
Five bugs/inconsistencies identified in PR review: incorrect case-sensitive matching in
_print_verbose_workaround, fragile frontmatter delimiter detection, and three doc misalignments.Code fixes
frontmatter.py— Replacestr.find('---', 3)with line-by-line scan; previously any---substring inside YAML values or body text would terminate parsing earlyconvert.py— Add.lower()to normalized key in_print_verbose_workaround(); VERBOSE_WORKAROUNDS keys like"alwaysApply"never matched lowercased warning strings, so--verboseworkaround hints were silently suppressedDoc fixes
mcp/index.md— Diagram showed~/.aam/sources-cache/; actual path is~/.aam/cache/git/DESIGN.md— Instructions detection table omitted.github/copilot-instructions.md; scanner still detects it — added for backward-compat accuracyplatforms/copilot.md— Deployed instruction example usedscope: project; Copilot consumesapplyTo— updated toapplyTo: "**"💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.