docs(compile): clarify target routing docstring

[WilliamK112]

This follow-up docs fix clarifies _resolve_compile_target() to match the current target-routing behavior.

What changed

• removed the outdated statement that the compiler only understands "vscode", "claude", and "all"
• documented how single targets pass through unchanged
• documented how multi-target lists collapse to "vscode", "claude", or "all"

Validation

• PYTHONPATH=src uv run pytest tests/unit/compilation/test_compile_target_flag.py -x

[Copilot]

Pull request overview

Clarifies the _resolve_compile_target() docstring in the compile CLI to match current target-routing behavior, so developers understand how single vs multi-target inputs are interpreted before detect_target() runs.

Changes:

• Updates _resolve_compile_target() docstring to remove outdated “compiler only understands vscode/claude/all” wording.
• Documents pass-through behavior for single targets and routing collapse behavior for multi-target lists.

[Copilot]

The term "AGENTS-family target" is introduced here but not defined anywhere else in this module, which makes the mapping rules ambiguous (especially since the list case treats cursor/opencode/codex as part of that family). Consider explicitly listing the values included in this family (e.g., copilot/vscode/agents/cursor/opencode/codex) or rephrasing the bullets in terms of "any non-claude targets" so readers can predict how a given multi-target list will be routed.

[github-actions]

APM Review Panel Verdict: REJECT

All three technical specialists agreed on the same root cause -- the PR was authored against a stale fork and the proposed docstring silently drops gemini routing that already exists in both the implementation and the current main docstring.

Required before merge (3 items)

• [python-architect] Docstring omits gemini family and would regress main's already-accurate docstring at src/apm_cli/commands/compile/cli.py:167

  • Why: The implementation at lines 188-195 treats gemini as a distinct routing family: has_gemini = 'gemini' in target_set feeds into distinct = sum([has_agents_family, has_claude, has_gemini]), and elif has_gemini: return 'gemini' is an explicit branch. The main branch docstring already documents this correctly. The PR replaces that with a bullet list covering only agents-family, claude, and agents+claude->all, silently dropping the gemini->gemini case and the agents+gemini->all / claude+gemini->all / all-three->all collapse paths. Merging this PR would make the docstring factually wrong relative to the implementation.
  • Suggested fix: Rebase onto current main. The existing docstring on main is already accurate. If the bullet-list format is preferred, add the missing cases: - gemini only -> "gemini" and generalize the all rule to - any two or more distinct families -> "all".

• [cli-logging-expert] Proposed docstring omits gemini target -- regression vs current main at src/apm_cli/commands/compile/cli.py:170

  • Why: The function body handles has_gemini (line 188) and returns "gemini" (lines 194-195) as a distinct compiler family. The current main docstring already documents gemini. A PR titled "clarify" that makes the docstring less accurate than what it replaces is a regression, not a fix. Any developer relying on this docstring to understand compile-target routing would miss an entire target family.
  • Suggested fix: Add a bullet - "gemini" only -> "gemini" and update the all bullet to - two or more distinct families (agents, claude, gemini) -> "all".

• [devx-ux-expert] Proposed docstring omits gemini routing that exists in the implementation at src/apm_cli/commands/compile/cli.py:167

  • Why: The function body (lines 188-195) treats gemini as a distinct third compiler family: has_gemini is computed, it participates in the distinct-families count, and has its own elif branch returning 'gemini'. The PR's replacement bullet list describes only two families (AGENTS-family and claude), leading any reader to conclude gemini is not handled -- which is wrong. Docstrings that contradict the code they document erode trust in every other docstring in the CLI. This is a factual regression.
  • Suggested fix: Add - "gemini" only -> "gemini" and update the mixed-target bullet to - two or more distinct families (AGENTS-family, claude, gemini) -> "all".

Nits (4 items, skip if you want)

• [python-architect] Bullet-list format from this PR is clearer than main's prose -- consider adopting it (with all families) in a follow-up
• [cli-logging-expert] "AGENTS-family" is undefined jargon; a parenthetical listing members (copilot, vscode, agents, cursor, opencode, codex) would save future readers a round-trip to the code at src/apm_cli/commands/compile/cli.py:170
• [devx-ux-expert] "AGENTS-family" is undefined jargon in the docstring with no member list; inline the member list so the docstring is self-contained
• [oss-growth-hacker] "AGENTS-family" is undefined jargon in the docstring; a parenthetical listing key members would cut contributor ramp-up time

CEO arbitration

All three active technical panelists -- python-architect, cli-logging-expert, and devx-ux-expert -- converge on the same root cause with zero dissent: the PR was authored against a stale fork that predates the gemini routing family, and the proposed bullet-list docstring silently drops gemini handling that already exists in both the implementation (lines 188-195) and the current main docstring. A docstring titled "clarify" that makes the documentation less accurate than what it replaces is a factual regression, not a fix. The rejection is unambiguous on the merits.

Strategically, this should be a rebase-and-revise, not a close. The contributor's structural instinct is correct: the bullet-list format proposed in the PR is genuinely easier to scan than main's current prose paragraph, and all three technical panelists acknowledged this in their nits. The right outcome is to guide the contributor to rebase onto current main, add the missing gemini bullet, generalize the mixed-target rule to "any two or more distinct families -> all", and inline the AGENTS-family member list so the docstring is self-contained. That produces a strictly better docstring than what main has today.

No security, auth, or supply-chain concerns apply -- this is a docstring-only change with no runtime effect. The blocking issue is purely factual completeness.

