Fix #1019: Improve compile target specification

[tillig]

Description

Enables the ability to be more specific about compile targets so GEMINI.md does not get generated unless it's actually requested.

Fixes #1019

Type of change

• Bug fix
• New feature
• Documentation
• Maintenance / refactor

Testing

• Tested locally
• All existing tests pass
• Added tests for new functionality (if applicable)

[github-actions]

APM Review Panel Verdict

Disposition: APPROVE (one pre-merge doc-sync check; two optional follow-ups)

---

Per-persona findings

Python Architect: Bug is clear: multi-target lists (e.g. --target claude,codex) previously collapsed to the string "all", causing AgentsCompiler to generate AGENTS.md + CLAUDE.md + GEMINI.md even when only the first two were requested. The fix introduces CompileTargetType = Union[str, frozenset], constructs a frozenset of compiler-family names from multi-target CLI args, bypasses detect_target() for frozenset paths, and guards the three should_compile_* helpers with isinstance(target, frozenset) dispatch. This is a routine PR (one bug fix, no new abstract base, no hierarchy restructure).

classDiagram
    direction LR

    class CompilationConfig {
        <<Dataclass>>
        +target: CompileTargetType
        +dry_run: bool
        +single_agents: bool
        +from_apm_yml() CompilationConfig
    }
    class AgentsCompiler {
        +compile(config, primitives) CompilationResult
    }
    class CompileTargetType {
        <<TypeAlias>>
        Union~str, frozenset~
    }
    class TargetDetection {
        <<Module>>
        +detect_target(explicit_target, config_target) TargetType
        +should_compile_agents_md(target) bool
        +should_compile_claude_md(target) bool
        +should_compile_gemini_md(target) bool
    }
    class ResolveCompileTarget {
        <<Pure>>
        +_resolve_compile_target(target) CompileTargetType
    }
    class CompilationResult {
        <<ValueObject>>
        +success bool
        +output_path str
    }

    AgentsCompiler ..> CompilationConfig : reads
    AgentsCompiler ..> TargetDetection : delegates routing
    ResolveCompileTarget ..> CompileTargetType : returns
    CompilationConfig *-- CompileTargetType : typed field
    AgentsCompiler ..> CompilationResult : returns

    class CompilationConfig:::touched
    class TargetDetection:::touched
    class ResolveCompileTarget:::touched
    classDef touched fill:#fff3b0,stroke:#d47600

Loading

flowchart TD
    A["apm compile --target claude,codex"] --> B["_resolve_compile_target\ncli.py:_resolve_compile_target()"]
    B --> C{families >= 2?}
    C -- "pre-fix: returns 'all'" --> D["detect_target(explicit='all')"]
    D --> E["[FS] GEMINI.md written -- WRONG"]
    C -- "post-fix: returns frozenset" --> F["frozenset({'agents','claude'})"]
    F --> G{isinstance frozenset?\ncli.py:408}
    G -- yes --> H["bypass detect_target()\ndetection_reason='explicit --target flag'"]
    H --> I["should_compile_agents_md(frozenset) --> True\ntarget_detection.py:143"]
    H --> J["should_compile_claude_md(frozenset) --> True\ntarget_detection.py:158"]
    H --> K["should_compile_gemini_md(frozenset) --> False\ntarget_detection.py:173"]
    I --> L["[FS] AGENTS.md written"]
    J --> M["[FS] CLAUDE.md written"]
    K --> N["GEMINI.md NOT written -- correct"]
    G -- no (string) --> O["detect_target() string path\nunchanged from before"]

Loading

Design patterns

• Used in this PR: Type-based dispatch (isinstance(target, frozenset) guards) -- applied in three should_compile_* helpers (target_detection.py:143,158,173), in AgentsCompiler.compile() (agents_compiler.py:218), and in the compile command (cli.py:408,412). Six guards across three files for a two-value type union.
• Pragmatic suggestion: none -- six isinstance guards is appropriate for this scope. A _family_set(target) -> frozenset[str] coercer would only pay off if a third target representation is added.

Non-blocking observations:

1. CompileTargetType = Union[str, frozenset] should be Union[str, frozenset[str]] for Python 3.12 precision.
2. The frozenset routing path in AgentsCompiler.compile() skips the _KNOWN_TARGETS validation (lines 225-235), since the frozenset is always built from CLI-validated args -- not currently exploitable but worth a guard comment.
3. "no target" in detection_reason (cli.py:464) is a fragile string-match for the minimal-target progress message. A named constant (e.g. _REASON_NO_TARGET_FOLDER = "no target folder found") in target_detection.py would eliminate the coupling.

---

CLI Logging Expert: Output paths are clean. All progress messages route through logger.progress() (CommandLogger) -- no raw _rich_* calls in command scope. The new dynamic progress message (Compiling for AGENTS.md + CLAUDE.md (--target claude,codex)) is built from the same should_compile_* helpers used by the compiler itself, so the message will never drift from what is actually generated. ASCII compliance confirmed. One style nit: the local import of should_compile_* inside the if isinstance(target, list): branch (cli.py:450-454) is inconsistent with file-level imports -- minor, not a blocker. The "no target" in detection_reason substring at cli.py:464 is the same brittleness flagged by the Python Architect; if the detection-reason string ever changes, the "Compiling for AGENTS.md only" branch silently becomes dead code.

