Merged
Conversation
18 tasks
Contributor
|
Caution Review failedPull request was closed or merged during review No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (11)
✅ Files skipped from review due to trivial changes (3)
🚧 Files skipped from review as they are similar to previous changes (6)
📝 WalkthroughSummary by CodeRabbit
WalkthroughThis change adds OCR request pricing support: new DB columns and migration, pricing model and calculation updates, response extra-field population for OCR, UI fields and types for OCR pricing overrides, and changelog entries across framework and transports. Changes
Sequence Diagram(s)sequenceDiagram
participant UI as Client/UI
participant API as Server/API
participant Pricing as PricingService
participant DB as Database
UI->>API: create/update pricing override (ocr fields)
API->>DB: persist pricing row (ocr_cost_per_page, annotation_cost_per_page)
Note over DB: Migration may add columns if absent
UI->>API: submit OCR request/result
API->>Pricing: extract usage (pages, annotated)
Pricing->>DB: read model pricing (including ocr columns)
Pricing->>API: return computed cost
API->>UI: return cost & populated OCR extra fields
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (2.11.4)level=error msg="[linters_context] typechecking error: pattern ./...: directory prefix . does not contain main module or its selected dependencies" Comment |
17 tasks
Collaborator
Author
This was referenced Apr 20, 2026
Contributor
Confidence Score: 5/5Safe to merge — all changes follow established patterns, no data-integrity or security concerns. Only a single P2 style finding (zero-page guard inconsistency that produces correct results). All other code is consistent with the existing pricing infrastructure, the migration is idempotent with proper rollback, and the UI types align with the backend. No files require special attention. Important Files Changed
Reviews (7): Last reviewed commit: "fix: add support for pricing in OCR requ..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Pratham-Mishra04
force-pushed
the
04-20-fix_per_user_oauth_in_codemode_fixes
branch
from
21c2071 to
6f43215
Compare
Pratham-Mishra04
force-pushed
the
04-20-fix_add_support_for_pricing_in_ocr_requests
branch
from
0888812 to
1d7f479
Compare
|
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@framework/modelcatalog/pricing.go`:
- Around line 403-404: The code currently sets isAnnotated using
result.OCRResponse.DocumentAnnotation != nil &&
*result.OCRResponse.DocumentAnnotation != "" which treats an empty-string
pointer as unannotated; change the check to only test pointer presence so any
non-nil DocumentAnnotation counts as annotated: set isAnnotated :=
result.OCRResponse.DocumentAnnotation != nil and keep assigning
input.ocrIsAnnotated = &isAnnotated (refer to
result.OCRResponse.DocumentAnnotation, isAnnotated, input.ocrIsAnnotated).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 1968951b-4e1a-425b-94f6-0043bd9731aa
📒 Files selected for processing (11)
core/schemas/bifrost.goframework/changelog.mdframework/configstore/migrations.goframework/configstore/tables/modelpricing.goframework/modelcatalog/pricing.goframework/modelcatalog/pricing_overrides.goframework/modelcatalog/utils.gotransports/changelog.mdui/app/workspace/custom-pricing/overrides/pricingFieldSelector.tsxui/app/workspace/custom-pricing/overrides/pricingOverrideSheet.tsxui/lib/types/governance.ts
Pratham-Mishra04
force-pushed
the
04-20-fix_per_user_oauth_in_codemode_fixes
branch
from
6f43215 to
94b3fc9
Compare
Pratham-Mishra04
force-pushed
the
04-20-fix_add_support_for_pricing_in_ocr_requests
branch
from
1d7f479 to
6335f97
Compare
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
framework/modelcatalog/pricing.go (1)
403-404:⚠️ Potential issue | 🟠 MajorBill annotation by presence, not by non-empty annotation text.
At Line 403, annotation billing is skipped when
DocumentAnnotationis a non-nil pointer to"". That undercharges OCR requests where annotation is present but empty. Downstream effect: Line 811 won’t add annotation cost in those cases.💡 Proposed fix
- isAnnotated := result.OCRResponse.DocumentAnnotation != nil && *result.OCRResponse.DocumentAnnotation != "" + isAnnotated := result.OCRResponse.DocumentAnnotation != nil input.ocrIsAnnotated = &isAnnotated🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@framework/modelcatalog/pricing.go` around lines 403 - 404, The code determines annotation presence by checking DocumentAnnotation != nil && *DocumentAnnotation != "", which treats a present-but-empty annotation as absent; change the logic in the block that sets input.ocrIsAnnotated (where result.OCRResponse.DocumentAnnotation is read) to mark annotation present whenever DocumentAnnotation is non-nil (e.g., isAnnotated := result.OCRResponse.DocumentAnnotation != nil) so downstream billing (the logic that adds annotation cost based on input.ocrIsAnnotated) correctly charges for annotated documents even when the annotation text is empty.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@framework/modelcatalog/pricing.go`:
- Around line 403-404: The code determines annotation presence by checking
DocumentAnnotation != nil && *DocumentAnnotation != "", which treats a
present-but-empty annotation as absent; change the logic in the block that sets
input.ocrIsAnnotated (where result.OCRResponse.DocumentAnnotation is read) to
mark annotation present whenever DocumentAnnotation is non-nil (e.g.,
isAnnotated := result.OCRResponse.DocumentAnnotation != nil) so downstream
billing (the logic that adds annotation cost based on input.ocrIsAnnotated)
correctly charges for annotated documents even when the annotation text is
empty.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b8f765e3-94ec-4bde-91f3-3054f4da2149
📒 Files selected for processing (11)
core/schemas/bifrost.goframework/changelog.mdframework/configstore/migrations.goframework/configstore/tables/modelpricing.goframework/modelcatalog/pricing.goframework/modelcatalog/pricing_overrides.goframework/modelcatalog/utils.gotransports/changelog.mdui/app/workspace/custom-pricing/overrides/pricingFieldSelector.tsxui/app/workspace/custom-pricing/overrides/pricingOverrideSheet.tsxui/lib/types/governance.ts
✅ Files skipped from review due to trivial changes (3)
- framework/changelog.md
- transports/changelog.md
- ui/app/workspace/custom-pricing/overrides/pricingFieldSelector.tsx
🚧 Files skipped from review as they are similar to previous changes (6)
- framework/modelcatalog/pricing_overrides.go
- ui/lib/types/governance.ts
- framework/configstore/tables/modelpricing.go
- ui/app/workspace/custom-pricing/overrides/pricingOverrideSheet.tsx
- framework/modelcatalog/utils.go
- core/schemas/bifrost.go
Pratham-Mishra04
force-pushed
the
04-20-fix_add_support_for_pricing_in_ocr_requests
branch
from
6335f97 to
b9107b1
Compare
Pratham-Mishra04
force-pushed
the
04-20-fix_per_user_oauth_in_codemode_fixes
branch
from
94b3fc9 to
6c8788b
Compare
Merged
18 tasks
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@core/mcp/utils/utils.go`:
- Around line 66-70: BuildPerUserOAuthHeaders can panic when headers is nil
because headers.Clone() returns nil; add a nil-safety guard by checking if
headers == nil (or if cloned h == nil) and initialize h = make(http.Header)
before calling h.Set("Authorization", "Bearer "+accessToken"), then return h;
reference the BuildPerUserOAuthHeaders function and the headers.Clone()/h.Set
usage when making the change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 8cdcfad1-44f1-4946-8f06-2e5746b08b22
📒 Files selected for processing (14)
core/mcp/codemode/starlark/executecode.gocore/mcp/toolmanager.gocore/mcp/utils/utils.gocore/schemas/bifrost.goframework/changelog.mdframework/configstore/migrations.goframework/configstore/tables/modelpricing.goframework/modelcatalog/pricing.goframework/modelcatalog/pricing_overrides.goframework/modelcatalog/utils.gotransports/changelog.mdui/app/workspace/custom-pricing/overrides/pricingFieldSelector.tsxui/app/workspace/custom-pricing/overrides/pricingOverrideSheet.tsxui/lib/types/governance.ts
✅ Files skipped from review due to trivial changes (3)
- framework/changelog.md
- ui/app/workspace/custom-pricing/overrides/pricingFieldSelector.tsx
- transports/changelog.md
🚧 Files skipped from review as they are similar to previous changes (5)
- framework/modelcatalog/utils.go
- ui/lib/types/governance.ts
- framework/modelcatalog/pricing.go
- framework/configstore/migrations.go
- framework/configstore/tables/modelpricing.go
Pratham-Mishra04
force-pushed
the
04-20-fix_add_support_for_pricing_in_ocr_requests
branch
from
b9107b1 to
8f2c722
Compare
Pratham-Mishra04
force-pushed
the
04-20-fix_per_user_oauth_in_codemode_fixes
branch
from
6c8788b to
7992fbe
Compare
Pratham-Mishra04
force-pushed
the
04-20-fix_per_user_oauth_in_codemode_fixes
branch
from
7992fbe to
cf03677
Compare
Pratham-Mishra04
force-pushed
the
04-20-fix_add_support_for_pricing_in_ocr_requests
branch
from
8f2c722 to
bd0b53b
Compare
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
framework/modelcatalog/pricing.go (1)
397-404:⚠️ Potential issue | 🟠 MajorBill annotation by presence, not by non-empty string content.
At Line 403, requiring
*DocumentAnnotation != ""can skip annotation charges when annotation is present but empty-string encoded, causing underbilling.💡 Proposed fix
- isAnnotated := result.OCRResponse.DocumentAnnotation != nil && *result.OCRResponse.DocumentAnnotation != "" + isAnnotated := result.OCRResponse.DocumentAnnotation != nil input.ocrIsAnnotated = &isAnnotated🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@framework/modelcatalog/pricing.go` around lines 397 - 404, The code currently sets isAnnotated by checking result.OCRResponse.DocumentAnnotation != nil && *result.OCRResponse.DocumentAnnotation != "" which can miss cases where an annotation exists but is an empty string; change the check to only test presence (e.g., result.OCRResponse.DocumentAnnotation != nil) and assign that boolean to input.ocrIsAnnotated so annotation billing is based on the existence of DocumentAnnotation rather than its non-empty content (referencing result.OCRResponse.DocumentAnnotation and input.ocrIsAnnotated).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@framework/modelcatalog/pricing.go`:
- Around line 397-404: The code currently sets isAnnotated by checking
result.OCRResponse.DocumentAnnotation != nil &&
*result.OCRResponse.DocumentAnnotation != "" which can miss cases where an
annotation exists but is an empty string; change the check to only test presence
(e.g., result.OCRResponse.DocumentAnnotation != nil) and assign that boolean to
input.ocrIsAnnotated so annotation billing is based on the existence of
DocumentAnnotation rather than its non-empty content (referencing
result.OCRResponse.DocumentAnnotation and input.ocrIsAnnotated).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 724fb345-11a9-4874-824c-9bc5a84a3f23
📒 Files selected for processing (11)
core/schemas/bifrost.goframework/changelog.mdframework/configstore/migrations.goframework/configstore/tables/modelpricing.goframework/modelcatalog/pricing.goframework/modelcatalog/pricing_overrides.goframework/modelcatalog/utils.gotransports/changelog.mdui/app/workspace/custom-pricing/overrides/pricingFieldSelector.tsxui/app/workspace/custom-pricing/overrides/pricingOverrideSheet.tsxui/lib/types/governance.ts
✅ Files skipped from review due to trivial changes (2)
- framework/changelog.md
- transports/changelog.md
🚧 Files skipped from review as they are similar to previous changes (4)
- ui/app/workspace/custom-pricing/overrides/pricingFieldSelector.tsx
- framework/modelcatalog/pricing_overrides.go
- framework/modelcatalog/utils.go
- framework/configstore/migrations.go
Contributor
Merge activity
|
akshaydeo
changed the base branch from
04-20-fix_per_user_oauth_in_codemode_fixes
to
graphite-base/2864
akshaydeo
dismissed
coderabbitai[bot]’s stale review
The base branch was changed.
akshaydeo
force-pushed
the
04-20-fix_add_support_for_pricing_in_ocr_requests
branch
from
bd0b53b to
ffae11d
Compare
dominictayloruk
pushed a commit
to dominictayloruk/bifrost
that referenced
this pull request
Apr 21, 2026
## Summary Adds pricing support for OCR requests, enabling cost calculation based on the number of pages processed and whether document annotation is present. ## Changes - Added `ocr_cost_per_page` and `annotation_cost_per_page` fields to the `TableModelPricing` table and `PricingOptions` struct - Added a database migration (`add_ocr_pricing_columns`) to introduce the two new pricing columns with rollback support - Implemented `computeOCRCost` which bills per page processed, adding annotation cost on top when a document annotation is present - Wired `OCRRequest` into the pricing lookup, cost extraction, and cost calculation pipeline - Mapped `OCRRequest` to the `"ocr"` normalized request type key for pricing catalog lookups - Propagated the new OCR pricing fields through the conversion helpers between `PricingEntry` and `TableModelPricing` ## Type of change - [x] Bug fix - [ ] Feature - [ ] Refactor - [ ] Documentation - [ ] Chore/CI ## Affected areas - [x] Core (Go) - [x] Transports (HTTP) - [ ] Providers/Integrations - [ ] Plugins - [ ] UI (React) - [ ] Docs ## How to test Send an OCR request through a model that has `ocr_cost_per_page` and/or `annotation_cost_per_page` configured. Verify that the resulting cost reflects the number of pages processed, and that annotation cost is included when `DocumentAnnotation` is non-empty. ```sh go test ./... ``` Ensure the migration runs cleanly on a fresh and existing database, and that rollback drops the two columns without error. ## Breaking changes - [ ] Yes - [x] No ## Related issues Companion to the OCR request logging support added previously. ## Security considerations No new auth, secrets, or PII handling introduced. Pricing fields are internal configuration values. ## Checklist - [ ] I read `docs/contributing/README.md` and followed the guidelines - [ ] I added/updated tests where appropriate - [ ] I updated documentation where needed - [ ] I verified builds succeed (Go and UI) - [ ] I verified the CI pipeline passes locally if applicable
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds pricing support for OCR requests, enabling cost calculation based on the number of pages processed and whether document annotation is present.
Changes
ocr_cost_per_pageandannotation_cost_per_pagefields to theTableModelPricingtable andPricingOptionsstructadd_ocr_pricing_columns) to introduce the two new pricing columns with rollback supportcomputeOCRCostwhich bills per page processed, adding annotation cost on top when a document annotation is presentOCRRequestinto the pricing lookup, cost extraction, and cost calculation pipelineOCRRequestto the"ocr"normalized request type key for pricing catalog lookupsPricingEntryandTableModelPricingType of change
Affected areas
How to test
Send an OCR request through a model that has
ocr_cost_per_pageand/orannotation_cost_per_pageconfigured. Verify that the resulting cost reflects the number of pages processed, and that annotation cost is included whenDocumentAnnotationis non-empty.go test ./...Ensure the migration runs cleanly on a fresh and existing database, and that rollback drops the two columns without error.
Breaking changes
Related issues
Companion to the OCR request logging support added previously.
Security considerations
No new auth, secrets, or PII handling introduced. Pricing fields are internal configuration values.
Checklist
docs/contributing/README.mdand followed the guidelines