Growth/positioning note: This is an external contributor (WilliamK112) voluntarily improving internal documentation -- exactly the on-ramp behavior APM needs to reward. The rejection should lead with thanks, explicitly acknowledge that the bullet-list format is an improvement over the current prose, and provide concrete rebase guidance (rebase on main, add the gemini bullet, inline the AGENTS-family members). A warm, actionable rejection that models a low-friction contribution loop is more valuable to adoption velocity than any single docstring fix.

---

Per-persona findings (full)

Python Architect

classDiagram
    direction LR
    class compile_cli {
        <<Module>>
        +compile(ctx, output, target) void
        +_resolve_compile_target(target) str
        +_display_single_file_summary(...) void
        +_display_next_steps(output) void
        +_display_validation_errors(errors) void
        +_get_validation_suggestion(error_msg) str
    }
    class target_detection {
        <<Module>>
        +detect_target(project_root, explicit_target, config_target) tuple
        +get_target_description(target) str
    }
    class TargetParamType {
        <<click.ParamType>>
        +convert(value, param, ctx) str
    }
    class AgentsCompiler {
        <<Compiler>>
        +compile(config) CompilationResult
    }
    class CompilationConfig {
        <<Dataclass>>
        +target str
        +output_path Path
    }
    compile_cli ..> TargetParamType : click option type
    compile_cli ..> target_detection : detect_target() call
    compile_cli ..> AgentsCompiler : instantiates
    compile_cli ..> CompilationConfig : builds
    class compile_cli:::touched
    classDef touched fill:#fff3b0,stroke:#d47600

Loading

flowchart TD
    A["compile() CLI entry"] --> B["_resolve_compile_target(target)"]
    B --> C{"target is None?"}
    C -- yes --> D["return None -- triggers auto-detection"]
    C -- no --> E{"isinstance(target, list)?"}
    E -- no --> F["return target -- single string pass-through"]
    E -- yes --> G["build target_set, check families"]
    G --> H{"has_agents_family?"}
    G --> I{"has_claude?"}
    G --> J{"has_gemini?"}
    H --> K["distinct = sum of booleans"]
    I --> K
    J --> K
    K --> L{"distinct >= 2?"}
    L -- yes --> M["return 'all'"]
    L -- no --> N{"has_claude?"}
    N -- yes --> O["return 'claude'"]
    N -- no --> P{"has_gemini?"}
    P -- yes --> Q["return 'gemini'"]
    P -- no --> R["return 'vscode' -- agents-family only"]
    D --> S["detect_target() -- target_detection.py"]
    F --> S
    M --> S
    O --> S
    Q --> S
    R --> S

Loading

Required:

• Docstring omits gemini family and would regress main's already-accurate docstring at src/apm_cli/commands/compile/cli.py:167
  • Why: The implementation at lines 188-195 treats gemini as a distinct routing family with its own elif has_gemini: return 'gemini' branch. The main branch docstring already documents this. The PR's bullet list drops gemini->gemini, agents+gemini->all, claude+gemini->all, and all-three->all paths.
  • Suggested fix: Rebase onto main; if bullet-list format is preferred, add - gemini only -> "gemini" and generalize the all rule to - any two or more distinct families -> "all".

Nits:

• Bullet-list format from this PR is clearer than main's prose -- consider adopting it (with all families) in a follow-up

CLI Logging Expert

Required:

• Proposed docstring omits gemini target -- regression vs current main at src/apm_cli/commands/compile/cli.py:170
  • Why: The function body handles has_gemini (line 188) and returns "gemini" (lines 194-195) as a distinct compiler family. The current main docstring already documents gemini. Dropping it is a regression.
  • Suggested fix: Add - "gemini" only -> "gemini" bullet; update all bullet to - two or more distinct families (agents, claude, gemini) -> "all".

Nits:

• "AGENTS-family" is undefined jargon; parenthetical with members (copilot, vscode, agents, cursor, opencode, codex) would help at src/apm_cli/commands/compile/cli.py:170

DevX UX Expert

Required:

• Proposed docstring omits gemini routing that exists in the implementation at src/apm_cli/commands/compile/cli.py:167
  • Why: has_gemini is computed, participates in the distinct-families count, and has its own elif return branch. The PR's bullet list maps everything to vscode/claude/all, leading any reader to conclude gemini is not handled. Docstrings that contradict the code erode trust in every other docstring in the CLI.
  • Suggested fix: Add - "gemini" only -> "gemini"; update the mixed-target bullet to - two or more distinct families (AGENTS-family, claude, gemini) -> "all".

Nits:

• "AGENTS-family" is undefined jargon with no member list; inline the members (copilot, vscode, agents, cursor, opencode, codex) so the docstring is self-contained

Supply Chain Security Expert

No findings.

Auth Expert

Inactive -- PR #931 modifies only a docstring in src/apm_cli/commands/compile/cli.py; no auth, token, credential, host-classification, or AuthResolver-related files are touched.

OSS Growth Hacker

Nits:

• "AGENTS-family" is undefined jargon in the docstring; a parenthetical listing key members would cut contributor ramp-up time for newcomers scanning the codebase

_{Verdict computed deterministically: 3 required findings across 5 active panelists. APPROVE iff N == 0. Push a new commit to clear this verdict label automatically.}

Generated by PR Review Panel · ● 5.9M · ◷

[danielmeppiel]

Note from APM maintainer: this panel verdict was produced by an end-to-end test of a refactored panel skill (PR #1022) and was dispatched manually against your PR for validation purposes. The technical findings are real and worth addressing (the gemini routing case is genuinely missing from the proposed docstring), but please do not feel pressured by the panel-rejected label -- I will remove it momentarily. Thanks for the docs contribution!