---

DevX UX Expert: Clear UX improvement. Before: apm compile --target claude,codex silently polluted the repo with an unrequested GEMINI.md -- a trust-eroding surprise, especially in CI. After: the command produces exactly what the user asked for, matching the mental model from cargo build --target and tsc --target. The progress message now names the actual output files, following the "name the thing" principle. --target all still works identically (tested). Single-target strings are unchanged. One pre-merge check: the docs (docs/src/content/docs/reference/cli-commands.md and packages/apm-guide/.apm/skills/apm-usage/commands.md) should be verified to not document the old "multi-target collapses to all" behavior. If they do, a one-line update is required per repo Rule 4.

---

Supply Chain Security Expert: CLEAN. No new path construction, no lockfile changes, no token or auth touch points. The frozenset is always constructed by _resolve_compile_target() from CLI-validated target strings (VALID_TARGET_VALUES gating is upstream). The only theoretical gap -- frozenset routing bypassing _KNOWN_TARGETS validation in AgentsCompiler.compile() -- is not exploitable through the current code path. No new attack surface introduced.

---

Auth Expert: Not activated -- PR modifies commands/compile/cli.py, compilation/agents_compiler.py, and core/target_detection.py, all exclusively in compile-target routing for markdown file generation; no auth, token management, credential resolution, or host classification code paths appear in the diff.

---

OSS Growth Hacker: This is a correctness prerequisite for the "one repo, multiple AI assistants" adoption story. Teams using APM across Claude + GitHub Copilot workflows (the most common multi-assistant setup) will immediately benefit: --target claude,copilot now produces exactly AGENTS.md + CLAUDE.md without an unexpected GEMINI.md showing up in PRs. Gemini is a growing target (Google Gemini CLI, Gemini Code Assist) -- getting its compile semantics right now positions APM well. The CHANGELOG entry is user-readable and accurate. Side-channel to CEO: consider bundling this with one or two other quality fixes into a v0.10.1 release post framed as "APM now compiles exactly the AI context files you request." Low friction, high enterprise-trust signal.

---

CEO arbitration

All specialists agree -- no disagreements to arbitrate. This is a correct, well-scoped bug fix with a proper regression test class (TestMultiTargetDoesNotGenerateUnrequestedFiles). The --target all regression is explicitly covered. The one pre-merge action is a quick docs sync check (Rule 4); the two follow-ups (type precision, detection-reason constant) are quality-of-life improvements that belong in a separate PR. The fix is strategically sound: multi-target compilation is a differentiator for teams with mixed AI toolchains, and getting the semantics right is table stakes for enterprise credibility. Ship it.

---

Required actions before merge

1. Doc sync check (Rule 4): Verify that docs/src/content/docs/reference/cli-commands.md and packages/apm-guide/.apm/skills/apm-usage/commands.md do not document the old "multi-target list collapses to all" behavior. If either file contains such a claim or example, update it to reflect the new frozenset semantics (one line each). If neither file mentions multi-target collapse, this item is resolved with no edit needed.

---

Optional follow-ups

• Type precision (target_detection.py:34): Change CompileTargetType = Union[str, frozenset] to Union[str, frozenset[str]] -- Python 3.12 supports the parameterized form natively, and it closes the door on non-string family names in future frozensets.
• Detection-reason coupling (cli.py:464): Extract "no target folder found" from target_detection.py:detect_target() into a named module-level constant (e.g. REASON_NO_TARGET_FOLDER) and reference it in the cli.py condition instead of "no target" in detection_reason. Eliminates the silent dead-code risk if the reason string ever changes.
• Frozenset validation guard (agents_compiler.py:218-219): Add a comment (or a runtime assertion) documenting that frozenset routing_target is guaranteed to contain only valid family names because _resolve_compile_target() only adds "agents", "claude", or "gemini". Makes the intentional skip of _KNOWN_TARGETS validation self-documenting.

Generated by PR Review Panel for issue #1020 · ● 935.9K · ◷

[danielmeppiel]

Pushed one follow-up commit (fb314d3) onto this branch addressing the three optional nits from the Review Panel, @tillig — feel free to revert/squash/amend as you see fit:

1. CompileTargetType: Union[str, frozenset] → Union[str, frozenset[str]] (target_detection.py)
2. Extracted REASON_NO_TARGET_FOLDER constant; replaced brittle "no target" in detection_reason substring match with equality comparison. User-facing log message unchanged.
3. Added _KNOWN_COMPILE_FAMILIES validation in AgentsCompiler — direct API callers passing frozenset({"bogus"}) now fail explicitly instead of silently no-op'ing. Plus regression test test_unknown_frozenset_target_family_returns_failure.

