Skip to content

fix(routing): mark a matrix that shadows a same-named matrix in routing list/show - #293

Merged
Brian Krabach (bkrabach) merged 2 commits into
mainfrom
lane/adq-routing-list-shadowing
Sep 3, 2026
Merged

fix(routing): mark a matrix that shadows a same-named matrix in routing list/show#293
Brian Krabach (bkrabach) merged 2 commits into
mainfrom
lane/adq-routing-list-shadowing

Conversation

@bkrabach

Copy link
Copy Markdown
Collaborator

DRAFT. Research-lane deliverable for work item model_performance-adq
(the user-facing half of the routing-matrix shadowing arc). Not for merge
without a maintainer read on the seam described under "Reachability".

The defect

amplifier routing list — the command a human runs to ask "what matrices do
I have?"
— shows a user matrix and a bundle matrix of the same name as
peers
, with no indication that one is suppressing the other.

Only one of them is ever loaded. hooks-routing's mount() searches
[*custom_routing_dirs, bundle routing/] and takes the first hit, so a
same-named file in ~/.amplifier/routing/ silently makes the shipped bundle
matrix dead. This is why a matrix change shipped in the bundle can be
completely inert on a host (proven on n=99 wire captures by
model_performance-8vq; it is what closed lane df1's PR).

model_performance-ell (routing-matrix #52, merged d17d03c) made this
observable at load time — an INFO/WARNING naming the winner and every
suppressed file, plus matrix_path / matrix_source / shadowed_paths on
the model_role_resolver capability. This PR is the missing half: making it
observable in the command a human actually runs.

What the CLI shows today (file:line)

what where
the list command amplifier_app_cli/commands/routing.py:297-354
where the files come from _discover_matrix_files() — same file, :98-141
where they become rows _load_all_matrices() — same file, :153-160

routing list renders one row per matrix — name, active arrow, description,
covered/total roles compatibility, updated — from a name -> data dict
built by _load_all_matrices(). Nothing in that pipeline carries the file a
row came from
, so nothing can say that two rows collapsed into one, or which
file won.

Reachability: the fields ell publishes are not importable here

Stated precisely, because it drove the design:

  1. resolve_matrix_source cannot be imported. hooks-routing is a
    bundle module. It is not a distribution amplifier-app-cli depends on
    and it is not on sys.path.
  2. The capability fields cannot be read either. matrix_path /
    matrix_source / shadowed_paths are published on the
    model_role_resolver capability at session start. amplifier routing list never mounts a bundle, so no coordinator and no capability exist in
    that process.

The smallest seam that still avoids re-implementing precedence: the
routing-matrix bundle is on disk, in the very cache directory
_discover_matrix_files() already globs. So the CLI loads
modules/hooks-routing/amplifier_module_hooks_routing/matrix_loader.py by
file path
and calls the real resolve_matrix_source. One rule, one home —
this PR adds no precedence logic of its own.

Degradation is deliberate: a cached bundle older than #52 has no
resolve_matrix_source, and in that case no marker is drawn at all. A
wrong shadowing claim is worse than none.

(A smaller long-term seam exists and is worth a maintainer's opinion:
hooks-routing shipping resolve_matrix_source somewhere app-cli can import
outright. That is a cross-repo packaging decision, not this lane's call.)

Before / after

Real run against a fixture home carrying the actual upstream
matrix_loader.py at d17d03c (not a stub), with ~/.amplifier/routing/openai.yaml
shadowing the bundle's openai.yaml.

Before

$ amplifier routing list
── routing matrices (1 active, 1 disabled) ──
  [off]    balanced  (available)  ← disabled
  [on]  → openai  (active)

$ amplifier routing show openai --compact
           Routing: openai
┏━━━━━━━━━┳━━━━━━━━━━━━━━━┳━━━━━━━━━━┓
┃ Role    ┃ Model         ┃ Provider ┃
┡━━━━━━━━━╇━━━━━━━━━━━━━━━╇━━━━━━━━━━┩
│ general │ gpt-5.6-terra │ openai   │
│ fast    │ gpt-5.6-luna  │ openai   │
└─────────┴───────────────┴──────────┘

Two openai.yaml files exist. One is dead. Nothing says so.

After

$ amplifier routing list
── routing matrices (1 active, 1 disabled) ──
  [off]    balanced  (available)  ← disabled
  [on]  → openai  ⚠ shadows bundle  (active)

⚠ 1 matrix is shadowed — only the 'in use' file is loaded:
  openai
    in use      ~/.amplifier/routing/openai.yaml
    suppressed  ~/.amplifier/cache/amplifier-bundle-routing-matrix-demo/routing/openai.yaml  (bundle)

$ amplifier routing show openai --compact

⚠ 'openai' shadows a bundle matrix of the same name — only the 'in use' file is loaded:
    in use      ~/.amplifier/routing/openai.yaml
    suppressed  ~/.amplifier/cache/amplifier-bundle-routing-matrix-demo/routing/openai.yaml  (bundle)

           Routing: openai
┏━━━━━━━━━┳━━━━━━━━━━━━━━━┳━━━━━━━━━━┓
┃ Role    ┃ Model         ┃ Provider ┃
┡━━━━━━━━━╇━━━━━━━━━━━━━━━╇━━━━━━━━━━┩
│ general │ gpt-5.6-terra │ openai   │
│ fast    │ gpt-5.6-luna  │ openai   │
└─────────┴───────────────┴──────────┘

A marker on the row (visible in every view, including --compact), then the
relationship itself: the winning path, and each path it suppresses, one per
line. Two extra lines per collision — not a wall of text.

--format json additionally carries MatrixSource.to_dict() verbatim
(matrix_name / matrix_path / matrix_source / matrix_shadowed /
shadowed_paths) on shadowed entries — the same field names ell publishes on
the capability, so a script reads one vocabulary either way.

Unshadowed output is unchanged

Not "looks the same" — asserted byte-identical. Two tests render the same
unshadowed tree twice, once through the new path and once with provenance
forced unavailable (the pre-change path), and compare the strings exactly.
Text and JSON both. Nothing at all is printed when nothing is shadowed.

Tests

tests/test_routing_shadowing.py, 15 tests:

  • shadowed setup marks the row and names the winner + suppressed path
    — including a non-vacuity assertion: the same tree through the
    pre-change path shows no marker;
  • unshadowed output byte-identical, text and JSON;
  • routing list works with no user routing dir at all;
  • no marker when the cached bundle predates #52 (missing
    matrix_loader.py, or present without the function);
  • the loaded function really comes from the bundle file on disk
    (fn.__code__.co_filename), not from this repo;
  • unit coverage of dir classification and MatrixSource resolution.

Full suite: 1588 passed, 1 skipped, 1 xfailed. ruff check clean on all
touched files (8 pre-existing session.py findings unchanged).

Known follow-up, not fixed here

_load_all_matrices() (routing.py:153-160) keys rows by the name: field
inside the YAML with last-write-wins over sorted(files). That is not
the loader's rule — it agrees with it today only by alphabetical accident
(cache < routing) and can diverge. This PR reads provenance by the winning
row's file stem, which is the loader's actual key, so the marker is
correct either way; but the row-selection defect itself is untouched and
deserves its own item.

Amplifier Lane adq added 2 commits September 2, 2026 17:46
…ing list`/`show`

`amplifier routing list` showed a user matrix and a bundle matrix of the
same name as peers. Only one is ever loaded: hooks-routing's mount()
searches `[*custom_routing_dirs, bundle routing/]` and takes the first
hit, so a file in ~/.amplifier/routing/ silently makes the shipped bundle
matrix dead -- and nothing in the CLI said so. That is why a matrix change
shipped in the bundle can be completely inert on a host.

The precedence rule is NOT re-derived here. It is consumed from
hooks-routing's own `resolve_matrix_source()` (routing-matrix PR #52),
loaded by file path out of the same cached bundle directory the CLI
already globs -- app-cli does not depend on hooks-routing as a
distribution, and `routing list` never mounts a bundle, so neither an
import nor the session-time `model_role_resolver` capability is reachable
from this process.

When the cached bundle predates PR #52 (no `resolve_matrix_source`), the
CLI draws no marker at all rather than guessing the search order: a wrong
shadowing claim is worse than none. Unshadowed output is byte-identical
to before, in both text and JSON.

- amplifier_app_cli/lib/routing_provenance.py: locate + load the bundle's
  matrix_loader, classify custom vs bundle routing dirs, resolve one
  MatrixSource per matrix name.
- commands/routing.py: row marker (`⚠ shadows bundle`), a footer naming
  the file in use and each file it suppresses, the same note on
  `routing show`, and MatrixSource.to_dict() in `--format json`.
- tests/test_routing_shadowing.py: 15 tests -- shadowed marks the winner,
  unshadowed output byte-identical, no user routing dir, and the
  old-bundle degradation path.
@bkrabach
Brian Krabach (bkrabach) force-pushed the lane/adq-routing-list-shadowing branch from 7aa6bd9 to 450f309 Compare September 3, 2026 00:49
@bkrabach
Brian Krabach (bkrabach) marked this pull request as ready for review September 3, 2026 00:50
@bkrabach

Copy link
Copy Markdown
Collaborator Author

Merge-queue verification — lane adq

Fresh scratch clone (scratch/merge9/amplifier-app-cli). PR branch was opened before #292 (31ad917, the resume-thread-role fix, lane n1i) merged, so it was 2 commits behind main.

Conflict check: no textual conflict. git diff origin/main pr-head --stat (pre-update) showed churn in session_spawner.py / session_runner.py / runtime/config.py / two test files that turned out to be entirely #292's own changes appearing only because this branch predated them, not an overlap — #292 touches resume/spawn plumbing, #293 touches only commands/routing.py + a new lib/routing_provenance.py + its own test file. git rebase origin/main applied with zero conflicts, proving independence. Pushed the rebase to lane/adq-routing-list-shadowing (force-with-lease); the PR now diffs cleanly against current main — confirmed via gh pr diff --name-only: exactly 4 files (routing.py, routing_provenance.py, the lane's own DONE-NOTE, test_routing_shadowing.py).

Gate Method Result
(a) shadowed setup marks the suppressed matrix AND names the winning file test_shadowed_matrix_is_marked_and_winner_named — asserts "shadows bundle" on the row, "in use" / "suppressed" with both concrete paths present, plus a non-vacuity check: the same tree through the pre-change (no-provenance) path shows neither marker PASS
(b) unshadowed setup's output is UNCHANGED (test required) test_unshadowed_output_is_byte_identical_to_pre_change and test_unshadowed_json_is_byte_identical_to_pre_change — literal string-equality against the pre-change path, both text and JSON PASS
(c) command still works with no user routing dir at all test_no_user_routing_dir_at_all PASS
(d) consumes ell's published fields rather than re-deriving discovery test_loads_the_bundles_own_resolve_matrix_source proves the loaded function is literally the bundle's own file on disk via fn.__code__.co_filename — not reimplemented; graceful degradation covered by test_no_marker_when_bundle_has_no_matrix_loader / test_no_marker_when_loader_lacks_the_function PASS
(e) PR body shows a before/after sample Present in the PR description (real run against a fixture home with the actual upstream matrix_loader.py) PASS
(f) full suite green uv sync --all-extras --dev; uv run pytest -q1609 passed, 1 skipped, 13 deselected, 1 xfailed; uv run pytest -m integration -q13 passed (the deselected integration tests, run as CI's separate job does). ruff check on the 3 touched source/test files: clean. (Repo-wide ruff check . shows 14 pre-existing findings elsewhere, untouched by this diff — consistent with the PR's own note of pre-existing findings.) PASS
Diff scope matches title 4 files: commands/routing.py, new lib/routing_provenance.py, the lane's DONE-NOTE, tests/test_routing_shadowing.py. Nothing in session_spawner.py/resume path touched. PASS
Performance/measurement claims None made by this PR. N/A

Known incompleteness (not a blocker, noted per instructions): the underlying _load_all_matrices() row-selection still keys by last-write-wins over sorted(files) rather than the loader's actual precedence rule — it agrees with the real rule today only by alphabetical accident (cache < routing) and can diverge. This PR's own provenance lookup reads by the winning row's file stem (the loader's actual key), so the marker is correct regardless, but the row-selection defect itself is untouched. The PR body says this was filed as its own item (9kk) for a follow-up lane to fix; I did not independently verify the tracker entry but take the PR's disclosure at face value since it's explicitly named as a known, non-blocking gap rather than hidden.

All gates pass. Merging with --admin (required-review ruleset on this repo; I am the PR author).

@bkrabach
Brian Krabach (bkrabach) merged commit 90b6b3b into main Sep 3, 2026
7 of 9 checks passed
Brian Krabach (bkrabach) added a commit that referenced this pull request Sep 3, 2026
…-write-wins (#294)

* fix(routing): mark a matrix that shadows a same-named matrix in `routing list`/`show`

`amplifier routing list` showed a user matrix and a bundle matrix of the
same name as peers. Only one is ever loaded: hooks-routing's mount()
searches `[*custom_routing_dirs, bundle routing/]` and takes the first
hit, so a file in ~/.amplifier/routing/ silently makes the shipped bundle
matrix dead -- and nothing in the CLI said so. That is why a matrix change
shipped in the bundle can be completely inert on a host.

The precedence rule is NOT re-derived here. It is consumed from
hooks-routing's own `resolve_matrix_source()` (routing-matrix PR #52),
loaded by file path out of the same cached bundle directory the CLI
already globs -- app-cli does not depend on hooks-routing as a
distribution, and `routing list` never mounts a bundle, so neither an
import nor the session-time `model_role_resolver` capability is reachable
from this process.

When the cached bundle predates PR #52 (no `resolve_matrix_source`), the
CLI draws no marker at all rather than guessing the search order: a wrong
shadowing claim is worse than none. Unshadowed output is byte-identical
to before, in both text and JSON.

- amplifier_app_cli/lib/routing_provenance.py: locate + load the bundle's
  matrix_loader, classify custom vs bundle routing dirs, resolve one
  MatrixSource per matrix name.
- commands/routing.py: row marker (`⚠ shadows bundle`), a footer naming
  the file in use and each file it suppresses, the same note on
  `routing show`, and MatrixSource.to_dict() in `--format json`.
- tests/test_routing_shadowing.py: 15 tests -- shadowed marks the winner,
  unshadowed output byte-identical, no user routing dir, and the
  old-bundle degradation path.

* docs(adq): lane DONE-NOTE — findings, seam rationale, honest test limitation

* fix(routing): select the listed matrix by the loader's rule, not last-write-wins

`amplifier routing list` built its rows with

    matrices[data["name"]] = (data, path)   # over sorted(discovered files)

which diverges from hooks-routing in two independent ways at once: it keyed on
the `name:` field INSIDE each YAML (the loader resolves by file STEM), and it let
the LAST file in sort order win (the loader takes the FIRST hit in
`[*custom_routing_dirs, bundle routing/]`).

The two rules agreed only by alphabetical accident -- `~/.amplifier/cache/...`
sorts before `~/.amplifier/routing/...` because "c" < "r". When they disagree the
command asserts something false: it names a file as in use that the loader would
never read. A user file `my-fast.yaml` declaring `name: balanced` overwrote the
row for the real `balanced` matrix outright.

Rows are now keyed by file stem, and the file behind each row is
`MatrixSource.path` -- the value hooks-routing's own `resolve_matrix_source()`
assigns to `matrix_path` and loads. Those fields are reachable from the CLI
through the seam PR #293 already built, so precedence is not re-derived a third
time. Only the winning file is parsed, so a shadowed file can no longer supply a
row's description, `updated:` date or compatibility count.

Also: `routing use` now writes the filename the loader resolves (it could write
an unloadable internal name before), a `name:`/stem disagreement is surfaced
rather than silently keyed on the internal name, and every JSON row carries
`matrix_file`.

When the cached bundle predates routing-matrix PR #52 there is no
`resolve_matrix_source` to ask. The shadowing MARKER is still withheld (#293's
rule: a wrong marker is worse than none), but a row must point at some file, so
selection falls back to the first candidate in `[*custom_dirs, *bundle_dirs]` --
the same list hooks-routing builds as `search_dirs`, in one labelled function.

Tests: `tests/test_routing_winner_selection.py` constructs both disagreement
classes explicitly, each with a non-vacuity gate that re-runs the old algorithm
inline and asserts it picks the other file. Full suite green (1605 passed);
existing routing list/show/use tests unmodified.

---------

Co-authored-by: Amplifier Lane adq <lane-adq@localhost>
Co-authored-by: Amplifier <240397093+microsoft-amplifier@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.

1 participant