feat(deps): add 'apm deps why <pkg>' to explain transitive dependencies#1495
Conversation
Implements #1490. Adds a new subcommand under the existing 'apm deps' group that walks the lockfile bottom-up from a target package back to the direct dependencies that pulled it in. Inspired by 'npm why', 'yarn why', and 'cargo tree -i'. Pure walker (src/apm_cli/deps/why_walker.py) is decoupled from the Click command (src/apm_cli/commands/deps/why.py). The walker inverts resolved_by edges, walks parents iteratively, breaks cycles via a per-traversal visited set, and returns frozen WhyEdge / WhyPath / WhyResult dataclasses for deterministic output. Soft dependency on #1488: the walker reads LockedDependency.constraint via getattr; on main today the attribute is absent so edges carry constraint=None and human output collapses to "[declared in apm.yml]". Once #1488 lands, edges automatically gain "[constraint: ^1.2.0, declared in apm.yml]" with zero further code change. A regression test pins the graceful-degrade path. Query supports bare basename (when unique), owner/repo, or full repo URL. Ambiguous basenames list all matches with a hint to use the canonical form. Exit codes: 0 ok, 1 not-found/ambiguous, 2 no-lockfile. --json emits a stable schema for scripting. Tests - 13 unit tests for the walker - 9 unit tests for the CLI command (incl. no-network regression trap asserting subprocess.run and urlopen are never touched) - 4 integration tests against a real on-disk lockfile artifact Verification: ruff check + format clean, pylint R0801 clean, auth-signals lint clean, 242 existing deps-area tests still pass. Closes #1490 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Adds a new apm deps why <pkg> command to explain (offline) why a dependency is present by walking parent links (resolved_by) in apm.lock.yaml, complementing the existing top-down apm deps tree.
Changes:
- Introduces a pure lockfile walker (
deps/why_walker.py) to resolve a package query and compute the parent chain(s). - Adds a Click command (
commands/deps/why.py) with human and JSON output modes plus--globalsupport, and wires it intoapm deps. - Adds unit/integration tests and updates user docs + the apm-usage command reference.
Show a summary per file
| File | Description |
|---|---|
src/apm_cli/deps/why_walker.py |
Implements query resolution and upward parent-chain computation over LockFile. |
src/apm_cli/commands/deps/why.py |
Adds the apm deps why CLI surface, formatting, exit codes, and lockfile loading. |
src/apm_cli/commands/deps/cli.py |
Registers the new why subcommand under the existing deps group. |
src/apm_cli/commands/deps/__init__.py |
Exports the new why command. |
tests/unit/deps/test_why_walker.py |
Unit tests for query resolution and chain computation behavior. |
tests/unit/commands/deps/test_why_command.py |
CLI-level tests for output formats, exit codes, --global, and offline behavior. |
tests/integration/test_deps_why_e2e.py |
End-to-end CLI test using a real on-disk lockfile (no network). |
packages/apm-guide/.apm/skills/apm-usage/commands.md |
Adds the new command to the apm-usage command matrix. |
docs/src/content/docs/consumer/manage-dependencies.md |
Documents apm deps why usage, output shape, and exit codes. |
Copilot's findings
- Files reviewed: 9/10 changed files
- Comments generated: 6
Folds in the high-signal items from the apm-review-panel round 1 on PR #1495: JSON stream discipline (cli-logging-expert REQUIRED + devx-ux-expert + supply-chain-security convergent): - Under --json, set_console_stderr(True) early so all logger output and error JSON envelopes route to stderr, keeping stdout clean for `apm deps why <pkg> --json | jq`. Mirrors `apm pack --json`. - New regression-trap test: `test_why_no_lockfile_json_emits_error_ envelope_on_stderr` asserts stdout is empty and the error payload lands on stderr. Docs (devx-ux-expert + oss-growth-hacker + doc-writer): - docs/src/content/docs/reference/cli/deps.md: add `apm deps why` row in subcommand table and a full section documenting flags, identifier styles, exit codes, and JSON stream discipline. - CHANGELOG.md [Unreleased] / Added: new entry for `apm deps why`. - manage-dependencies.md: add the "Coming from npm/yarn/cargo?" switcher callout, fix sample-output indentation (5->4 spaces), state identifier styles inline (decouples accuracy from the `apm deps info` resolver), cross-link to The lockfile / apm deps tree / apm deps info, document --json stderr routing. - packages/apm-guide/.apm/skills/apm-usage/commands.md: append "analogue of `npm why` / `yarn why`" to the why row. Walker contract (python-architect): - compute_why docstring tightened: states single resolved_by chain per record today, notes the iterative worklist generalises to N paths if #1488's resolver evolves multi-parent fan-in. - WhyResult: NOTE clarifying that `target` is a borrowed reference to the live LockedDependency; do not mutate. - Add a defensive max_paths=256 cap on the worklist (supply-chain-security nit; today unreachable, matters once multi-parent lands). - Rename test_compute_why_diamond_dependency_lists_both_routes -> test_compute_why_diamond_records_single_resolved_by_path so the test name matches the actual assertion (single canonical chain). Lint chain (ruff check/format, pylint R0801, auth-signals) clean. Tests: 27 why-area tests pass (was 26); full deps + integration regression suite at 10057 passed / 223 skipped. Refs: #1490 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
APM Review Panel: ship-after-fold-in
cc @danielmeppiel @sergio-sisternes-epam -- a fresh advisory pass is ready for your review. Three personas (cli-logging-expert, devx-ux-expert, supply-chain-security-expert) converged on the same defect: under Aligned with: ship the simple thing first; lockfile is the source of truth; offline-by-default; cross-tool mental model (npm/yarn/cargo analogue). Growth signal. Panel summary
Top 5 follow-ups
ArchitectureclassDiagram
class LockedDependency {
+str repo_url
+str version
+int depth
+str resolved_by
}
class WhyEdge {
+str parent_key
+str child_key
+str constraint
}
class WhyPath {
+tuple chain
}
class WhyResult {
+LockedDependency target
+bool is_direct
+tuple paths
}
LockedDependency <.. WhyResult : borrowed ref
WhyEdge --o WhyPath : composes
WhyPath --o WhyResult : aggregates
flowchart LR
CLI["apm deps why pkg"] --> Cmd["commands/deps/why.py"]
Cmd -->|load| Lock["LockFile.read()"]
Cmd -->|resolve| Resolve["resolve_package_query()"]
Cmd -->|walk| Walk["compute_why()"]
Walk -->|WhyResult| Render{as_json?}
Render -->|no| Human["_render_human (stdout)"]
Render -->|yes| JSON["_render_json (stdout)"]
Cmd -.->|logs/errors| Stderr["set_console_stderr(True) under --json"]
RecommendationShip. The fold-in commit a39b095 addresses every convergent and recommended finding from round 1; remaining items are documented nits or future-multi-parent defensive work that should land as separate follow-up issues once #1488 evolves the resolver. The feature itself is small (~500 LOC), pure, offline-by-default, and exhaustively tested (27 dedicated tests + 10,030 regression tests green). The cross-tool mental-model framing ( Full per-persona findingsPython architect
CLI logging UX
DevX UX
Supply-chain security
Confirmed: no-network guarantee is solid (regression-trap test asserts OSS growth
Auth -- inactiveNo auth surfaces touched (no token resolution, no Doc writer
Test coverage
All 27 dedicated tests pass (1.05s). Critical surfaces (exit codes, This panel is advisory. It does not block merge. Re-apply the |
# Conflicts: # CHANGELOG.md
|
Follow-ups from the apm-review-panel pass have landed. Summary:
Lint contract: CI: Ready for maintainer review. |
Render-layer fixes: - _render_human: drop self-appended trailing newline; click.echo() already appends one, so the old code emitted a blank trailing line. - _render_human / _render_json: derive 'is_root' / 'is_direct' from edge.parent_key is None, not chain position. The walker can emit truncated chains (missing parent, cycle break, depth cap) whose first edge is NOT a real root; the old position-based logic mislabeled those edges as direct dependencies / 'declared in apm.yml'. Test rename: drop misleading 'multiple_paths' test name -- the assertions and comments document single-parent walk behavior. Docs: clarify in manage-dependencies.md that the lockfile records a single resolved parent per package, so the output is one root-to-target chain (not a fan-out of every theoretical route). Regression-trap tests added (each verified to fail without its fix): - test_why_human_output_has_no_trailing_blank_line - test_why_corrupt_chain_human_output_omits_declared_annotation - test_why_json_corrupt_chain_marks_no_edge_as_direct Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot review addendumAddressed all six Copilot review threads (4 unresolved + 2 outdated) in commit Substance:
Regression-trap tests (each verified via mutation-break gate):
Validation: All six threads resolved. Ready to merge. |
Folds in missing integration-tier coverage for every user-visible promise of 'apm deps why <pkg>' surfaced by the apm-review-panel test-coverage lens. One test per promise, named as the promise sentence, all run end-to-end through CliRunner against a real on-disk apm.lock.yaml fixture. Promises now defended at integration tier: - A: human chain renders root-to-target in order - B: exit 0 on success; exit 1 with clear error when pkg missing - C: --json mode keeps stdout jq-safe; errors go to stderr - D: constraint annotation surfaces when present (post-#1488) and degrades gracefully when absent (parametrized) - E: shared transitive collapses to the documented single chain (matches docs/.../manage-dependencies.md contract) - F: is_direct is derived from edge.parent_key being None, NOT from idx==0 (Copilot regression-trap, lifted to e2e) - G: lockfile resolves at the cwd project root; no upward walk - H: --help advertises the flag names actually wired to click Each test was confirmed to fail under a targeted production mutation and pass once the mutation was reverted (mutation-break gate per test-coverage-expert procedure). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Test-coverage addendum: e2e fold-in for
|
| Promise | Test | Defends |
|---|---|---|
| A | test_promise_a_..._human_chain_from_root_to_target |
human chain renders root-to-target in order |
| B | test_promise_b_..._exit_0_..._nonzero_when_pkg_missing |
exit 0 on success; exit 1 with clear named error when pkg missing |
| C | test_promise_c_..._json_to_stdout_..._errors_to_stderr |
--json keeps stdout jq-safe; error envelope on stderr |
| D | test_promise_d_..._constraint_..._degrades_gracefully (parametrized with_constraint=[True, False]) |
constraint annotation surfaces post-#1488; degrades cleanly today |
| E | test_promise_e_..._single_collapsed_chain |
shared transitive collapses to one chain per docs contract |
| F | test_promise_f_..._reflects_declared_root_not_position |
is_direct derived from edge.parent_key is None, not idx == 0 (Copilot regression-trap lifted to e2e) |
| G | test_promise_g_..._project_root_cwd |
lockfile resolves at cwd; sibling dir exits 2 (no upward walk) |
| H | test_promise_h_..._documents_actual_flags_and_argument |
help text advertises the flag names actually wired to click |
Tests added: 9 (one parametrized -> 10 cases).
File: tests/integration/test_deps_why_promises_e2e.py (no new production code, no new fixtures dir; uses tmp_path + the existing LockFile helpers).
Mutation-break gate: every test was verified to FAIL under a targeted production mutation (exit-code flip, dropped set_console_stderr, removed constraint render, position-based is_direct, renamed flag, etc.) and PASS once reverted.
Lint: ruff check + ruff format --check + pylint R0801 + scripts/lint-auth-signals.sh all clean.
CI: all checks green on commit 39a3392f.
TL;DR
Adds
apm deps why <pkg>-- the bottom-up companion toapm deps tree.When something unexpected lands in
apm_modules/, this command walksthe lockfile back to one or more direct dependencies that pulled it in,
fully offline, and explains the chain. Inspired by
npm why,yarn why, andcargo tree -i. Closes #1490.Note
Soft dependency on #1488. Constraint annotations
(
[constraint: ^1.2.0, declared in apm.yml]) only appear once theconstraintfield lands onLockedDependency. Until then theycollapse to
[declared in apm.yml]. The walker usesgetattrand adedicated regression test pins the graceful-degrade path -- no code
change is needed when #1488 merges.
Problem (WHY)
apm deps treeshows the dependency graph top-down. After runningapm install,apm_modules/may contain transitive packages theconsumer did not declare directly. When something breaks -- an
unexpected MCP server, a skill that surprises the author, a version
that triggers a policy block -- the only way to find out who pulled
it in today is to read
apm.lock.yamlby hand and traceresolved_bychains.
apm deps why <pkg>is the inverted view. It answers "how did thisget here?" without leaving the terminal and without hitting the
network.
Approach (WHAT)
Two-module split mirroring the existing
_build_dep_tree/treepattern incommands/deps/cli.py:src/apm_cli/deps/why_walker.pyLockFile. No Click, no I/O. Invertsresolved_byedges and collects every root-to-target chain.src/apm_cli/commands/deps/why.py--globaluser scope), runs the walker, renders human or JSON output, owns exit codes.The Click command is registered into the existing
depsgroup with oneline in
commands/deps/cli.py-- no restructuring of any existingmodule.
Implementation (HOW)
Walker (
why_walker.py)flowchart LR Q[query string] --> R[resolve_package_query] R -->|exact key| T[target LockedDependency] R -->|owner/repo| T R -->|basename| T R -.->|ambiguous| AE[AmbiguousPackageError] R -.->|none| PE[PackageNotInstalledError] T --> W[compute_why] W -->|direct| Direct[WhyResult is_direct=True] W -->|transitive| Walk[iterative parent walk] Walk --> WR[WhyResult is_direct=False<br/>paths: tuple of WhyPath]Key invariants:
visitedfrozen set; eachrepo_urlappears at most once per chain. Defensive depth cap of
max(64, len(deps)+1).pathsare sorted by their stringifiedchain so JSON snapshots and human output are stable.
_dep_constraintisgetattr(dep, "constraint", None). Soft-couples to feat(deps): resolve semver constraints on git-source dependencies against repo tags #1488.Result types are frozen dataclasses (
WhyEdge,WhyPath,WhyResult)to keep snapshots deterministic and prevent accidental mutation.
Command (
commands/deps/why.py)flowchart TD CLI[apm deps why PKG] --> Scope{--global?} Scope -->|no| Proj[get_apm_dir PROJECT] Scope -->|yes| User[get_apm_dir USER] Proj & User --> LF[load lockfile] LF -->|missing| E2[exit 2 + error] LF -->|ok| RES[resolve_package_query] RES -->|ambiguous| E1A[exit 1 + matches list] RES -->|not found| E1B[exit 1 + hint] RES -->|hit| CW[compute_why] CW --> Render{--json?} Render -->|no| Human[plain ASCII indented tree] Render -->|yes| JSON[stable JSON schema]Rendering choices (per design pack convergence note):
rich.tree. A list of N short chains is not a singlehierarchy;
rich.treewould visually imply one that does not exist.Plain indented text with ASCII
+--markers is more scannable andkeeps snapshot tests deterministic.
[i](matches_rich_info). Constraint and declaration metadata wrapped in[brackets]-- pure ASCII, Windows-cp1252-safe.Wiring
Trade-offs
rich.treegetattrforconstraintrepo_urlLockedDependencyrecords one parent (resolved_by); a true multi-parent diamond would need lockfile-schema changes. Out of scope for #1490.1vs2)1for all errors2) from "wrong package name" (1).Validation evidence
ruff check src/ tests/ruff format --check src/ tests/pylint -e R0801 --min-similarity-lines=10 --fail-on=R0801bash scripts/lint-auth-signals.shsubprocess.runandurlopenmocked, asserted untouched in both human and JSON pathsHow to test
# Run the new tests: uv run --extra dev pytest \ tests/unit/deps/test_why_walker.py \ tests/unit/commands/deps/test_why_command.py \ tests/integration/test_deps_why_e2e.py -v