Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 9 additions & 12 deletions src/apm_cli/commands/compile/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -154,8 +154,8 @@ def _get_validation_suggestion(error_msg):
"""Get actionable suggestions for validation errors."""
if "Missing 'description'" in error_msg:
return "Add 'description: Your description here' to frontmatter"
elif "Missing 'applyTo'" in error_msg:
return "Add 'applyTo: \"**/*.py\"' to frontmatter"
elif "applyTo" in error_msg and "globally" in error_msg:
return "Add 'applyTo: \"**/*.py\"' to scope the instruction, or leave as-is for global"
elif "Empty content" in error_msg:
return "Add markdown content below the frontmatter"
else:
Expand Down Expand Up @@ -529,16 +529,13 @@ def compile(
else:
_display_next_steps(output)

# Common error handling for both compilation modes
# Note: Warnings are handled by professional formatters for distributed mode
if config.strategy != "distributed" or single_agents:
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure about this line, but it took me some time to realize that I was missing the applyTo field. The apm compile wasn't adding the instruction to the AGENTS/CLAUDE.md, and I didn't know why.

# Only show warnings for single-file mode (backward compatibility)
if result.warnings:
logger.warning(
f"Compilation completed with {len(result.warnings)} warnings:"
)
for warning in result.warnings:
logger.warning(f" {warning}")
# Display warnings for all compilation modes
if result.warnings:
logger.warning(
f"Compilation completed with {len(result.warnings)} warning(s):"
)
for warning in result.warnings:
logger.warning(f" {warning}")

if result.errors:
logger.error(f"Compilation failed with {len(result.errors)} errors:")
Expand Down
14 changes: 10 additions & 4 deletions src/apm_cli/compilation/agents_compiler.py
Original file line number Diff line number Diff line change
Expand Up @@ -253,6 +253,9 @@ def _compile_distributed(self, config: CompilationConfig, primitives: PrimitiveC
"""
from .distributed_compiler import DistributedAgentsCompiler

errors = self.validate_primitives(primitives)
self.errors.extend(errors)

# Create distributed compiler with exclude patterns
distributed_compiler = DistributedAgentsCompiler(
str(self.base_dir),
Expand Down Expand Up @@ -317,8 +320,8 @@ def _compile_distributed(self, config: CompilationConfig, primitives: PrimitiveC
success=True,
output_path="Preview mode - no files written",
content=self._generate_placement_summary(distributed_result),
warnings=distributed_result.warnings,
errors=distributed_result.errors,
warnings=self.warnings + distributed_result.warnings,
errors=self.errors + distributed_result.errors,
stats=distributed_result.stats
)

Expand Down Expand Up @@ -405,6 +408,9 @@ def _compile_claude_md(self, config: CompilationConfig, primitives: PrimitiveCol
Returns:
CompilationResult: Result of the CLAUDE.md compilation.
"""
errors = self.validate_primitives(primitives)
self.errors.extend(errors)

# Create Claude formatter
claude_formatter = ClaudeFormatter(str(self.base_dir))

Expand Down Expand Up @@ -435,8 +441,8 @@ def _compile_claude_md(self, config: CompilationConfig, primitives: PrimitiveCol
# not at compile time. This keeps behavior consistent with VSCode prompt integration.

# Merge warnings and errors (no command result anymore)
all_warnings = claude_result.warnings
all_errors = claude_result.errors
all_warnings = self.warnings + claude_result.warnings
all_errors = self.errors + claude_result.errors

# Handle dry-run mode
if config.dry_run:
Expand Down
2 changes: 1 addition & 1 deletion src/apm_cli/primitives/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ def validate(self) -> List[str]:
if not self.description:
errors.append("Missing 'description' in frontmatter")
if not self.apply_to:
errors.append("Missing 'applyTo' in frontmatter (required for instructions)")
errors.append("No 'applyTo' pattern specified -- instruction will apply globally")
if not self.content.strip():
errors.append("Empty content")
return errors
Expand Down
88 changes: 86 additions & 2 deletions tests/unit/compilation/test_compilation.py
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,29 @@ def test_validate_primitives(self):
errors = compiler.validate_primitives(primitives)
self.assertEqual(len(errors), 0)

def test_validate_primitives_warns_on_missing_apply_to(self):
"""Test that validate_primitives adds a warning when applyTo is missing."""
compiler = AgentsCompiler(str(self.temp_path))

primitives = PrimitiveCollection()
instruction = Instruction(
name="no-scope",
file_path=self.temp_path / "no-scope.instructions.md",
description="Instruction without applyTo",
apply_to="",
content="Some content",
author="test",
)
primitives.add_primitive(instruction)

errors = compiler.validate_primitives(primitives)
self.assertEqual(len(errors), 0)
self.assertTrue(len(compiler.warnings) > 0)
self.assertTrue(
any("applyTo" in w for w in compiler.warnings),
f"Expected a warning mentioning 'applyTo', got: {compiler.warnings}",
)

@patch('apm_cli.primitives.discovery.discover_primitives')
def test_compile_with_mock_primitives(self, mock_discover):
"""Test compilation with mocked primitives."""
Expand Down Expand Up @@ -185,6 +208,67 @@ def test_compile_with_mock_primitives(self, mock_discover):
self.assertIn("Files matching `**/*.py`", result.content)
self.assertIn("Use type hints.", result.content)

def test_distributed_compile_includes_validation_warnings(self):
"""Test that distributed compilation surfaces warnings for missing applyTo."""
primitives = PrimitiveCollection()

good_instruction = Instruction(
name="good",
file_path=self.temp_path / "good.instructions.md",
description="Valid instruction",
apply_to="**/*.py",
content="Follow PEP 8.",
author="test",
)
bad_instruction = Instruction(
name="bad",
file_path=self.temp_path / "bad.instructions.md",
description="Missing applyTo",
apply_to="",
content="This has no scope.",
author="test",
)
primitives.add_primitive(good_instruction)
primitives.add_primitive(bad_instruction)

compiler = AgentsCompiler(str(self.temp_path))
config = CompilationConfig(
dry_run=True, resolve_links=False, strategy="distributed", target="agents"
)

result = compiler.compile(config, primitives)

self.assertTrue(
any("applyTo" in w for w in result.warnings),
f"Expected a warning about missing 'applyTo', got: {result.warnings}",
)

def test_claude_md_compile_includes_validation_warnings(self):
"""Test that CLAUDE.md compilation surfaces warnings for missing applyTo."""
primitives = PrimitiveCollection()

bad_instruction = Instruction(
name="no-scope",
file_path=self.temp_path / "no-scope.instructions.md",
description="Missing applyTo",
apply_to="",
content="This has no scope.",
author="test",
)
primitives.add_primitive(bad_instruction)

compiler = AgentsCompiler(str(self.temp_path))
config = CompilationConfig(
dry_run=True, resolve_links=False, target="claude"
)

result = compiler.compile(config, primitives)

self.assertTrue(
any("applyTo" in w for w in result.warnings),
f"Expected a warning about missing 'applyTo', got: {result.warnings}",
)

def test_compile_agents_md_function(self):
"""Test the standalone compile function."""
# Create test primitives
Expand Down Expand Up @@ -319,8 +403,8 @@ def test_validate_mode_with_valid_primitives(self):
suggestion = _get_validation_suggestion("Missing 'description' in frontmatter")
self.assertIn("Add 'description:", suggestion)

suggestion = _get_validation_suggestion("Missing 'applyTo' in frontmatter")
self.assertIn("Add 'applyTo:", suggestion)
suggestion = _get_validation_suggestion("No 'applyTo' pattern specified -- instruction will apply globally")
self.assertIn("applyTo", suggestion)

suggestion = _get_validation_suggestion("Empty content")
self.assertIn("Add markdown content", suggestion)
Expand Down
60 changes: 60 additions & 0 deletions tests/unit/compilation/test_compile_target_flag.py
Original file line number Diff line number Diff line change
Expand Up @@ -847,3 +847,63 @@ def test_cli_override_takes_precedence(self, temp_project_with_config):
assert config.target == "vscode"
finally:
os.chdir(original_dir)


class TestCompileWarningOnMissingApplyTo:
"""Tests that apm compile warns when an instruction is missing applyTo."""

@pytest.fixture
def runner(self):
return CliRunner()

@pytest.fixture
def project_with_bad_instruction(self):
temp_dir = tempfile.mkdtemp()
temp_path = Path(temp_dir)

(temp_path / "apm.yml").write_text("name: test-project\nversion: 0.1.0\n")

apm_dir = temp_path / ".apm" / "instructions"
apm_dir.mkdir(parents=True)

(apm_dir / "good.instructions.md").write_text(
"---\napplyTo: '**/*.py'\n---\nFollow PEP 8.\n"
)
(apm_dir / "bad.instructions.md").write_text(
"---\ndescription: Missing applyTo\n---\nThis instruction has no scope.\n"
)

yield temp_path
shutil.rmtree(temp_dir, ignore_errors=True)

def test_cli_warns_missing_apply_to_distributed(
self, runner, project_with_bad_instruction
):
"""Test that apm compile --dry-run warns about missing applyTo in distributed mode."""
original_dir = os.getcwd()
try:
os.chdir(project_with_bad_instruction)
result = runner.invoke(
cli, ["compile", "--target", "vscode", "--dry-run"]
)
Comment on lines +883 to +888
Copy link

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These CLI tests mutate the process working directory, which is easy to get wrong and makes tests harder to compose. Prefer pytest’s monkeypatch.chdir(project_with_bad_instruction) (or CliRunner.isolated_filesystem() plus constructing the project inside it) to keep the cwd change scoped and automatically restored.

Copilot uses AI. Check for mistakes.
assert "applyTo" in result.output, (
f"Expected warning about missing 'applyTo' in CLI output, got:\n{result.output}"
)
finally:
os.chdir(original_dir)

def test_cli_warns_missing_apply_to_claude(
self, runner, project_with_bad_instruction
):
"""Test that apm compile --target claude --dry-run warns about missing applyTo."""
original_dir = os.getcwd()
try:
os.chdir(project_with_bad_instruction)
result = runner.invoke(
cli, ["compile", "--target", "claude", "--dry-run"]
)
assert "applyTo" in result.output, (
f"Expected warning about missing 'applyTo' in CLI output, got:\n{result.output}"
)
finally:
os.chdir(original_dir)
Comment on lines +880 to +909
Copy link

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These CLI tests mutate the process working directory, which is easy to get wrong and makes tests harder to compose. Prefer pytest’s monkeypatch.chdir(project_with_bad_instruction) (or CliRunner.isolated_filesystem() plus constructing the project inside it) to keep the cwd change scoped and automatically restored.

Suggested change
self, runner, project_with_bad_instruction
):
"""Test that apm compile --dry-run warns about missing applyTo in distributed mode."""
original_dir = os.getcwd()
try:
os.chdir(project_with_bad_instruction)
result = runner.invoke(
cli, ["compile", "--target", "vscode", "--dry-run"]
)
assert "applyTo" in result.output, (
f"Expected warning about missing 'applyTo' in CLI output, got:\n{result.output}"
)
finally:
os.chdir(original_dir)
def test_cli_warns_missing_apply_to_claude(
self, runner, project_with_bad_instruction
):
"""Test that apm compile --target claude --dry-run warns about missing applyTo."""
original_dir = os.getcwd()
try:
os.chdir(project_with_bad_instruction)
result = runner.invoke(
cli, ["compile", "--target", "claude", "--dry-run"]
)
assert "applyTo" in result.output, (
f"Expected warning about missing 'applyTo' in CLI output, got:\n{result.output}"
)
finally:
os.chdir(original_dir)
self, runner, project_with_bad_instruction, monkeypatch
):
"""Test that apm compile --dry-run warns about missing applyTo in distributed mode."""
monkeypatch.chdir(project_with_bad_instruction)
result = runner.invoke(cli, ["compile", "--target", "vscode", "--dry-run"])
assert "applyTo" in result.output, (
f"Expected warning about missing 'applyTo' in CLI output, got:\n{result.output}"
)
def test_cli_warns_missing_apply_to_claude(
self, runner, project_with_bad_instruction, monkeypatch
):
"""Test that apm compile --target claude --dry-run warns about missing applyTo."""
monkeypatch.chdir(project_with_bad_instruction)
result = runner.invoke(cli, ["compile", "--target", "claude", "--dry-run"])
assert "applyTo" in result.output, (
f"Expected warning about missing 'applyTo' in CLI output, got:\n{result.output}"
)

Copilot uses AI. Check for mistakes.
2 changes: 1 addition & 1 deletion tests/unit/primitives/test_primitives.py
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ def test_instruction_validation(self):
)
self.assertEqual(instruction.validate(), [])

# Missing applyTo (required for instructions)
# Missing applyTo (instruction will apply globally)
instruction_no_apply = Instruction(
name="test",
file_path=Path("test.instructions.md"),
Expand Down
Loading