docs: use the real bfl/flux-2-pro id in the model-run examples - #127
docs: use the real bfl/flux-2-pro id in the model-run examples#127mattmillerai wants to merge 2 commits into
Conversation
The quickstart told a reader to call `fal-ai/flux-pro`, which cannot succeed: `flux-pro` is not an enrolled Comfy Router model under any provider, and `fal-ai` is not a Comfy provider slug either — the fal provider's slug is `fal`, and `fal-ai` is the upstream path segment inside it. Flux is served direct from Black Forest Labs under `bfl/*`. Replace it with `bfl/flux-2-pro` everywhere it appears — the README quickstart call, the `POST /v2/models/...` prose, the id-format bullet (including its three-segment variant example), the async example, the three `example:` values in the vendored Router spec, and the one percent-encoding fixture that used it as a synthetic id. Where the provider and model segments are shown apart they become `bfl` and `flux-2-pro`. Documentation and examples only: no generated code carried the string and no behaviour changes.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Team Run ID: 📒 Files selected for processing (3)
Included review availability: 0 reviews are currently available. Your included PR review attempts over the past 7 days set your current allowance at 1 review per hour. Important Approval pendingCodeRabbit has no unresolved comments, but it has not reviewed the latest commit. Use the checkbox below to review the latest commit. CodeRabbit will approve the changes if it finds no blocking issues.
📝 WalkthroughWalkthroughThe PR replaces ChangesCanonical model identifier update
Estimated code review effort: 1 (Trivial) | ~3 minutes Merge Risk: ⚪ Minimal · up to Examples and routing-test coverage now consistently use bfl/flux-2-pro. No merge-readiness risk remains. Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Full details: Docstring CoverageExplanation No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 1 files. (2 skipped: 2 unsupported.) ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
🔍 Cursor Review — Consolidated panel
Triggered by @mattmillerai.
Found 1 finding(s).
| Severity | Count |
|---|---|
| 🟢 Low | 1 |
Panel: 6/6 reviewers contributed findings.
…tests only
`spec/router-openapi.yaml` is a one-way vendored copy of the canonical Router
contract. Both `spec/README.md` ("do not hand-edit either file") and
`AGENTS.md` ("Vendored, synced one-way. Never hand-edit.") say so, and nothing
in CI catches an in-place edit: `scripts/check_drift.py` and
`tests/test_router_spec_contract.py` only compare
`RouterErrorType.x-comfy-error-types`. So the three `example:` values this PR
rewrote (`RouterModelId`, `RouterModelSegment`, `RouterProviderSegment`) would
have diverged silently from upstream and either been reverted by the next sync
or read as phantom churn in a diff reviewers are meant to take as pure upstream
change.
Reverts the vendored file to its `main` contents. The example values belong in
the canonical contract and should arrive here via a sync; the README prose and
the routing-test fixture are this repo's to fix and are unchanged by this
commit.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 participant
ELI-5
The README's very first Router example told you to run a model called
fal-ai/flux-pro. That model does not exist, so the first line a new reader copy-pastes could never work. It is wrong twice over:flux-prois not an enrolled Comfy Router model under any provider, andfal-aiis not a Comfy provider slug either — the fal provider's slug isfal, andfal-aiis the upstream path segment inside it. Flux is served direct from Black Forest Labs underbfl/*. This swaps the example forbfl/flux-2-pro, which is real, enrolled, and already the canonical id in the Router quickstart (picked there for a measured ~3.1s p50, the fastest path on the Router).What changed
Docs and one test fixture — nothing in
src/moved, no generated artifact changed, no vendored file was touched, andmodels.runbehaviour is untouched.README.md— the quickstart call, thePOST https://api.comfy.org/v2/models/...prose, the id-format bullet (both its one-segment example and its three-segment variant example, now"bfl"and"bfl/flux-2-pro/fp8"), and the async-client example.tests/test_models_run.py— the percent-encoding fixture that used the bad id as a synthetic one, renamed to the real id.Why
spec/router-openapi.yamlis not in this diffAn earlier revision of this PR also rewrote the three
example:values the vendored Router spec carries (RouterModelId,RouterModelSegment,RouterProviderSegment). That was reverted in9675e8e, on review.spec/router-openapi.yamlis a one-way vendored copy of the canonical Router contract, and bothspec/README.md("do not hand-edit either file") andAGENTS.md("Vendored, synced one-way. Never hand-edit.") say it is not this repo's to edit. Nothing in CI would have caught the edit either —scripts/check_drift.pyandtests/test_router_spec_contract.pyonly compareRouterErrorType.x-comfy-error-types, notexample:values — so the local copy would have diverged from upstream silently, then either been reverted by the next routine sync or read as phantom churn in a diff reviewers are meant to take as pure upstream change. Those three example values belong in the canonical contract; they should arrive here via a sync, not by hand.The consequence is deliberate and visible:
fal-ai/flux-prostill appears three times inspec/router-openapi.yamlon this branch. That is the vendored upstream's text, not this repo's prose.Sweep
The whole tree was swept for the string, not just the lines the ticket enumerated. It appeared on 9 lines across 3 files (README 5, spec 3, tests 1). After this change it appears on 3 lines in 1 file — the vendored spec, left untouched on purpose per the section above; 0 lines in everything this repo authors. Nothing in
src/carried it: the spec'sexample:values do not propagate intosrc/comfy_low/models/_generated.py, which is why the drift gate stays green with no regeneration.Why the test rename is safe
The renamed row's stated job is that
.,_and-are legal inside a segment and are not percent-encoded. The old id carried-in both segments;bfl/flux-2-procarries it only in the model segment. That is not a coverage loss:model_run_requestapplies the identicalquote(..., safe="")to both segments, so provider-vs-model is not a distinct code path, and the sibling rows still cover_in a provider segment (acme_labs/v1.5) and_/.in a model segment (acme/sd_xl.turbo). The new id also exercises a digit-only inner segment (2), which the old one did not.Verifying the premise, not just the diff
This change rests on a non-existence claim, so it was checked against the catalog rather than from memory: on the platform's default branch today, every enrolled Flux route lives under the
bflprovider —flux-2-proamong them, with an authored per-model input schema alongside it — and thefalprovider carries no Flux route at all. The replacement id is also the one the platform's own Router quickstart uses.Residual
spec/router-openapi.yamlis hand-synced, not generated, and the drift gate does not compareexample:values — so the stale ids there redden no check and will not self-update. Verified today: the upstream spec still carries all three originalexample:values on its default branch. Fixing it in the canonical contract, and picking it up here via a normal sync, is the correct path and is tracked separately.README.md:554andREADME.md:566(the idempotency-replay example) callclient.models.run("acme/flux/dev", ...). That is a three-segment id, whichparse_model_idrefuses locally withValueErrorbefore any request — the same string is the fixture fortest_a_three_segment_id_says_the_variant_is_not_addressable_yet. It is a different defect (a malformed id shape, not a non-existent model) and outside this change's enumerated scope, but it is a second README example a reader cannot run. The fix is to collapse it to a two-segment id.GET /v2/models/{provider}/{model}/openapi.jsonrequires authentication and answered401 No Authorization header foundfor every id probed, so the live surface could not discriminate a real id from a fake one here without a key this run does not hold. The verification above is against the platform's source of truth instead. Confirmingbfl/flux-2-proagainst the live catalog with a real key is a one-command check worth doing.Provenance
9675e8e) with the repo's pinned toolchain (ruff~=0.15.22via thedevextra) —ruff check .: all checks passed;ruff format --check .: 51 files already formatted;mypy src: no issues in 19 source files;pytest -q: 718 passed, 4 skipped;scripts/check_public_repo_hygiene.py: no internal-only references;scripts/check_drift.py: both Router checks OK (its_check_modelsstep needs the codegen binary, absent from this local venv — CI'scomfy_low codegen driftjob covers it and passes).git diff origin/main -- spec/router-openapi.yamlis empty, confirming the vendored copy is byte-identical tomain. Note the ambientruffon this machine is 0.16.0, which also reformats Markdown and reports spurious README diffs against this repo.spec/router-openapi.yamlis not in this diff". The remaining criteria are met; the items under Residual are out-of-scope findings and unexercisable artifacts, not skipped criteria.Summary by CodeRabbit
Documentation
models.runexamples and validation descriptions to use the canonicalbfl/flux-2-promodel identifier.bfl/flux-2-promodel path.Tests