Notably declined the assert suggestion in favor of an explicit validation branch — assert is stripped under python -O and would surface as a generic Compilation failed: ..., strictly worse UX than the current explicit unknown-target error path. Used the same pattern that exists for the non-frozenset case ~10 lines below.

Doc sync (the required action): verified as a no-op — docs/src/content/docs/reference/cli-commands.md:1712 and packages/apm-guide/.apm/skills/apm-usage/commands.md:28 already document the post-fix multi-target behavior.

Tests: pytest tests/unit/compilation/ tests/unit/core/ tests/unit/commands/ -q → 806 passed in 5.50s.

[Copilot]

Pull request overview

Fixes #1019 by changing how multi-target apm compile --target lists are normalized so cross-family combinations no longer collapse to "all" (which caused unrequested artifacts like GEMINI.md to be generated).

Changes:

• Update compile target resolution to return a frozenset of requested compiler families for multi-target lists (instead of "all").
• Extend target detection helpers and compiler routing to understand family frozenset targets.
• Add/adjust unit tests and update the changelog entry for the regression.

Show a summary per file

File	Description
src/apm_cli/commands/compile/cli.py	Introduces frozenset-based multi-target resolution and adjusts compile logging/target selection flow.
src/apm_cli/core/target_detection.py	Adds CompileTargetType, a no-target-folder reason constant, and updates should_compile_* helpers for frozenset targets.
src/apm_cli/compilation/agents_compiler.py	Allows CompilationConfig.target to be a frozenset and validates allowed families defensively.
tests/unit/compilation/test_compile_target_flag.py	Updates target resolution expectations and adds regression coverage ensuring unrequested families do not compile.
CHANGELOG.md	Adds an Unreleased Fixed entry for #1019.

Copilot's findings

• Files reviewed: 5/5 changed files
• Comments generated: 3

[Copilot]

CompileTargetType = Union[str, frozenset[str]] is too permissive (any string/frozenset member type). This weakens type checking and makes it easier for invalid targets/families to flow into should_compile_* and callers. Consider tightening this to Union[TargetType, frozenset[Literal['agents','claude','gemini']]] (or define a CompileFamily Literal and use TypeAlias) so mypy/pyright can catch mistakes early.

See below for a potential fix:

from typing import List, Literal, Optional, Tuple, TypeAlias, Union

import click

# Valid target values (internal canonical form)
TargetType = Literal["vscode", "claude", "cursor", "opencode", "codex", "gemini", "all", "minimal"]

# Valid compiler families used when a multi-target list is reduced to the
# corresponding compilation outputs.
CompileFamily = Literal["agents", "claude", "gemini"]

# Compile target: either a single TargetType string or a frozenset of compiler
# families ({"agents", "claude", "gemini"}) for multi-target lists.
CompileTargetType: TypeAlias = Union[TargetType, frozenset[CompileFamily]]

[Copilot]

The multi-target progress message is only shown when target is a list, but effective_target can also be a frozenset when the target comes from apm.yml (config list) with no CLI --target. In that case this falls through to get_target_description(effective_target) and logs unknown target. Consider switching the condition to isinstance(effective_target, frozenset) (and deriving a label from config_target) so config-driven multi-targets get the same accurate output summary.

[Copilot]

detect_target() is typed to accept Optional[str] for config_target, but here compile_config_target can be a frozenset (from _resolve_compile_target() when apm.yml specifies a multi-target list and the user also passed a single-string --target). It happens to work today, but it's an implicit type contract violation and makes the control flow harder to reason about. Consider passing config_target=compile_config_target only when it's a string (otherwise None).

[danielmeppiel]

Rebased on main (resolved trivial CHANGELOG.md conflict — kept all four Fixed entries) and addressed the three Copilot reviewer findings on the follow-up commit, @tillig:

1. Type tightening (target_detection.py:34) — Union[str, frozenset[str]] → Union[TargetType, frozenset[CompileFamily]] with a new CompileFamily = Literal["agents", "claude", "gemini"] alias. mypy/pyright can now catch invalid families flowing into should_compile_*.

2. Latent bug fix (cli.py:453) — the isinstance(target, list) branch only fired for CLI multi-target lists. Config-driven multi-target (apm.yml: target: [claude, codex]) fell through to get_target_description(frozenset) and logged Compiling for unknown target - apm.yml target. Switched the gate to isinstance(effective_target, frozenset) so both paths produce the same accurate Compiling for AGENTS.md + CLAUDE.md (...) summary. Added two regression tests in TestMultiTargetLogOutput.

3. Type contract honored (cli.py:425) — detect_target() is typed config_target: Optional[str], but compile_config_target could be a frozenset. Now passes config_target=compile_config_target if isinstance(compile_config_target, str) else None (the frozenset case is already handled by the branch above).

All amended into the existing follow-up commit (force-with-lease used; only your three commits + my one follow-up are on the branch). Tests: 812 passed in 3.55s.

Branch should now be mergeable + green for @apm-bot's next pass.