Pin-aware audit rendering with notes column#78
Merged
Conversation
The ``pinned_version`` field on ``ToolCatalogEntry`` and the ``is_pinned`` / ``get_pinned_version`` / ``should_skip`` methods on ``ToolCatalog`` were never populated — zero ``catalog/*.json`` files set them, so the ``[PINNED]`` marker in the audit renderer could never fire from real data. Pins live per-user in ``~/.config/cli-audit/pins.json`` (managed by the shell helpers under ``scripts/``); the catalog-level mechanism was a dead parallel path. Documentation rewritten to point at the real pin store and the ``scripts/pin_version.sh`` / ``unpin_version.sh`` helpers. Signed-off-by: Sebastian Mendel <github@sebastianmendel.de>
Add ``cli_audit/pins.py`` that reads
``~/.config/cli-audit/pins.json`` — the file the shell helpers
(``scripts/pin_version.sh``, ``unpin_version.sh``,
``reset_pins.sh``) already populate. The Python side now understands:
- flat ``{"tool": "version"}`` pins
- nested ``{"tool": {"cycle": "version"}}`` for multi-version runtimes
- the ``"never"`` sentinel meaning "do not install or update"
- ``tool@cycle`` lookup for names like ``python@3.13``
``audit.py`` (update path) switches from the now-removed catalog
methods to the new API.
Signed-off-by: Sebastian Mendel <github@sebastianmendel.de>
``make audit`` was emitting hardcoded hints like ``Install python 3.13: check your package manager or version manager`` on every not-installed and outdated multi-version row. They added no information — the tool name and state already tell the user what action is possible — so rip them out everywhere: - ``audit.py``: stop generating the canned strings in ``audit_multi_version_tool`` and in the local-only update path; delete the ``CLI_AUDIT_HINTS`` env var and ``SHOW_HINTS`` plumbing. Catalog-provided ``hint`` values are still honored. - ``Makefile.d/user.mk``: drop ``CLI_AUDIT_HINTS=1`` from every audit target (it was overriding the env default anyway). - ``docs/``: remove ``CLI_AUDIT_HINTS`` from CLI / quick / deployment references. Signed-off-by: Sebastian Mendel <github@sebastianmendel.de>
Three rendering gaps surfaced in ``make audit``: 1. Install method (apt / cargo / uv / nvm / …) was collected but never shown. 2. Version pins from ``pins.json`` were invisible. 3. Auto-update flags from ``config.yml`` were invisible. Fixes: - Header grows from 4 to 5 columns, adding ``notes`` = ``method · auto``. Smoke test updated to match (it was asserting 6 columns against a 4-column reality — silent pass via ``|| true``). - PIN marker renders next to the version it constrains (installed column), e.g. ``3.12.7 [PIN:3.12.7]`` or ``[PIN:never]`` on not-installed rows. - Status icon computation now respects pins: a tool pinned to X with installed == X is ``✅`` regardless of upstream latest; installed != X is ``⚠️ CONFLICT`` (pin being violated); ``PIN:never`` + not installed is ``✅`` (intent honored). Previously ``✅`` could render on rows whose installed version diverged from the pin. - Summary tallies use the same pin-aware status so "Readiness: N outdated, N missing" matches the visible icons. - ``status_icon`` consults ``status`` before the empty-installed short-circuit so ``PIN:never`` rows don't show red-X for absence that is the intended state. Signed-off-by: Sebastian Mendel <github@sebastianmendel.de>
Propose treating each discovered installation of a tool as individually addressable. Current asymmetry: detection finds every binary on ``PATH``, ``choose_highest()`` discards everything but one before snapshot write, and only ``reconcile.py`` re-detects the rest — purely to delete them. ADR-009 spells out the data model (``Installation`` dataclass, stable ``tool#method`` keys), snapshot schema bump, per-install pins (``"ripgrep#apt": "14.0.0"``), per-install upgrade, the narrowed role for ``reconcile``, open questions, and a phased rollout. Status: Proposed — companion to the rendering work landed in the earlier commits of this branch; implementation is follow-up. Signed-off-by: Sebastian Mendel <github@sebastianmendel.de>
Contributor
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #78 +/- ##
==========================================
+ Coverage 66.78% 69.10% +2.32%
==========================================
Files 21 22 +1
Lines 3230 3301 +71
==========================================
+ Hits 2157 2281 +124
+ Misses 1073 1020 -53
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors the version pinning system by moving pin configurations from the tool catalog to a user-local JSON file, supported by a new cli_audit.pins module. It simplifies the audit output by removing redundant remediation hints and updating the table rendering to include a notes column for installation methods and auto-update status. Additionally, a new Architecture Decision Record (ADR-009) is introduced to propose support for multiple tool installations. Feedback identifies the need for better error reporting during pin file parsing, the removal of an unused parameter in the status logic, and a fix for a conflict detection bug that fails when ANSI color codes are present.
Self-review of the PR surfaced three things worth addressing before
a reviewer sees them:
- ``_apply_pin_to_status`` lived in ``render.py`` but it's pin logic,
not display logic. Moved into ``pins.py`` as public
``apply_pin_to_status`` (no ``latest`` arg — it was unused) and
exported from the package API.
- Inline ``from cli_audit.pins import …`` statements inside the
``cmd_update`` loop and ``_render_tool_row`` hot path promoted to
module-level imports.
- ``tests/test_pins.py``: 30 tests covering ``load_pins``,
``lookup_pin``, ``is_pinned``/``is_never``, ``should_skip``, and the
state matrix for ``apply_pin_to_status``. One test caught a real
ambiguity — looking up ``tool@cycle`` against a flat ``{tool:
"x.y"}`` pin returned the flat value; shape-mismatched lookups now
return empty instead, and the docstring says so.
Signed-off-by: Sebastian Mendel <github@sebastianmendel.de>
There was a problem hiding this comment.
Pull request overview
This PR updates the audit output to be pin-aware and more informative by rendering user pins from ~/.config/cli-audit/pins.json, adding a notes column (install method + auto-update flag), and removing the previously hardcoded “hints” plumbing.
Changes:
- Add a Python pins reader (
cli_audit/pins.py) and rewire pin usage away from the catalog. - Update audit rendering to 5 columns with pin-aware status icons and summary tallies.
- Remove the
CLI_AUDIT_HINTSfeature and update docs/smoke test accordingly.
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| scripts/test_smoke.sh | Updates the smoke test to assert the new 5-column header and notes column. |
| docs/adr/ADR-009-first-class-multi-install.md | Adds an ADR proposing first-class multi-install support (follow-up work). |
| docs/QUICK_REFERENCE.md | Removes references to the deleted CLI_AUDIT_HINTS env var. |
| docs/DEPLOYMENT.md | Removes references to the deleted CLI_AUDIT_HINTS env var. |
| docs/CLI_REFERENCE.md | Removes CLI_AUDIT_HINTS from the env var reference table. |
| docs/CATALOG_GUIDE.md | Updates pinning docs to point to pins.json and shell helpers (needs a small rendering-location fix). |
| docs/API_REFERENCE.md | Removes pinned_version from the catalog entry API docs. |
| cli_audit/render.py | Adds notes column, pin suffix rendering, and pin-aware status/summary logic. |
| cli_audit/pins.py | New module for reading pins from the user-local pins file. |
| cli_audit/catalog.py | Removes unused catalog pin fields/methods (pinned_version, is_pinned, should_skip, etc.). |
| cli_audit/init.py | Exposes pins helpers from the package top-level API. |
| audit.py | Removes hints env var usage, wires rendering to new signature, and uses pins for update progress markers. |
| Makefile.d/user.mk | Removes CLI_AUDIT_HINTS from targets (needs smart_column alignment + help-text tweak). |
Comments suppressed due to low confidence (1)
cli_audit/render.py:282
installed_display.startswith("CONFLICT:")will never match when colors are enabled, becauseinstalled_displayhas ANSI color prefixes aftercolorize(). Strip theCONFLICT:prefix from the rawinstalledstring before colorizing (or strip ANSI escapes before checking) so conflict messages render cleanly in the default colored output.
parts = [f"{total} tools", f"{outdated} outdated", f"{missing} missing"]
if conflicts > 0:
parts.append(f"{conflicts} conflicts")
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Three issues raised on the initial review: 1. ``pins.py`` silently swallowed ``json.JSONDecodeError`` and top-level non-dict payloads. A malformed ``pins.json`` would render as if no pins existed with zero diagnostic feedback. Now each failure mode logs a ``WARNING`` naming the file and the failure reason. 2. ``render.py`` had an unused ``latest`` parameter on the now-relocated ``apply_pin_to_status``. Dropped (was already gone in ``pins.apply_pin_to_status``; this just removes the stale reviewer hit on the earlier function-local copy). 3. ``render.py``: the ``CONFLICT:`` sentinel strip ran against the already-colorized ``installed_display`` (``\\033[33mCONFLICT:…``), so the ``startswith`` check never matched when color was enabled — i.e. always under ``make audit``. Strip the sentinel from the raw ``installed`` string before colorization. Tests: - ``tests/test_pins.py`` gains warning-log assertions for malformed JSON and non-dict payloads. - ``tests/test_render.py`` added, including a regression test for the CONFLICT-prefix / ANSI interaction. 585 tests pass; new files lint clean. Signed-off-by: Sebastian Mendel <github@sebastianmendel.de>
Copilot's review raised three additional issues: 1. ``Makefile.d/user.mk``: ``smart_column.py --right 3,5`` was right-aligning the new ``notes`` text column and leaving the ``latest_upstream`` version column left-aligned. Swap to ``3,4`` so both version columns (installed, latest) right-align and the notes column stays left-aligned. Applied to every audit target. 2. ``Makefile.d/user.mk``: the ``audit-offline`` help string still said "with hints" after canned hints were removed. Updated to "fast local scan, no network". 3. ``docs/CATALOG_GUIDE.md``: the "Pinning Versions" section said pins render in the ``notes`` column, but the final layout puts ``[PIN:…]`` in the ``installed`` column (next to the version the pin constrains). Docs corrected to match. Signed-off-by: Sebastian Mendel <github@sebastianmendel.de>
5 tasks
CybotTM
added a commit
that referenced
this pull request
Apr 21, 2026
## Summary Follow-up to #78 (merged). Three issues surfaced once users saw real pin data in the audit table: 1. **Cycle vs patch pins were indistinguishable.** `~/.config/cli-audit/pins.json` stores per-cycle pins as `{cycle: value}` but `value` can mean either "hold this cycle, any patch" (`"3.12"`) or "hold this exact patch" (`"3.12.7"`). The previous code treated every non-`never` pin as exact-version, flagging valid cycle-holds as conflicts. 2. **Stale skip-markers shouted as conflicts.** `guide.sh` pins to `$installed` or `$latest` from several code paths (pin-on-failure prompt, `s` skip). When the world moved past those pins, my rendering escalated them to `⚠️ ` even though the pin no longer applies meaningfully. 3. **Auto-update state was invisible.** User-configured `auto_update: true` existed in `config.yml` but had no marker on rows. ## Changes Atomic commits: 1. `feat(pins)` — new `classify_pin(pin, cycle)` returns `none|never|cycle|version`. `apply_pin_to_status` grows a `cycle` parameter: cycle-holds are honored when installed is within-cycle; multi-version rows with stale patch-pins pass their upstream status through (no CONFLICT escalation); single-version tools still conflict on mismatch. New `pin_label()` produces `PIN:never` / `CYCLE:3.12` / `PIN:x` / `PIN:x stale` — one source of truth for the rendered suffix. 2. `feat(render)` — cycle threaded into status computation; pin + AUTO combined in a single bracketed suffix on the installed column (`3.14.4 [AUTO]`, `3.12.7 [CYCLE:3.12 AUTO]`, `8.5.5 [PIN:8.5.3 stale]`); AUTO suppressed when pin is `never` (contradictory state). Notes column slims to just the install method. 3. `feat(scripts)` — `scripts/reset_pins.sh --stale [--dry-run]` removes only stale patch-level pins (installed present, pin != installed, pin != cycle, pin != `never`). Cycle-holds, `never` pins, and honored patch-holds are preserved. ## Before / after on this repo ``` before: after: ⚠ php@8.5 8.5.5 [PIN:8.5.3] apt ✓ php@8.5 8.5.5 apt ⚠ php@8.4 8.4.20 [PIN:8.4.18] apt ✓ php@8.4 8.4.20 apt ✓ python@3.12 3.12 [PIN:3.12] apt ✓ python@3.12 3.12 [CYCLE:3.12 AUTO] apt ✓ python@3.14 3.14.4 manual · auto ✓ python@3.14 3.14.4 [AUTO] manual ⚠ ruby@3.3 3.3.6 [PIN:never AUTO] manual · auto ⚠ ruby@3.3 3.3.6 [PIN:never] manual ``` Stale pins in the user's `pins.json` (`php@8.5: 8.5.3`, `php@8.4: 8.4.18`) cleaned with the new `--stale` flag; config is now internally consistent. ## Test plan - [x] `uv run pytest` — 608 passing (up from 585; 23 new tests: 17 in `test_pins.py` covering `classify_pin` / cycle-aware `apply_pin_to_status` / `pin_label`, 6 in `test_render.py` covering cycle-hold / stale / single-version conflict / AUTO visibility / base-tool inheritance). - [x] `./scripts/test_smoke.sh` passes. - [x] `flake8` clean on changed files. - [x] `scripts/reset_pins.sh --stale --dry-run` dry-runs correctly against the real snapshot. - [ ] Reviewer: verify your `pins.json` classifies as expected — run `bash scripts/reset_pins.sh --stale --dry-run`.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Three gaps surfaced in
make audit:installed_method(apt/cargo/uv/pipx/nvm/brew/manual) was captured per-tool but never rendered.catalog.is_pinned()read apinned_versioncatalog field that zero JSON files set, while the real pin store at~/.config/cli-audit/pins.jsonwas never read by the Python side.Install python 3.13: check your package manager or version manager— hardcoded, tautological, adding no information over tool name + state.Changes
Atomic commits, each independently revertable:
refactor(catalog)— remove unusedpinned_versionfield +is_pinned/get_pinned_version/should_skipmethods. Documentation rewritten to point at the real pin store and shell helpers.feat(pins)— newcli_audit/pins.pythat reads~/.config/cli-audit/pins.json(flat and nested schemas,tool@cycle,\"never\"sentinel). Rewire the one non-render caller inaudit.py.refactor(audit)— delete the hint plumbing end-to-end: generators inaudit.py,CLI_AUDIT_HINTSenv var,Makefile.d/user.mkoverrides, three doc files.feat(audit)— pin-aware rendering:state|tool|installed|latest_upstream|notes(5 columns, up from 4; smoke test's 6-column assertion was silently passing against a 4-column reality).method · auto.8.5.3 [PIN:8.5.3],[PIN:never]on absent rows.✅only when installed matches pin (regardless of upstream latest);⚠️ CONFLICTwhen pin is violated;PIN:never+absent is✅(intent honored) instead of❌.print_summaryuses the same pin-aware status so the readiness tallies match the icons.docs(adr)— ADR-009 proposes first-class multi-install support. Implementation is follow-up.Before / after (rows from this repo)
Test plan
./scripts/test_smoke.shpasses against the new 5-column header.make audit— spot-checked regular tools (ripgrep,jq,cargotools), multi-version runtimes (python@,php@,node@,ruby@,go@), and the readiness summary tallies.flake8clean on changed files.pins.jsonrenders as expected —[PIN:…]next to installed,PIN:neverhonored on absent rows.