Skip to content

fix: move pinning hint to diagnostics section#347

Merged
danielmeppiel merged 3 commits intomainfrom
fix/pinning-hint-diagnostics
Mar 17, 2026
Merged

fix: move pinning hint to diagnostics section#347
danielmeppiel merged 3 commits intomainfrom
fix/pinning-hint-diagnostics

Conversation

@danielmeppiel
Copy link
Collaborator

Summary

Moves the inline pinning tip from cluttering the package install tree into the ── Diagnostics ── section at the bottom, following the established CLI UX pattern.

Before

[checkmark] owner/repo
  └─ 2 prompts integrated
Installed 1 APM dependencies
Tip: Pin versions with #tag or #sha for reproducible installs (e.g. owner/repo#v1.0.0)

After

[checkmark] owner/repo
  └─ 2 prompts integrated
Installed 1 APM dependencies

── Diagnostics ──
  [i] 1 dependency has no pinned version -- pin with #tag or #sha to prevent drift

Changes

  • diagnostics.py: Add CATEGORY_INFO with info() method and _render_info_group() renderer (blue [i] prefix, rendered last after all warnings/errors)
  • install.py: Convert has_unpinned_deps bool → unpinned_count int; emit single aggregated diagnostics.info() with count and drift-focused message
  • test_diagnostics.py: Add TestInfoCategory (3 tests for info collection, rendering, ordering)
  • test_install_output.py: Remove TestUnpinnedDepsDetection (logic moved to diagnostics layer)

Testing

2451 tests pass. 2 pre-existing failures on main confirmed (run on clean main without changes):

  • test_install_local_package_relative_path (integration)
  • test_notion_npm_server_config_generation (codex docker args)

Replace inline _rich_info() tip with aggregated DiagnosticCollector
info() emission. Unpinned deps are now counted (not just flagged) and
rendered in the Diagnostics section at the bottom of install output,
keeping the package tree clean.

Changes:
- Add CATEGORY_INFO to DiagnosticCollector with info() method and
  _render_info_group() renderer (blue [i] prefix)
- Convert has_unpinned_deps bool to unpinned_count int in install.py
- Emit single aggregated diagnostic emphasizing drift prevention
- Remove TestUnpinnedDepsDetection (logic moved to diagnostics)
- Add TestInfoCategory tests for the new info category

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings March 17, 2026 15:56
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR adjusts APM’s CLI UX by moving the version-pinning guidance from the inline install output into the structured ── Diagnostics ── summary, aligning it with the existing “collect then render” diagnostics pattern.

Changes:

  • Added a new diagnostics category info with collection + rendering support, rendered last in the summary.
  • Switched install-time unpinned tracking from a boolean to a counter and emits a single aggregated info diagnostic.
  • Updated unit tests by adding coverage for the new info diagnostics behavior and removing the old unpinned detection unit test.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.

File Description
src/apm_cli/utils/diagnostics.py Introduces CATEGORY_INFO, info() recording, and info rendering in render_summary() (last).
src/apm_cli/commands/install.py Counts unpinned dependencies and emits a single aggregated diagnostics info message instead of an inline tip.
tests/unit/test_diagnostics.py Adds tests for collecting/rendering info diagnostics (but ordering test is currently incomplete).
tests/unit/test_install_output.py Removes the unit test class that previously validated “unpinned” detection via DependencyReference.reference.

danielmeppiel and others added 2 commits March 17, 2026 17:17
- Rename has_unpinned_deps → unpinned_count for clarity
- Fix double blank line before diagnostics header
- Align info detail indentation (6→4 spaces) with warnings
- Make test_info_appears_after_other_categories actually verify render order
- Add tests for unpinned deps message formatting (singular/plural)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants