Add Fireworks provider (30-day billing spend) - #2687
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0a1e4e3be5
ℹ️ 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".
| endTime: Date? = nil) -> URL | ||
| { | ||
| var components = URLComponents( | ||
| string: "https://api.fireworks.ai/v1/accounts/\\(accountSlug)/billing/summary")! |
There was a problem hiding this comment.
Fix Fireworks string interpolation
When Fireworks is configured, the extra backslash makes Swift emit a literal \\(accountSlug) instead of interpolating the slug, so the request goes to /accounts/%5C(accountSlug)/...; the same escaped interpolation at the Authorization header sends Bearer \\(cleanedKey) instead of the API key. As a result every real Fireworks usage fetch misses the account endpoint or is rejected, so remove the extra escaping in these strings.
Useful? React with 👍 / 👎.
|
Codex review: needs changes before merge. Reviewed August 8, 2026, 9:46 AM ET / 13:46 UTC. ClawSweeper reviewWhat this changesThis PR adds an optional Fireworks provider that uses an API key and account slug to show rated billing spend from the previous 30 days. Merge readinessKeep this PR open: current main does not contain a Fireworks provider, the owner has approved the feature direction, and the contributor supplied credible live API proof. One prior P2 blocker remains at the current head: the Settings billing link bypasses the fetcher’s slug validation and force-unwraps a URL built from saved input. Priority: P2 Review scores
Verification
How this fits togetherCodexBar provider descriptors turn configured credentials into usage fetches and menu-card snapshots. This provider also supplies Settings fields and a billing-page action, so the same account slug flows into both API requests and a user-triggered external URL. flowchart LR
A[API key and account slug] --> B[Fireworks provider settings]
B --> C[Credential and slug handling]
C --> D[Billing summary request]
D --> E[30-day spend snapshot]
E --> F[Menu bar card]
B --> G[Open billing action]
G --> H[Fireworks billing page]
Before merge
Findings
Agent review detailsSecurityNone. Review metrics
Root-cause clusterRelationship: Members:
Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything. Merge-risk optionsMaintainer options:
Copy recommended automerge instructionTechnical reviewBest possible solution: Share one safe account-slug validation path between the API request and billing-link builder, add direct link-construction coverage, then land the provider. Do we have a high-confidence way to reproduce the issue? Not applicable as a defect report: this is a new provider. The remaining problem is source-reproducible from the billing-link helper, while the contributor supplied redacted live endpoint output for the provider behavior. Is this the best way to solve the issue? No. The request fetcher already validates unsafe slugs, but the billing link uses the raw saved value; one shared validator and a direct helper test are the narrowest maintainable repair. Full review comments:
Overall correctness: patch is incorrect AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 26fd0bbd7eed. LabelsLabel justifications:
EvidenceAcceptance criteria:
What I checked:
Likely related people:
Rank-up movesOptional improvements that raise the rating; they are not merge blockers.
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
HistoryReview history (11 earlier review cycles; latest 8 shown)
|
|
Thanks — this is a well-shaped provider PR (clean descriptor-architecture conformance, honest spend-only scope matching what the Fireworks API offers, solid focused tests, and the recomputed gatekeeper fingerprints were appreciated). Two things before it's mergeable:
With those two, this should land without a deep re-review. |
|
Thanks for the review — both points addressed in 1. Slug robustness.
Added 2. Redacted live proof. Live call against the real account ( One transparency note: I couldn't capture a real app-menu screenshot because the full macOS app only builds in CI (I develop on a Command-Line-Tools-only host with no Xcode, where |
…-fireworks-provider
|
Pushed another upstream sync ( |
|
Landed. Verified before merge: the requested slug validation is parse-safe (no force-unwrap, credential can only route to api.fireworks.ai), the malformed-slug test exists, and the redacted live proof checks out. Clean provider-architecture conformance throughout — thanks @x0mh0x, nice first contribution! |
The Fireworks provider (#2687) landed through a path-gate that skipped the macOS lint and site-locales checks: every advertised provider count still said 67 and the new provider files were unformatted, breaking make check for any branch that runs it in full.
Summary
Adds Fireworks as a provider, showing last-30-days rated spend from the account billing summary API. Fireworks exposes no credit-balance API, so spend is the only usable usage signal (matches the ai& approach).
How it works
GET /v1/accounts/{account_slug}/billing/summary?startTime=...&endTime=with a Bearer API key; sumsunits + nanos / 1e9acrosslineItems, using the first rated currency and skipping other-currency rows.app.fireworks.ai/accounts/<slug>) is stored in config or supplied viaFIREWORKS_ACCOUNT_SLUG. A config validator flags a key-without-slug.FIREWORKS_API_KEY, orFIREWORKS_KEY. Settings shows an "Account slug" field plus an "Open Fireworks billing" action.Files
Sources/CodexBarCore/Providers/Fireworks/(descriptor + strategy, fetch/parser, settings reader, config extension, settings)Sources/CodexBar/Providers/Fireworks/(implementation + SettingsStore)ProviderIcon-fireworks.svg,docs/fireworks.md, providers table row, README line, changelog entryScripts/regenerate-provider-manifests.sh; addedfireworkscase toUsageProviderVerification
CodexBarCoremodule via a scratch core-only test target:FireworksUsageFetcherTests(parse, currency handling, empty, error mapping, URL/auth, interpolation) andFireworksSettingsReaderTests(env/config precedence). This surfaced and fixed two bugs before CI: a double-escaped\(...)string interpolation in the fetcher, and a??-precedence error in test assertions.billing/summaryresponse (9 line items →$1.120369125/ 30d).Note on local build
This Mac is CLT-only (no Xcode) with Swift 6.1.2, and CodexBar's manifest + deps require Swift 6.2/Xcode 26. The
CodexBarCoremodule (including my provider) compiles cleanly, and my tests pass against it. The fullswift testrun can't build on this toolchain because the app/widget targets use Xcode-owned@Entry/#Previewmacros and the repo's own test sources use Swift 6.2 syntax (backticked names,accuracy:), plus the pre-existingAntigravityCLISession.swiftneeds Swift 6.2. CI with Swift 6.2 will run the completemake test.Closes #615 (Fireworks provider feature request).