Skip to content

idea: compare_outputs MCP tool - face-crop grids, contact sheets and diff metrics across runs #871

Closed
1 of 1 issue completed

Description

@dkackman

Problem

Every quality judgement in the matrix needed local tooling: download each mp4, ffmpeg a contact sheet (every 30th frame tiled), crop each face at fixed frames into a grid (tools/facegrid2.py), and ffmpeg psnr against a baseline. An agent on a text-only client without local ffmpeg can't do any of it; get_output_frames returns one clip's frames, not an aligned comparison.

The comparisons that mattered:

  • Faces: same 300x260 native-res crop around each character at fixed frames (60, 100), rows = runs. Showed turbo smear vs base clarity at a glance, and that cache/compile rungs were indistinguishable.
  • Staging: contact sheet rows = runs, same frame indices. Showed which knobs re-stage (LoRA family, base weights) and which don't.
  • Numeric diff: PSNR vs a baseline separated "rerun noise" (44 dB) from "numerics changed" (23-27 dB) from "re-staged" (~19 dB).

Proposal

compare_outputs(outputs=[...], frames=[...], crops=[{label, x, y, w, h}] | "faces", layout="grid"|"sheet", baseline=<output>) returning one image plus per-output metrics (PSNR/SSIM vs baseline, mean abs diff, a same_staging heuristic from low-res frame similarity).

  • crops: "faces" could use a cheap face detector so the agent doesn't need coordinates (positions moved per run: B3 Howie at 244,42 vs L5 at 264,32).
  • Server-side so it works from any client; output saved under the run/workspace and returned as an image.

Questions

  • Face detection dependency (mediapipe / insightface) vs requiring coordinates?
  • Is same_staging reliable enough to report, or just give the numbers?

Source: H3 quality matrix on lem (2x RTX 3090 24 GB, 125 GB host RAM), workspace h3-quality, Known Issues ep 6 shots, seed 42. Full data and per-run assessments: ~/testing/opus/known-issues/quality/matrix.json, report quality/REPORT.md; every run in h3-quality carries a note with its config, minutes and score. Related: #861 (follow-up experiments).

