Conversation
…M4 Max verdict benchmarks/coreml-probe: one-command benchmark of the prebuilt keijiro-tk/coreml-sd-turbo UNet per compute unit, with the pre-registered 125 ms decision rule. Measured on the target machine (M4 Max, macOS 15.6): 131.6 ms median on CPU_AND_GPU, 178.6 ms on ALL — NO-GO, so the local inference tier stays out of v1 (batch-2 nuance and the SDXS-512 v1.1 path recorded in the README). benchmarks/idle-billing-probe: mints a realtime token exactly like app.py, holds the socket idle for N minutes sending nothing, then checks the usage API (dashboard fallback for non-admin keys) — settles whether idle connections bill anything. Owner-run only; never touches CI. Both probes defer heavy imports so their pure helpers are unit-tested (6 new tests) without coremltools/websockets in CI. A code-review pass found 7 issues, all fixed: the WS URL no longer duplicates /realtime (matches fal's own client guard), handshake failures are caught without ever printing the JWT-bearing URL, HF-download/metadata/usage-parse errors exit with guidance, usage_params rejects naive datetimes, and the ANE path warms up longer.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 82a1ec40d5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if session["dropped_early"]: | ||
| print(f"Connection ended early: {session['dropped_early']}") | ||
| print("An early drop at token expiry is itself a finding — note the time above.") |
There was a problem hiding this comment.
When the WebSocket handshake fails, hold_idle() returns dropped_early, but this branch only prints the error and continues to query usage. Since no idle connection existed, a zero-cost response will still trigger the later guidance that idle connections are free, producing exactly the false experimental conclusion this probe is meant to prevent. Distinguish handshake failures from post-connect drops and exit without a billing verdict when connection was never established.
Useful? React with 👍 / 👎.
| usage = httpx.get( | ||
| FAL_USAGE_ENDPOINT, | ||
| headers=auth, | ||
| params=usage_params(window_start, dt.datetime.now(dt.timezone.utc)), |
There was a problem hiding this comment.
The recorded window_end marks when the socket closes, but after waiting 90 seconds this request uses the current time as the query end. Any unrelated fal usage on the account during that settlement delay is therefore included and may be misattributed to the idle socket, while the output misleadingly prints the earlier window_end as the inspected boundary. Wait before querying, but use the captured socket-close time as the usage interval's end.
Useful? React with 👍 / 👎.
1 participant
Adds the two decision-settling probes from the v1 gate list — and closes one
gate outright, because the benchmark already ran on the target machine.
Core ML go/no-go probe (
benchmarks/coreml-probe/)One command (
./run.sh) that downloads the prebuilt, img2img-capablekeijiro-tk/coreml-sd-turboUNet (~1.7 GB, cached), times the forward pass oneach compute unit against verified input schema, and prints a verdict against
the pre-registered 125 ms rule.
Measured on the M4 Max (48 GB, macOS 15.6): 131.6 ms median on CPU_AND_GPU,
178.6 ms on ALL → NO-GO. The local inference tier stays out of v1. The
README records the nuance for later: the conversion is batch-2 (~66 ms/frame
of raw compute), so a batch-1 SDXS-512 conversion is the sensible v1.1
experiment, not a lost cause.
Idle-billing probe (
benchmarks/idle-billing-probe/)Settles the last unverified cost claim: does an idle-but-connected realtime
socket bill anything? Mints a token exactly the way
app.pydoes, holds thesocket idle for N minutes sending nothing, then queries the usage API (with a
dashboard fallback for non-admin keys). Owner-run only — it uses the local
FAL_KEY; expected cost $0. Not executed in this PR; ready whenever.Review hardening
A code-review pass cross-checked the WS URL against the vendored
@fal-ai/clientand found 7 issues, all fixed here: the URL no longerduplicates
/realtime(mirrors fal's own dedup guard), connect/handshakefailures are caught with type-only messages so the JWT-bearing URL can never
reach a traceback, download/metadata/usage-parse failures exit with guidance
instead of stack traces,
usage_paramsrejects naive datetimes, and theNeural Engine path warms up 3× longer.
Test plan
pytest -q— 20 passed (6 new: schema parsing, verdict boundaries,env-parser injection inertness, URL dedup, UTC guard)
huggingface_hub installed — CI never needs the heavy deps
🤖 Generated with Claude Code