Repository navigation
fix(media): unadvertise out-of-credit media sellers, clear video status, announced video fields - #1201
Conversation
bc7d79f to
99dbf2e
Compare
343b490 to
0d442e9
Compare
d9f09cd to
914e965
Compare
06e0834 to
577503a
Compare
…us, announced video fields
…ideos per-clip prices, multi-clip, frame cropping, 400 docs
…eo checks models with an empty-prompt request - Provider.checkUpstreamAccount (new, optional): key valid and account funded, once per provider. Venice reads api_keys/rate_limits; fal reads billing when FAL_VIDEO_BILLING_API_KEY is set. - Provider.checkServiceHealth now only answers whether each model is still offered. The health checker combines both per model. - Venice video model check: an empty-prompt POST /api/v1/video/queue, rejected without generating. 400 means the model exists, 404 means it is gone, anything else changes nothing. Replaces the public model-list lookup. If Venice ever accepts it, the seller warns and stops checking that model. - fal keeps its public model list: its queue accepts an empty request as a real job.
… lookups send the key and pace the per-model fallback - Venice: the empty-prompt queue request counted against the key's 40 video requests per minute (98 of 138 models got 429 in a full sweep). The model check now reads GET /api/v1/models?type=video, which is free and doesn't touch that limit; a model no longer listed is hidden. - fal: model lookups send the seller's key, since fal rate-limits anonymous lookups. The per-model fallback after 'Endpoint(s) not found' pauses 200 ms between lookups and still stops at the first unclear answer (e.g. 429), so the round ends as unknown. - Validated with real keys: all 138 Venice video models in one call per round, no 429; 30 fal models in 25s with no 429, hiding exactly the retired fal-ai/sora-2/text-to-video and a made-up model.
…e account and model checks
… is off, unadvertise a rejected key
b431c6b to
668c563
Compare
…video field names but treats the list as possibly incomplete
…e at startup, with a free rejected create
668c563 to
42ba7bf
Compare
…(video_not_delivered)
…lists none; pass fal's failure reason; clearer video errors
|
Superseded by a newer review: #1201 (comment) (previously reviewed |
Antseed Review SummaryThis PR adds free upstream account and model-health checks for media providers, removes unhealthy services from advertisements, and clears stale buyer catalog fields. It also normalizes video-download status responses, discovers video request parameters, and updates the video workflow documentation. At the supplied head d8f1f73, the two fixes described in the re-review comment are not present.
Confidence Score: 3/5Fix existing-job retrieval after service removal and fal model recovery before merging. Tracing health removal through seller matching and both video plugins confirms that existing jobs become inaccessible; tracing fal's disabled billing check through health-result combination confirms that recovered models remain hidden. The supplied wrapper passed 86 targeted node tests, 29 fal tests, 18 Venice tests, and the full CLI suite with 1,263 passed and one skipped; an earlier CLI invocation incorrectly supplied TypeScript source paths and failed module loading, while the subsequent normal package run passed. Native SQLite coverage was unavailable, and live upstream behavior, end-to-end payment execution, and exhaustive surrounding-code coverage remain unverified; several earlier source reads were truncated. Files that need attention:
|
| Filename | Overview |
|---|---|
packages/node/src/health/model-health-checker.ts |
Adds combined free account and model checks for media services, but removal is not compatible with existing-job retrieval and disabled-check recovery. |
packages/node/src/interfaces/seller-provider.ts |
Publishes optional health hooks and result types, but lacks the disabled account-check result needed for model-only recovery. |
plugins/provider-fal-video/src/index.ts |
Adds model and balance checks, schema-derived parameters, and richer job errors; its mutable retrieve allowlist and permanent unknown billing result require fixes. |
plugins/provider-venice-video/src/index.ts |
Adds account and model checks plus parameter discovery, but its retrieve allowlist is also removed by health checks. |
apps/cli/src/proxy/buyer-proxy.ts |
Clears previously cached catalog fields when refreshed peer announcements omit them; no additional issue was verified in the changed region. |
apps/cli/src/proxy/video-download.ts |
Normalizes known job statuses and distinguishes seller responses from connection failures; the CLI test suite passes. |
apps/cli/src/proxy/routing.ts |
Adds model as a baseline parameter for both native video protocols. |
packages/node/tests/model-health-checker.test.ts |
Covers media removal and funded-account recovery, but does not exercise the claimed nullable account-check recovery. |
CHANGELOG.md |
Documents the user-facing health, catalog, status, capability, and workflow changes. |
skills/antseed-videos/references/requests.md |
Updates capability selection, request-field naming, frame preparation, polling, and multi-clip instructions. |
P1 Keep existing video jobs retrievable after health removal
P1 — packages/node/src/health/model-health-checker.ts:246
The new media-check failure path calls _recordFailure, which eventually splices the service out of provider.services. For example, a buyer queues a video, that generation exhausts the seller's upstream balance, and repeated health checks remove its model before the buyer downloads it. packages/node/src/seller-request-handler.ts:1114 still requires provider.services.includes(requestedService) for every request, including retrieves, so the handler returns HTTP 400 model_not_found before reaching the existing job. There is a second gate in both plugins: plugins/provider-fal-video/src/index.ts:242 and plugins/provider-venice-video/src/index.ts:254 validate retrieves against the same mutable services array. Thus fixing seller matching alone would still leave downloads broken. The protocol-table matching and configured-service snapshots described in the tagging comment are absent from this head.
Suggested fix
For native video retrieves, match the configured service and protocol independently of current advertisement, retaining the existing job-ownership checks. Give both plugins an immutable configured-service list for retrieves, while continuing to require advertised-service membership for creates. Add regression tests that create a job, remove its service through health checks, successfully retrieve it, and verify that a new create is still refused.
P2 Distinguish disabled billing checks from inconclusive checks
P2 — plugins/provider-fal-video/src/index.ts:317
When billing permission is unavailable, checkUpstreamAccount returns unknown on every sweep at lines 317 and 322. In packages/node/src/health/model-health-checker.ts:230, an unavailable model result takes precedence and can remove the service, but when that model becomes available again the permanent account unknown wins instead. The checker then continues at lines 238–240 without restoring the service. A temporarily unavailable fal endpoint therefore stays hidden even after recovery for sellers using keys without billing permission. The interface at packages/node/src/interfaces/seller-provider.ts:97 still returns only Promise<UpstreamAccountStatus>, and the result filter excludes only undefined, so the nullable-check fix described in the comment is not implemented.
Suggested fix
Allow an explicit disabled/not-applicable result such as null in the account-check contract, return it when fal billing checks cannot run, and exclude it when combining check results. Preserve genuine unknown results as recovery blockers. Test model unavailable-to-available recovery with billing checks disabled, plus a separate case proving a real unknown still prevents restoration.
Diagram
sequenceDiagram
participant Buyer
participant Seller
participant Provider
participant Health
participant Upstream
Buyer->>Seller: Create video job
Seller->>Provider: Forward create
Provider->>Upstream: Queue generation
Upstream-->>Buyer: Job identifier via seller
Health->>Provider: Check account and models
Provider->>Upstream: Free health requests
Upstream-->>Health: Unavailable via provider
Health->>Provider: Remove advertised service after threshold
Buyer->>Seller: Retrieve existing job
Seller-->>Buyer: Model not found because service was removed
Core files read in full: packages/node/src/health/model-health-checker.ts, packages/node/src/interfaces/seller-provider.ts, apps/cli/src/proxy/video-download.ts, apps/cli/src/proxy/buyer-proxy.ts
Reviews (2): Last reviewed commit: fix(media): unadvertise out-of-credit media sellers, clear v (d8f1f73f17) | requested by @alexanderludwig | Tag @reviewant with "re-review" to run again
Executive summary
The bug: sellers whose upstream account ran out of credits kept advertising their image and video models, and buyers kept being routed to them. Image and video models were never health-checked, because a test render costs money. Users saw it as
503 Insufficient USD or Diem balancefrom a seller selling through Venice, and the buyer kept trying that seller.What this PR does:
processing,failed,completed,unknownorseller_unreachableinstead of the same502for everything.antseed-videosskill uses them (frames, aspect ratio, reference images).Not covered yet: image models (for example Venice images through
provider-openai) still get no health check. That's the main follow-up.Details, in priority order
P0: sellers with no credits still get traffic
Image and video services skip the regular health probe (a probe render costs money), so a seller whose upstream account ran dry, or whose model was dropped upstream, kept advertising them.
Provider.checkUpstreamAccount(): account check. Is the upstream key valid and the account funded? If it fails, every media model of that provider counts a failure.Provider.checkServiceHealth(services): model check. Does each model still exist upstream? If it fails, only that model counts a failure.unavailablefrom either counts as a failure; after the existing threshold the model is removed through_removeServiceand metadata is re-signed. A model is re-advertised only when both pass;unknownchanges nothing.503, and403can mean blocked content), so only the providers' own free checks decide.GET /api/v1/api_keys/rate_limits(key,accessPermitted, USD + DIEM + bundled balance, compared withVENICE_VIDEO_MIN_BALANCE_USD, default0).GET /api/v1/models?type=video, one call per sweep. A model Venice no longer lists is unavailable.POST /api/v1/video/queueper model was tried first (400= exists,404= gone, free), but it counts against the key's 40 video requests per minute: in a sweep of all 138 models, 98 got429, which would block real buyer creates. The model list doesn't touch that limit.429, balance unchanged; a made-up model was hidden and a bad key hid every model.GET https://api.fal.ai/v1/models?endpoint_id=…. An endpoint missing or notactiveis unavailable;deprecatedstays listed, with a warning.429without a key, none with it).404 Endpoint(s) not foundif any one ID is unknown. In that case each ID is looked up on its own, 200 ms apart, so only the retired model is hidden. The first unclear answer (another404, a429) ends the sweep asunknown, which changes nothing.429, hiding exactlyfal-ai/sora-2/text-to-video(still in fal's catalogue asdeprecated, but its endpoint returns404) and a made-up model. fal allows about 78 lookups per minute per key, so a seller with far more models ends its sweep asunknowninstead of catching retired ones.422after 0.04s).FAL_VIDEO_API_KEY(no separate billing key). With fal'sbilling:usage:readpermission it compares the balance withFAL_VIDEO_MIN_BALANCE_USD. Without it (fal answers403), the seller logs at startup that the balance check is off and how to turn it on, and keeps selling. A key fal rejects (401) hides every model. Checked live: our key gets403"not permitted", a made-up key gets401"Invalid API key"._replacePeersinapps/cli/src/proxy/buyer-proxy.tsnow deletes the catalog fields the new announcement leaves out. Liveness (lastReachedAt) and the metadata merge are unchanged, and an empty discovery still falls back to the previous peers. (Cleaner long-term fix tracked in Stale catalog fields survive peer cache refresh #1251.)unknown) no longer clears the last real status code.apps/website/docs/provide/{media,config}.md.P0: video status always looked like the same 502
apps/cli/src/proxy/video-download.tsnow returns onestatus:processing|failed|completed|unknown|seller_unreachable, plusupstream_status. Seller JSON client errors pass through unchanged.P1: aspect ratio
The
antseed-videosskill sendsaspect_ratioonly to sellers that list it (sendRatioin the price output). A newfit_framehelper center-crops frames that come back in the wrong shape.Announced request fields (folded in from #1222)
supportedParametersat startup, with the upstream API's own names. fal reads them from its public input schema and Venice from its public model list. The request body still goes upstream unchanged; nothing is renamed.video.inputsone create Venice must reject (empty prompt,duration: "999s", anend_image_url). Venice's400namesend_image_urlwhen the model can't take it; otherwise the model announcesend_image_url. No job starts and nothing is charged; checks are paced for Venice's 40 video requests a minute, and an unclear answer announces the first frame only. Validated live: 30 of 80 models take a last frame, the balance was unchanged, and a real Kling O3 job went from a red first frame to a blue last frame. fal needs no such check: its input schema already lists each endpoint's end-frame field.routing.tschecksfal-videoandvenice-videorequests against that list, withmodelalways allowed.fields, picks frame and audio field names from them (for examplestart_image_urlfor fal's Kling V3), sendsaspect_ratioonly when the seller lists it, and no longer reads fal's schema from fal. A seller that lists no fields gets the previous defaults.frames=ref), keeps only sellers that accept reference images, and sends them as a list under the seller's field (reference_image_urls, orimage_urlson fal's Kling O3).000), the skill says the job may have started and asks the user before creating again.Not in this PR
/v1/video/jobswould leak jobs between gateway users; this should be a loopback/_antseed/route instead.main) for OpenAI-style image sellers (for example Venice throughprovider-openai). An empty prompt to/images/generationscan't be used: Venice answers400even for a made-up model there. Venice's own/image/generate(400vs404) or its image model list would work.Idempotency-Keyon video creates. It was in an earlier version of this PR and was removed: a replayed create lost the link between the job and its payment channel, so the buyer refused to pay on delivery. It should come back in its own PR, with the seller returning the job's channel on replay.