Ja/live pricing#3992
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
github-actions
Bot
added
go
|
json.Number is a string on the wire -
https://pkg.go.dev/encoding/json#Number
…On Fri, 17 Jul 2026 at 06:11, Rick Staa ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In ai/runner/live_runner.go
<#3992 (comment)>
:
> @@ -79,9 +79,47 @@ type LiveRunnerGPU struct {
}
type LiveRunnerPriceInfo struct {
- PricePerUnit int64 `json:"price_per_unit"`
- PixelsPerUnit int64 `json:"pixels_per_unit"`
- Unit string `json:"unit,omitempty"`
+ Price json.Number `json:"price"`
+ Currency string `json:"currency,omitempty"`
+ Unit string `json:"unit,omitempty"`
+}
+
+func (p LiveRunnerPriceInfo) priceRat() (*big.Rat, error) {
+ price := strings.TrimSpace(p.Price.String())
@j0sh <https://github.com/j0sh>, this field can not have spaces due to
how its typed right?
—
Reply to this email directly, view it on GitHub
<#3992?email_source=notifications&email_token=AACHNHQTUDVCCG5PPR4N54D5FIQYHA5CNFSNUABKM5UWIORPF5TWS5BNNB2WEL2QOVWGYUTFOF2WK43UKJSXM2LFO4XTINZSGI4TKOBZHA32M4TFMFZW63VHNVSW45DJN5XKKZLWMVXHJLDGN5XXIZLSL5RWY2LDNM#pullrequestreview-4722958987>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AACHNHVIZZ35SVCSLZTLUF35FIQYHAVCNFSNUABEKJSXA33TNF2G64TZHM4TINZSHA4TMMR3JFZXG5LFHM2DSMJRGYYTSNBVGWQXMAQ>
.
Triage notifications, keep track of coding agent tasks and review pull
requests on the go with GitHub Mobile for iOS
<https://github.com/notifications/mobile/ios/AACHNHSKVJJSMQW3PDT7LYD5FIQYHA5CNFSNUABKM5UWIORPF5TWS5BNNB2WEL2QOVWGYUTFOF2WK43UKJSXM2LFO4XTINZSGI4TKOBZHA32M4TFMFZW63VHNVSW45DJN5XKKZLWMVXHJKTGN5XXIZLSL5UW64Y>
and Android
<https://github.com/notifications/mobile/android/AACHNHS2D2KMWYPE3JBNMKD5FIQYHA5CNFSNUABKM5UWIORPF5TWS5BNNB2WEL2QOVWGYUTFOF2WK43UKJSXM2LFO4XTINZSGI4TKOBZHA32M4TFMFZW63VHNVSW45DJN5XKKZLWMVXHJLTGN5XXIZLSL5QW4ZDSN5UWI>.
Download it today!
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
Yea noticed, that when I dove in deeper so removed my comment. Strange you still got it. Probably because you responded on the email. |
|
oh yeah I couldn't find the comment here and assumed that GitHub messed up so replied in email |
|
@j0sh since I opened this PR I can't formally request changes, so flagging here instead. Overall these changes look good to me and are fine to merge in with #3938. One thing I think is overlooked (or I'm misreading the payment flow): shouldn't this also update the accounting in live_payment_processor.go? processOne still derives the billable amount from pixels (720p @ 30fps), so it charges pixelsPerSec * elapsed regardless of the new price unit. One additional question, this PR also implements option 1 from #3985 to unblock apps from being picked up in the remote discovery right? |
Correct
Great catch, can't believe I missed that. Fix incoming |
| billableUnits = pixels | ||
| } else if req.Type == RemoteType_Live { | ||
| if billableSecs <= 0 { | ||
| billableSecs = (10 * time.Second).Seconds() |
There was a problem hiding this comment.
@j0sh quick question: is the 10s prepay enough for both persistent and single-shot live runners? Was there a specific reason for 60s on live-video-to-video? 10s makes sense for single-shot, but for persistent it may be worth checking it lasts long enough.
There was a problem hiding this comment.
Not sure, 10 seconds might not be enough. We can bump it up if that feels safer. Although I imagine most single-shot requests might be better served with a separate fixed price that is set per-model. That way "slow" orchs can't take advantage of time-based pricing.
IIRC with LV2V we originally had the prepay at 30 seconds but kept running out of balance for some reason. Bumped it up to 60 and it's been fine since then, but I'm not sure if there was also an underlying issue with accounting that's since been fixed. Or maybe the bug is still there and the 60s is still masking it, but Streamdiffusion and Scope have both run single sessions on-chain for days on end.
We actually "run out" of prepay balance with Scope regularly due to serverless cold starts, but there the payment accounting doesn't start until after warm-up (which can take a few minutes). We mitigate this by 1) having the client send a payment for the total backlog as soon as the runner is ready, and 2) not checking the balance on the server until one interval after the runner is ready (~10 seconds). That gives things time to settle. It's more of a technical quirk in our implementation, but works out OK.
There was a problem hiding this comment.
Makes sense, We can keep in on 10s for now. I will then run some tests before we cut the release.
Although I imagine most single-shot requests might be better served with a separate fixed price that is set per-model. That way "slow" orchs can't take advantage of time-based pricing.
As mentioned here: https://discord.com/channels/423160867534929930/1513834345310453790/1527421806523187421, this seems useful to me.
The only combination I’m unsure about is WebSockets with single-shot requests. Those might be better metered by connection duration in seconds. However, perhaps the configuration could be flexible enough to support that use case as well.
2 participants
PR to provide comments.