Activity

  1. added
    ideaAn idea for the researcher agent to assess
    featureWork bigger than a fix: designed with Don, built in stages (harnest R11)
    owner:leadFeature lead's turn (harnest R11)
    on Oct 14, 2026
  2. dkackman commented on Oct 14, 2026

    @dkackman
    OwnerAuthor

    Plan v2: compare_outputs, aligned grids and diff metrics across runs

    Lead agent, model claude-opus-5-5 via anthropic. v2 records Don's decisions from his review of v1 (approved); see "Decisions" below.

    Decisions (from Don's review of v1)

    • Q1: a new tool, compare_outputs.
    • Q2: per-output crops, yes. Metrics always compare the same pixels: per-output crops change only what is shown. Metrics cover the shared crop when one box is given, otherwise the full frame. Acceptance case 5 is changed to match.
    • Q3: raise SURFACE_BUDGET by the measured cost, with a dated ledger entry.
    • Doc move: no line in plugins/dw/skills/minimax-h3/SKILL.md (it is at 12,282 of 12,288 bytes). The pointer goes in plugins/dw/skills/minimax-h3/references/quality-and-speed.md.
    • Route is a sync def, so the decode and SSIM CPU runs in FastAPI's threadpool.
    • Stage B, if ever filed, must first settle where the YuNet weight download happens (the server process vs the worker, where face_track downloads them today).

    Verdict: build smaller

    Build a read-only compare_outputs that returns one aligned grid (rows = outputs, columns = frames, optionally cropped) plus PSNR, SSIM and mean abs diff against a baseline. Cut from the proposal:

    • crops: "faces": deferred to a possible stage B. It needs no new dependency, because YuNet is already in dw/tasks/face_track.py. It would come back if a field report shows per-output coordinates are still the bottleneck after stage A ships.
    • same_staging: cut. dw reports findings, not verdicts (assess_output says so in its description). Instead, the bands the matrix measured go in the guide as a reading aid: ~44 dB rerun noise, 23–27 dB numerics changed, ~19 dB re-staged. It would come back if two or more comparison sessions read those bands the same way and it is wanted as a field.
    • layout: cut. A "sheet" and a "grid" are the same output with more or fewer columns.
    • Saving the image under the run/workspace: cut. dw has no run-less output, and list_gallery, run versions and delete_output all assume one can't exist. The grid comes back inline, like get_output_frames. It would come back if someone asks to keep a grid, as a keep_as that writes to the asset library the way upload_asset does.

    Value

    Build cost

    • Stage A: ~$4–6, one server and MCP session.
    • Stage B, if ever built: ~$3–5.

    Inertia

    • MCP surface: one new tool, 64 → 65.
    • Tool-description budget: SURFACE_BUDGET has 0.5 tokens of headroom (tests/test_mcp_server.py:1594). A tool with this schema costs about 200–300 tokens. Stage A raises the ceiling with a dated ledger entry (Q3).
    • REST surface: one new route, POST /api/gallery/compare.
    • Tests and docs to keep true: the stated tool count in 3 places, docs/MCP.md, docs/SERVER.md, the OpenAPI snapshot, and the guide.
    • Security boundary: not loosened. Names resolve through the same resolve_output_file / asset_file path as get_output_frames, and the decode goes through open_bounded.
    • New concept: none. It is a read-only measure, like assess_output.

    Reversibility

    • Removing the tool later is a breaking-change for MCP clients, but nothing stores data that depends on it. Low lock-in.

    Cheaper alternatives

    1. Do nothing: keep the N get_output_frames calls. This gives up the numbers entirely, plus the single aligned view.
    2. Add baseline= to get_output_frames: declined (Q1).
    3. A compare workflow task: no MCP surface cost. But it needs a queued job per comparison and writes a run output each time. That is heavier to use than the read-only call, and the GPU queue would block it behind renders.

    Overlap

    Problem and non-goals

    Problem: judging "did this knob change the picture, and how much" across runs needs local tools. A client with no ffmpeg can't do it.

    Non-goals:

    • face detection (stage B, deferred);
    • a staging verdict;
    • persisting the grid;
    • latest / vN expansion in names (gallery routes take literal names, as get_output_frames does);
    • audio comparison;
    • resizing outputs to make different sizes comparable.

    Design

    MCP

    compare_outputs(names: list[str], at: list[str|float] | None = None,
                    crop: list[int] | list[list[int]] | None = None,
                    baseline: str | None = None, max_dimension: int = 384,
                    workspace: str | None = None)
    
    • names: 2–8 gallery names or asset: references. Videos and still images can both be given. A still is a single frame, and at doesn't apply to it.
    • at: the moments to compare, using get_output_frames' grammar (seconds or "frame:N"), 1–8 of them. The default is 4 evenly spaced frames of the shortest video. With rows × columns capped at 8 × 8 = 64 cells, the grid stays within MAX_CONTACT_SHEET_FRAMES.
      • The matrix compared the same frame indices across runs at the same fps. at keeps that. Each moment resolves per output to a frame index, and outputs whose fps differs are compared at the same frame number. The reply flags this as fps_differs, so the agent knows the columns are not the same instant.
    • crop: a single [x, y, w, h] box in source pixels, applied to every output, or one box per name. It is applied before any downscale, as in get_output_image / get_output_frames.
      • One box: it is both what is shown and the region the metrics cover.
      • One box per name: each row shows its own box. Metrics ignore the per-output boxes and cover the full frame, so every metric compares the same pixels in every output. The reply says so (metrics region: full frame (per-output crops are display only)).
    • baseline: a name from names. It defaults to names[0].
    • Reply:
      • One image: a grid with rows = outputs in the given order and columns = moments. Each row is labelled with its name (shortened to the run id and file) and each column with its frame number. Over the 4 MiB budget, the whole grid shrinks together (fit_tiles_within_budget), and the reply says downscaled_to.
      • A text part with metrics for each non-baseline output, and the region they cover. These are computed per column at source resolution, on the shared crop when one box is given and on the full frame otherwise, then summarised as mean and min:
        • psnr_db (∞ is reported as identical);
        • ssim (greyscale, Gaussian window, numpy + cv2.GaussianBlur; no new dependency, no skimage);
        • mean_abs_diff (0–255).
      • When metrics are null, the reply says why. This happens when the output's frame size differs from the baseline's (size differs from baseline: 1344x768 vs 960x544), or when a moment is past an output's end. In that second case the cell is blank as well, and the reply says past end (N frames). Never resize to force a number: a resize is a difference.
    • Annotation: read-only.

    REST

    • POST /api/gallery/compare: the body is {names, at, crop, baseline, max_dimension, max_total_bytes}. POST /api/gallery/archive is the precedent for a multi-name gallery route. The handler is a sync def, so the decode and SSIM CPU runs in FastAPI's threadpool, not on the event loop.
    • Errors:
      • an unknown name is a 404 naming which one;
      • a corrupt file is a 422;
      • an oversized decode is a 413;
      • bad arguments are a 400. Bad arguments include fewer than 2 names, more than 8 names or moments, a baseline not in names, a crop out of bounds for an output, and a list of crops whose length is not len(names).

    Engine

    • Module: dw/media_compare.py holds the metrics: psnr, ssim, mean_abs_diff on uint8 RGB arrays, plus row and column assembly that reuses dw/media_frames.py (grid_tile, compose_grid).
    • Frame decoding goes through the existing dw/media.py read_frames.
    • Where it runs: the server process, not the worker, so it needs no GPU and doesn't queue. This is the same as assess_output.

    Syntax

    • None. Workflow JSON is untouched.

    Docs

    • Tool count: 64 → 65 in README.md, dw/server/guides.py and docs/MCP.md.
    • docs/MCP.md: a row in the Media table.
    • docs/SERVER.md: the new route.
    • docs/WORKFLOW_GUIDE.md: a paragraph in Assessing a run's output. It covers what each metric means, which region it covers (per-output crops are display only), and the matrix's dB bands as a reading aid, not thresholds. The bands were measured on full frames.
    • plugins/dw/skills/minimax-h3/references/quality-and-speed.md: one line pointing A/B comparisons at the tool. Not SKILL.md (at its byte cap).

    Stages

    Stage A: compare_outputs, end to end (server deploy, plus plugin reference line)

    What it builds: everything in the design above.

    Tests and docs it owes:

    • Unit tests:
      • the metrics on synthetic arrays: identical gives identical / SSIM 1.0, a known noise gives the expected PSNR, and a shift lowers SSIM;
      • the route's 400, 404, 413 and 422 cases;
      • size mismatch gives null metrics with a reason;
      • per-output crops: metrics are the full-frame values (equal to the uncropped call's);
      • a moment past the end gives a blank cell plus a reason;
      • the budget-shrink path;
      • an image + video mix.
    • MCP pins: EXPECTED_TOOLS, TOOL_WIRING, WRAPPER_HANDLER_MAP, the stated tool count, and the SURFACE_BUDGET ledger entry.
    • Other: the OpenAPI snapshot, and the docs listed above.

    Deploy: server restart on lem.

    Estimate: $4–6.

    Acceptance intent, as seen over MCP:

    1. Two reruns compared. Take two runs of the same workflow and seed, at=["frame:60","frame:100"]. The reply has one image with 2 rows × 2 columns, labelled. Metrics for the second row show a high PSNR (tens of dB) and SSIM near 1.
    2. Identical input. The same name given twice reports identical and SSIM 1.0.
    3. Different seeds. Two different seeds report a clearly lower PSNR and SSIM than case 1.
    4. Crop. crop=[x,y,w,h] shows only that region in every cell. The metrics change from the uncropped call's, because they are computed on the region.
    5. Per-output crop. With one box per name, each row shows its own region. The metrics equal the uncropped call's (full frame), whether or not the boxes are the same size, and the reply names the region as the full frame.
    6. Mixed sizes. Outputs of different resolutions still give a grid, but the mismatched rows' metrics are null with the reason naming both sizes.
    7. Mixed lengths. A moment past one output's end leaves that cell blank, with the reason in the text.
    8. Default at. Omitting at gives 4 evenly spaced columns.
    9. Still images. Two still images can be compared (one column).
    10. Refusals. Each of these is refused with a message naming the problem:
      • 1 name, or 9 names;
      • 9 moments;
      • baseline not in names;
      • a crop out of bounds;
      • a crop list of the wrong length;
      • an unknown name (the message names it).
    11. Budget. Eight 1344×768 outputs × 8 moments return within budget, with downscaled_to reported.
    12. Read-only. Nothing new appears in list_gallery or list_assets afterwards.

    Stage B (deferred, not filed): crop="faces"

    What it builds: YuNet (already pinned in face_track.py) per output per moment. The k-th face, left to right, would be cropped to a fixed box centred on it, and the reply would give each detected box. It would be filed only if the trigger under the verdict is met.

    To settle before filing: where the YuNet weight download happens. face_track downloads them in the worker today; stage B would otherwise fetch them from HF in the server process on first call.

    Order and dependencies

    Risks

    • Surface budget: stage A raises the ceiling by the measured cost, with a dated ledger entry. A careless docstring still breaks the pin.
    • Server-process CPU: decoding 8 outputs × 8 frames at 1344×768 plus SSIM takes a few seconds of CPU per call. That's acceptable for a read-only call; the sync def route keeps it off the event loop.
    • PSNR across different fps is compared by frame number, and the reply flags it rather than hiding it.

    Questions

    None open. Q1–Q3 are answered under "Decisions".

  3. added
    owner:donParked for human decision
    status:plan-reviewA feature plan is posted and waiting for Don
    and removed
    ideaAn idea for the researcher agent to assess
    owner:leadFeature lead's turn (harnest R11)
    on Oct 14, 2026
  4. dkackman commented on Oct 14, 2026

    @dkackman
    OwnerAuthor

    Review of plan v1: approve with one design fix and one doc move. The code facts were checked against current develop:

    • YuNet is in dw/tasks/face_track.py.
    • contact_sheet, grid_tile, compose_grid and MAX_CONTACT_SHEET_FRAMES = 64 are in dw/media_frames.py.
    • fit_tiles_within_budget and open_bounded are in dw/server/inline_media.py.
    • POST /api/gallery/archive is the precedent route.
    • Nothing in dw or dw_mcp computes PSNR or SSIM.

    The cuts of same_staging, layout and saving the grid are right.

    Design fix: metrics must compare the same pixels.
    With per-output crops (Q2), the plan computes metrics whenever the boxes are the same size. Two boxes at different positions are different pixels, though. PSNR between them mixes "the face moved" with "the picture changed", and the guide's reading bands (44 / 23–27 / ~19 dB) came from full-frame comparisons. Rule:

    • Per-output crops change only what is shown.
    • Metrics always cover the same region in every output: the shared crop when there is one box, otherwise the full frame.
    • Acceptance case 5 changes to match.

    Doc move: no line in minimax-h3/SKILL.md.
    It is at 12,282 of 12,288 bytes, and #862 and #868 already compete for its only trim. Put the pointer in minimax-h3/references/quality-and-speed.md, or in #862's ramp section, where "how to judge" lives.

    Minor

    • Stage B would download YuNet weights inside the server process; face_track downloads them in the worker today. Settle where the download happens before filing stage B.
    • Make the route a sync def, so the decode and SSIM CPU runs in FastAPI's threadpool.

    Questions

    • Q1: a new tool.
    • Q2: per-output crops yes, with the fix above.
    • Q3: raise SURFACE_BUDGET with a dated ledger entry.

    claude-opus-5-5 / anthropic, for Don

  5. added
    owner:leadFeature lead's turn (harnest R11)
    status:plan-approvedDon approved the feature plan (Don only)
    and removed
    owner:donParked for human decision
    status:plan-reviewA feature plan is posted and waiting for Don
    on Oct 14, 2026
  6. dkackman commented on Oct 14, 2026

    @dkackman
    OwnerAuthor

    Plan v2: changed since v1. Records Don's review decisions; scope unchanged.

    • Q1 new tool, Q2 per-output crops, Q3 raise SURFACE_BUDGET with a ledger entry: all recorded as decided.
    • Metrics always compare the same pixels: per-output crops are display only, and metrics then cover the full frame. Acceptance case 5 and a unit test now say so.
    • The plugin pointer moves from minimax-h3/SKILL.md to minimax-h3/references/quality-and-speed.md.
    • The route is a sync def (threadpool).
    • Stage B (still deferred) must settle where the YuNet download happens before it is filed.

    Lead agent, claude-opus-5-5 / anthropic

  7. 4 remaining items

  8. added
    target:mini-aiFound against a non-lem server (DW_TARGET=mini-ai); Don triages
    on Oct 14, 2026
  9. dkackman commented on Oct 14, 2026

    @dkackman
    OwnerAuthor

    Spec cases written from plan v2. Model: claude-opus-5-5 via anthropic. Each case carries pending: #889.

    #889, stage A: the compare_outputs tool

    • regression-suite-complete.md:
      • C-F454: two reruns give a labelled grid with identical or high metrics, and seconds resolve to a frame. It also defines the fixture: a CPU-only qr_code → film_grain → loop_frames workflow with size, length and fps variants, so no model and no lem-only assets.
      • C-F455: the same name twice reads identical, with SSIM 1.0, and the last frame is valid.
      • C-F456: a different output scores lower than a rerun on all three metrics, and baseline selects the reference without reordering rows.
      • C-F457: a single crop is both the display and the metrics region, and a box touching the frame edge is accepted.
      • C-F458: per-output crops (the v2 case 5) are display only, the metrics equal the uncropped ones, and the reply names the full frame.
      • C-F459: a size mismatch gives null metrics with both sizes named, and nothing is resized.
      • C-F460: a moment past the end gives past end (N frames) (49/50 off-by-one), and fps_differs is flagged.
      • C-F461: the default at is 4 evenly spaced frames of the shortest clip.
      • C-F462: stills give one column, and asset: names and still/video mixes are accepted.
      • C-F463: every refusal the plan names, plus an empty list and negative moments, with the 2-name and 8×8 boundaries accepted.
      • C-F464: an over-budget grid shrinks as a whole and reports downscaled_to, and the metrics don't change with the downscale.
      • C-F465: the tool is read-only: no gallery, asset or job entries.
      • C-F466: the guide's "Assessing a run's output" section covers the metrics, the region rule and the dB bands as a reading aid, and the tool count is 65.
    • regression-suite-security.md:
      • SE-F049: names and baseline can't reach a file outside the gallery or asset library (traversal, absolute paths, asset: escapes, percent-encoding). An existing file and a missing one get the same error, and no path is disclosed.

    Notes (not blocking):

    • A consumer can't exercise the REST route's status codes (404/422/413/400), so the cases cover only the MCP refusals.
    • The line in the plugin's minimax-h3/references/quality-and-speed.md isn't something a consumer can check. Mark it docs-review at hand-off if it's the only part left unverified.
    • Some details the plan leaves open, which the cases accept either way:
      • at=[] and negative moments may be refused or reported per cell, but a negative moment must never wrap to the end of the clip.
      • at on a still may give one column or repeat the image.
      • A single crop that fits the baseline but not another output may be refused or nulled for that row.
    • The ordering fixture uses a different QR payload, not grain, as the "heavier difference". Film grain is midtone-weighted and may do nothing on a two-tone QR, so C-F456's grain arm is optional and skips with a note.
  10. added
    owner:leadFeature lead's turn (harnest R11)
    and removed
    status:needs-specTester writes acceptance cases for the plan's stages
    on Oct 14, 2026
  11. dkackman commented on Oct 14, 2026

    @dkackman
    OwnerAuthor

    Close-out: every stage of #871 is closed and the design record is on develop. Lead agent, model claude-opus-5-5 via anthropic.

    • Closed: #871 stage A: compare_outputs MCP tool and POST /api/gallery/compare #889, stage A: compare_outputs and POST /api/gallery/compare. It was verified on mps (mini-ai) and all 14 cases pass. It had 2 bounces: one architecture review (map rows and single owners) and one verify (C-F463, negative moments).
    • Deferred, not filed: stage B (crop="faces"). Its trigger, and the YuNet download question to settle first, are in the record.
    • Design record: docs/proposals/complete/compare-outputs-complete.md, and docs/proposals/todo.md has a shipped entry. Commit 5340e63, merged into develop at 9e8f15d. Docs only, so no deploy is needed.

    verify: run every case whose source: names one of this feature's stages (#889: C-F454–C-F466, SE-F049), and close the parent if they all pass.

  12. added
    status:verifiedTester confirmed the fix via a real MCP call
    and removed
    owner:leadFeature lead's turn (harnest R11)
    on Oct 14, 2026
  13. dkackman commented on Oct 14, 2026

    @dkackman
    OwnerAuthor

    Verified, by claude-opus-5-5 via anthropic, on mps on mini-ai.lan (develop @ 9e8f15d).

    I ran every case whose source: names #889, exactly as written, over MCP. All of them pass.

    Case Result
    C-F454 / C-F455 / C-F456 pass. The basic grid, baseline default and metrics lines are as specified. I didn't run the optional grain arm.
    C-F457 pass. With no crop: 5.22 / 0.4103 / 82.115. With crop [0,0,128,128], the region line reads crop [0, 0, 128, 128] and the metrics change (6.84 / 0.6066 / 55.78). The edge box [384,384,128,128] is accepted.
    C-F458 pass. With per-output crops the region reads full frame (per-output crops are display only), and the metrics equal the no-crop ones.
    C-F459 pass in behavior (see the note below). A size mismatch is reported as size differs from baseline: …. A2 is identical. The out-of-bounds crop is refused with crop [0, 0, 600, 300] is out of bounds for …, which is 512x512.
    C-F460 pass. When a clip runs out, the column shows skipped: past end (50 frames). Frame 49 is measured and frame 50 is past end. Different frame rates add fps_differs: the videos' frame rates differ.
    C-F461 pass. The default columns are 0/40/80/120 for [A,A2] and 0/16/33/49 for [A,T].
    C-F462 pass. Two stills give columns: still. With at, the reply adds note: \at` ignored: every output is a still image`. An asset compared with its own still is identical, and a png against an mp4 is measured.
    C-F463 pass. Each bad input gets its own specific refusal, including negative moments (is negative - moments count from the start of the clip). The 8-name boundary is accepted.
    C-F464 pass. The metrics match between max_dimension 1344 and 384. Note: the 1344 grid came back at 10752×10976 with no downscaled_to, so this run never took the downscale path.
    C-F465 pass. The gallery, asset and job counts (16 / 129 / 164) and their newest entries were the same before and after. The tool is read-only.
    C-F466 pass. The guide section covers the tool, what each metric means, the shared vs per-output crop rule, "never resized", and the dB bands as "a reading aid, not thresholds". Neither the guide index nor get_server_info states a tool count, so the tool-count check had nothing to read.
    SE-F049 pass. All 18 probes (9 names, each as a name and as baseline=) were refused, with no image, no metrics and no server path. Each reply echoes only the name as sent: Unknown file: <name> - path contains a disallowed pattern, or for asset: names a validator message (segment '..' starts with '.', segment 1 is empty, '%' … is not a character this kind of name may contain). /etc/hosts and ../../../../etc/no-such-file get identical wording, so the reply doesn't reveal whether a file exists. The [G, G] control returned identical.

    C-F459 fixture note: the case expects S to be 640×384, but the fixture's qr_code renders square, so S is really 640×640 (get_gallery_metadata agrees). The reason line correctly read size differs from baseline: 640x640 vs 512x512. The case's literal is wrong, not the code. I filed the amendment as dkackman/harnest#119 rather than editing the case.

    I deleted all the fixtures (8 c-f454-cmp runs and the se-f049 run) with delete_output(job_id=…). The asset c-f462-still.png was removed during the run. CUDA hasn't seen this, so the label is verified-on:mps.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    featureWork bigger than a fix: designed with Don, built in stages (harnest R11)owner:testerTester's turn to actstatus:plan-approvedDon approved the feature plan (Don only)status:verifiedTester confirmed the fix via a real MCP calltarget:mini-aiFound against a non-lem server (DW_TARGET=mini-ai); Don triagesverified-on:mps

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions