Comfy-Org/comfy-skills

Fork 0

mirror of https://github.com/Comfy-Org/comfy-skills.git synced 2026-11-04 23:37:45 +00:00

audio #35

New issue

Closed

opened 2026-10-26 22:34:14 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-26 22:34:14 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/Comfy-Org/comfy-skills/pull/33
Author: @mattmillerai
Created: 9/9/2026
Status: ✅ Merged
Merged: 9/16/2026
Merged by: @mattmillerai

Base: fix/partner-generate-guess-and-bounce ← Head: matt/be-10663-bounce-retry-qualifiers

📝 Commits (1)

3e8cde6 fix(skills): carry the bounce-retry qualifiers into generate-video/image/audio

📊 Changes

6 files changed (+7 additions, -7 deletions)

View changed files

📝 claude-code/commands/generate-audio.md (+1 -1)
📝 claude-code/commands/generate-image.md (+1 -1)
📝 claude-code/commands/generate-video.md (+1 -1)
📝 skills/comfy-generate-audio.md (+1 -1)
📝 skills/comfy-generate-image.md (+1 -1)
📝 skills/comfy-generate-video.md (+2 -2)

📄 Description

STACKED — merging lands on fix/partner-generate-guess-and-bounce (owned by @mattmillerai, PR #19), NOT main. Do not treat this as ready to merge to main on its own: #19 must land first, after which GitHub retargets this PR. Its diff is the net change on top of #19 only.

ELI-5

When you ask for a paid partner model by name, the tool takes a guess at the model's slug. If the guess is wrong, the server doesn't just say "no" — it says "did you mean this one?", and sometimes "…and that one needs a tier passed in params.model". #19 already taught the skills to retry with the suggested slug. This PR adds the three things that make the retry actually correct: carry the tier the bounce names (or you silently run a different-priced tier), say out loud which slug you swapped to (the suggestion is ranked by word overlap across the whole registry, so it can be a plausible-but-different model), and inside submit_batch resend only the item that bounced (its siblings are already queued and charged).

What changed

Four sentences, in both the skills/ copy and its claude-code/commands/ twin, extended to carry the three qualifiers from the merged comfy-cloud-mcp-server routing bullet (src/instructions.ts, PR #1389, copied from main rather than from the branch):

skills/comfy-generate-video.md — the "Family exists only as partner/API" bullet, and the Hailuo/MiniMax bullet.
skills/comfy-generate-image.md / skills/comfy-generate-audio.md — Step 0.
claude-code/commands/generate-{video,image,audio}.md — the same three sentences.

#19's else-branch wording is unchanged in all six files ("unknown model" with no close match named / "not yet implemented" → Step 1, or the paid api_* template per the video bullet) — verified: the with no close match named clause is still present in all 6.

skills/comfy-generate-3d.md is deliberately untouched, and this is verified rather than assumed: THREED_MODELS is {} in src/partner/client/registry/threed.ts, and in assertPartnerModelSupported (src/tools-execution.ts) the branch args.type !== 'image' && !modalityHasPersistSupport(args.type) throws templateRedirectMessage before partnerUnknownModelMessage can ever emit a Did you mean bounce. So the 3D skill's existing "continue below" is correct for every reachable outcome, and adding bounce-retry text there would describe an unreachable state.

Why these three qualifiers — evidence read from `comfy-cloud-mcp-server` `main`

Rejects before queuing / resend only the bounced item. submitBatchPartnerItem calls assertPartnerModelSupported(item) per item, and preflightBatchItem runs the same check across items[] before the batch spend gate. The per-item outcome is deliberately isolated — the source comment reads "the tool promises failures land in failed[], so a healthy sibling still submits" — so a healthy sibling is queued and charged, which is exactly why a blanket resend of the whole batch double-charges.
Carry params.model. tierRoutingSentence (src/partner/model-search.ts) appends "<tier>" is a tier of <slug> — pass params: {"model": "<tier>"}. whenever the guess matched a tier. Re-calling the bare slug runs that entry's default tier at a different price.
State the substitution. rankPartnerModels scores by token overlap across the entire registry (and, since a later change, over params.model tier names too), so a suggestion can be a plausible-but-different model. A silent swap is how a user gets billed for a model they did not ask for.

Verification

This repo has no test suite, linter or formatter (CI is detect-unreviewed-merge.yml only), so verification is diff review plus checks against the artifacts named in the work item:

Exactly the intended 4 sentences changed, across 6 files, 7 insertions / 7 deletions. No other file touched.
grep -c "Did you mean" = 1 in each of skills/comfy-generate-{video,image,audio}.md.
3D untouched: git diff --name-only origin/main -- '*3d*' returns 0 files.
Each of the three qualifiers present once per changed file (before queuing, params.model tier, say-which-slug, resend only the bounced item).
Every claim above about server behaviour was read from Comfy-Org/comfy-cloud-mcp-server main (not the PR branch): src/instructions.ts, src/partner/model-search.ts, src/tools-execution.ts, src/partner/client/registry/threed.ts.

Pre-flighting the upstream `guide-negative-claims` lint

After merge, the mcp-server's refresh-skills.yml opens a snapshot PR and runs src/guide-negative-claims.test.ts against it. Rather than discover that lint's verdict from a draft PR with a > [!WARNING] banner, I ported its logicalLines / splitSentences / deniesCapability / claimFingerprint logic and ran it over the changed files at three revisions. Results:

comparison	claims removed / added	existing `NEGATIVE_CLAIM_INVENTORY` rows made stale
#19 alone (`main` → #19 base)	−4 / +6	1
this PR alone (#19 base → head)	−5 / +6	1
what the refresh PR will see (`main` → head)	−5 / +8	2

This is the lint working as designed — it exists to make a changed negative claim a deliberate act — not a sign the wording is wrong, so the fix is inventory rows upstream, not a reworded snapshot. The important part is which row is mine, and it is called out under Residual below.

Judgment calls

Stacked rather than blocked. The work item's prerequisite says to stop if #19 is unmerged. #19 is still open (REVIEW_REQUIRED, mergeable, opened 2026-08-20) and none of its rewrite is on main. The standing fleet directive is the opposite of stopping — when the blocker is an unmerged PR that carries buildable code, stack on its branch rather than gate — and stacking also satisfies the prerequisite's actual intent, which was "do not fork a competing rewrite of the same lines": this branch builds on #19's exact text and re-words none of it.
The Hailuo mirror was not applied to claude-code/commands/generate-video.md. The work item said to mirror it "if that file carries the Hailuo bullet". It carries a Hailuo bullet (line 60), but that bullet is about telling the old hailuo family apart from H3 — it contains no slug-refusal clause to extend, so appending a bounce-retry qualifier there would be a non-sequitur. The commands twin's Step 0 partner bullet already carries the full qualifier set, so the guidance is not missing from that file.
Wording deviates slightly from the drafted example to keep the sentences readable: the qualifiers are a following sentence rather than a third level of nested parentheses, and the retry is conditioned ("On a bounce, re-call once with the slug it names…") rather than left as a bare imperative that reads as unconditional. All three qualifiers, the Did you mean: … marker and the before-queuing point are carried verbatim in substance.

Provenance

Authored by: agent-work loop
Verified: no repo test suite or linter exists (CI is detect-unreviewed-merge.yml only); diff review — 4 sentences × their copies, 6 files, 7 insertions / 7 deletions, nothing else touched; grep -c "Did you mean" = 1 in each of the three skills/ generate files; 3D diff vs origin/main = 0 files; else-branch clause intact in 6/6 files; ported guide-negative-claims lint run over the changed files at 3 revisions (results tabled above).
Deviations: the Hailuo mirror was not applied to claude-code/commands/generate-video.md (that file's Hailuo bullet has no slug-refusal clause to extend); wording restructured for readability as described under Judgment calls.

Residual

One existing NEGATIVE_CLAIM_INVENTORY row goes stale because of this PR specifically, and it is outside the scope of the ticket that owns the inventory refresh. That work item is scoped to updating the inventory after #19's reword; the row this PR invalidates is the Hailuo/MiniMax one, which #19 never touches: Confirm the live set with search_models({ source: "partner", q: "minimax" }) rather than trusting this list; if a slug is refused, the "unknown model" bounce names the closest real one. in Comfy-Org/comfy-cloud-mcp-server src/guide-negative-claims.ts. Its new rendered form ends … names the closest real one — re-call with that slug (and any params.model tier it names), saying which one you switched to. Unless that row is updated alongside #19's, the refresh PR will fail guide-negative-claims.test.ts's "no inventory row is stale" assertion on a row nobody is expecting, in addition to the 8 unrecorded claims the main → head comparison produces. Both guide-negative-claims.test.ts and guide-capability-assertions.test.ts are unexercised artifacts here — they live in another repo and run against a snapshot that does not exist until refresh-skills.yml builds it, so the table above is a faithful port of the lint's logic, not a run of the real suite.
The batch rationale is restated from current main, not from the mechanism the work item cited. The work item justified "resend only the bounced item" via mapWithConcurrency(items, BATCH_SUBMIT_CONCURRENCY=4, …) racing siblings into the queue. On main today, preflightBatchItem statically rejects doomed partner items before the spend gate, so the concurrency race is no longer the operative mechanism. The shipped guidance is still correct, for the reason quoted above — per-item failure isolation means healthy siblings submit and are charged regardless — but a reader reconciling this text against the cited concurrency evidence should know the two no longer describe the same code path.
No live partner_generate / submit_batch call was made. This change adds a recovery path rather than denying a capability, so the falsification trigger is not load-bearing here; nonetheless the premises it encodes (Did you mean suggestions, the params.model tier redirect, per-item batch isolation, the empty 3D registry) were verified by reading the merged source of record on comfy-cloud-mcp-server main, not by exercising the live MCP server, for which this environment has no credentials.
claude-code/commands/generate-video.md did not receive the Hailuo mirror, per the judgment call above. If a maintainer wants that file's Hailuo bullet to restate the retry rule anyway, it needs a slug-refusal clause written first — it has none today.

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/Comfy-Org/comfy-skills/pull/33 **Author:** [@mattmillerai](https://github.com/mattmillerai) **Created:** 9/9/2026 **Status:** ✅ Merged **Merged:** 9/16/2026 **Merged by:** [@mattmillerai](https://github.com/mattmillerai) **Base:** `fix/partner-generate-guess-and-bounce` ← **Head:** `matt/be-10663-bounce-retry-qualifiers` --- ### 📝 Commits (1) - [`3e8cde6`](https://github.com/Comfy-Org/comfy-skills/commit/3e8cde6051257b32b35ae53db1f0f3579adce0e1) fix(skills): carry the bounce-retry qualifiers into generate-video/image/audio ### 📊 Changes **6 files changed** (+7 additions, -7 deletions) <details> <summary>View changed files</summary> 📝 `claude-code/commands/generate-audio.md` (+1 -1) 📝 `claude-code/commands/generate-image.md` (+1 -1) 📝 `claude-code/commands/generate-video.md` (+1 -1) 📝 `skills/comfy-generate-audio.md` (+1 -1) 📝 `skills/comfy-generate-image.md` (+1 -1) 📝 `skills/comfy-generate-video.md` (+2 -2) </details> ### 📄 Description > **STACKED — merging lands on `fix/partner-generate-guess-and-bounce` (owned by @mattmillerai, PR #19), NOT `main`.** Do not treat this as ready to merge to `main` on its own: #19 must land first, after which GitHub retargets this PR. Its diff is the net change on top of #19 only. ## ELI-5 When you ask for a paid partner model by name, the tool takes a guess at the model's slug. If the guess is wrong, the server doesn't just say "no" — it says "did you mean *this* one?", and sometimes "…and that one needs a tier passed in `params.model`". #19 already taught the skills to retry with the suggested slug. This PR adds the three things that make the retry actually correct: carry the tier the bounce names (or you silently run a different-priced tier), say out loud which slug you swapped to (the suggestion is ranked by word overlap across the whole registry, so it can be a plausible-but-different model), and inside `submit_batch` resend only the item that bounced (its siblings are already queued and charged). ## What changed Four sentences, in both the `skills/` copy and its `claude-code/commands/` twin, extended to carry the three qualifiers from the merged `comfy-cloud-mcp-server` routing bullet (`src/instructions.ts`, PR #1389, copied from `main` rather than from the branch): - `skills/comfy-generate-video.md` — the "Family exists only as partner/API" bullet, and the Hailuo/MiniMax bullet. - `skills/comfy-generate-image.md` / `skills/comfy-generate-audio.md` — Step 0. - `claude-code/commands/generate-{video,image,audio}.md` — the same three sentences. #19's else-branch wording is unchanged in all six files (`"unknown model"` **with no close match named** / `"not yet implemented"` → Step 1, or the paid `api_*` template per the video bullet) — verified: the `with no close match named` clause is still present in all 6. `skills/comfy-generate-3d.md` is deliberately untouched, and this is verified rather than assumed: `THREED_MODELS` is `{}` in `src/partner/client/registry/threed.ts`, and in `assertPartnerModelSupported` (`src/tools-execution.ts`) the branch `args.type !== 'image' && !modalityHasPersistSupport(args.type)` throws `templateRedirectMessage` **before** `partnerUnknownModelMessage` can ever emit a `Did you mean` bounce. So the 3D skill's existing "continue below" is correct for every reachable outcome, and adding bounce-retry text there would describe an unreachable state. ## Why these three qualifiers — evidence read from `comfy-cloud-mcp-server` `main` - **Rejects before queuing / resend only the bounced item.** `submitBatchPartnerItem` calls `assertPartnerModelSupported(item)` per item, and `preflightBatchItem` runs the same check across `items[]` before the batch spend gate. The per-item outcome is deliberately isolated — the source comment reads "the tool promises failures land in `failed[]`, so a healthy sibling still submits" — so a healthy sibling **is** queued and charged, which is exactly why a blanket resend of the whole batch double-charges. - **Carry `params.model`.** `tierRoutingSentence` (`src/partner/model-search.ts`) appends `"<tier>" is a tier of <slug> — pass params: {"model": "<tier>"}.` whenever the guess matched a tier. Re-calling the bare slug runs that entry's default tier at a different price. - **State the substitution.** `rankPartnerModels` scores by token overlap across the entire registry (and, since a later change, over `params.model` tier names too), so a suggestion can be a plausible-but-different model. A silent swap is how a user gets billed for a model they did not ask for. ## Verification This repo has no test suite, linter or formatter (CI is `detect-unreviewed-merge.yml` only), so verification is diff review plus checks against the artifacts named in the work item: - Exactly the intended 4 sentences changed, across 6 files, 7 insertions / 7 deletions. No other file touched. - `grep -c "Did you mean"` = 1 in each of `skills/comfy-generate-{video,image,audio}.md`. - 3D untouched: `git diff --name-only origin/main -- '*3d*'` returns 0 files. - Each of the three qualifiers present once per changed file (`before queuing`, `params.model` tier, say-which-slug, `resend only the bounced item`). - Every claim above about server behaviour was read from `Comfy-Org/comfy-cloud-mcp-server` `main` (not the PR branch): `src/instructions.ts`, `src/partner/model-search.ts`, `src/tools-execution.ts`, `src/partner/client/registry/threed.ts`. ### Pre-flighting the upstream `guide-negative-claims` lint After merge, the mcp-server's `refresh-skills.yml` opens a snapshot PR and runs `src/guide-negative-claims.test.ts` against it. Rather than discover that lint's verdict from a draft PR with a `> [!WARNING]` banner, I ported its `logicalLines` / `splitSentences` / `deniesCapability` / `claimFingerprint` logic and ran it over the changed files at three revisions. Results: | comparison | claims removed / added | existing `NEGATIVE_CLAIM_INVENTORY` rows made stale | | --- | --- | --- | | #19 alone (`main` → #19 base) | −4 / +6 | 1 | | **this PR alone** (#19 base → head) | −5 / +6 | **1** | | what the refresh PR will see (`main` → head) | −5 / +8 | 2 | This is the lint working as designed — it exists to make a changed negative claim a deliberate act — not a sign the wording is wrong, so the fix is inventory rows upstream, not a reworded snapshot. The important part is *which* row is mine, and it is called out under Residual below. ## Judgment calls - **Stacked rather than blocked.** The work item's prerequisite says to stop if #19 is unmerged. #19 is still open (`REVIEW_REQUIRED`, mergeable, opened 2026-08-20) and none of its rewrite is on `main`. The standing fleet directive is the opposite of stopping — when the blocker is an unmerged PR that carries buildable code, stack on its branch rather than gate — and stacking also satisfies the prerequisite's actual intent, which was "do not fork a competing rewrite of the same lines": this branch builds on #19's exact text and re-words none of it. - **The Hailuo mirror was not applied to `claude-code/commands/generate-video.md`.** The work item said to mirror it "if that file carries the Hailuo bullet". It carries *a* Hailuo bullet (line 60), but that bullet is about telling the old `hailuo` family apart from H3 — it contains no slug-refusal clause to extend, so appending a bounce-retry qualifier there would be a non-sequitur. The commands twin's Step 0 partner bullet already carries the full qualifier set, so the guidance is not missing from that file. - **Wording deviates slightly from the drafted example** to keep the sentences readable: the qualifiers are a following sentence rather than a third level of nested parentheses, and the retry is conditioned ("On a bounce, re-call once with the slug it names…") rather than left as a bare imperative that reads as unconditional. All three qualifiers, the `Did you mean: …` marker and the before-queuing point are carried verbatim in substance. ## Provenance - **Authored by:** agent-work loop - **Verified:** no repo test suite or linter exists (CI is `detect-unreviewed-merge.yml` only); diff review — 4 sentences × their copies, 6 files, 7 insertions / 7 deletions, nothing else touched; `grep -c "Did you mean"` = 1 in each of the three `skills/` generate files; 3D diff vs `origin/main` = 0 files; else-branch clause intact in 6/6 files; ported `guide-negative-claims` lint run over the changed files at 3 revisions (results tabled above). - **Deviations:** the Hailuo mirror was not applied to `claude-code/commands/generate-video.md` (that file's Hailuo bullet has no slug-refusal clause to extend); wording restructured for readability as described under Judgment calls. ## Residual - **One existing `NEGATIVE_CLAIM_INVENTORY` row goes stale because of *this* PR specifically, and it is outside the scope of the ticket that owns the inventory refresh.** That work item is scoped to updating the inventory after #19's reword; the row this PR invalidates is the **Hailuo/MiniMax** one, which #19 never touches: `Confirm the live set with `search_models({ source: "partner", q: "minimax" })` rather than trusting this list; if a slug is refused, the "unknown model" bounce names the closest real one.` in `Comfy-Org/comfy-cloud-mcp-server` `src/guide-negative-claims.ts`. Its new rendered form ends `… names the closest real one — re-call with that slug (and any `params.model` tier it names), saying which one you switched to.` Unless that row is updated alongside #19's, the refresh PR will fail `guide-negative-claims.test.ts`'s "no inventory row is stale" assertion on a row nobody is expecting, in addition to the 8 unrecorded claims the `main` → head comparison produces. Both `guide-negative-claims.test.ts` and `guide-capability-assertions.test.ts` are **unexercised artifacts here** — they live in another repo and run against a snapshot that does not exist until `refresh-skills.yml` builds it, so the table above is a faithful port of the lint's logic, not a run of the real suite. - **The batch rationale is restated from current `main`, not from the mechanism the work item cited.** The work item justified "resend only the bounced item" via `mapWithConcurrency(items, BATCH_SUBMIT_CONCURRENCY=4, …)` racing siblings into the queue. On `main` today, `preflightBatchItem` statically rejects doomed partner items *before* the spend gate, so the concurrency race is no longer the operative mechanism. The shipped guidance is still correct, for the reason quoted above — per-item failure isolation means healthy siblings submit and are charged regardless — but a reader reconciling this text against the cited concurrency evidence should know the two no longer describe the same code path. - **No live `partner_generate` / `submit_batch` call was made.** This change adds a recovery path rather than denying a capability, so the falsification trigger is not load-bearing here; nonetheless the premises it encodes (`Did you mean` suggestions, the `params.model` tier redirect, per-item batch isolation, the empty 3D registry) were verified by reading the merged source of record on `comfy-cloud-mcp-server` `main`, not by exercising the live MCP server, for which this environment has no credentials. - **`claude-code/commands/generate-video.md` did not receive the Hailuo mirror**, per the judgment call above. If a maintainer wants that file's Hailuo bullet to restate the retry rule anyway, it needs a slug-refusal clause written first — it has none today. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-10-26 22:34:14 +00:00

closed this issue
added the
pull-request
label

No labels

pull-request

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

Comfy-Org/comfy-skills#35

No description provided.

Rows
Columns