feat(currencies): refactor currencies package#4585
Merged
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds optional ChangesCurrency and cost-basis flow
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
mark-vass-konghq
added
release-note/feature
Contributor
Greptile SummaryThis PR refactors currency handling and adds cost-basis range support. The main changes are:
Confidence Score: 5/5This looks safe to merge.
Important Files Changed
Reviews (9): Last reviewed commit: "fix: rabbit comment and tests" | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
mark-vass-konghq
force-pushed
the
feat/reactor-currencies-api
branch
from
83d7f1f to
6a2acf8
Compare
Contributor
|
Want your agent to iterate on Greptile's feedback? Try greploops. |
![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: 5
🧹 Nitpick comments (1)
tools/migrate/currency_cost_basis_test.go (1)
11-23: 🗄️ Data Integrity & Integration | 🔵 Trivial | 🏗️ Heavy liftThis only checks file text, not migration behavior.
A migration can still fail to execute or mishandle existing rows while these substring checks keep passing. Since this change renames a persisted column and adds
effective_to, I'd back this with a DB-backed migration test that seeds a row, runs the up/down migrations, and verifies the row survives with the expected values. As per path instructions,**/*_test.go: "When appropriate, recommend e2e tests for critical changes" and "Suggest missing tests only when the PR introduces behavior without meaningful coverage, or when the current tests would pass despite a concrete regression in the changed code."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tools/migrate/currency_cost_basis_test.go` around lines 11 - 23, The current test only validates migration SQL text, so it can miss real execution failures or row-loss regressions. Add a DB-backed migration test around the currency cost basis migration that uses the existing migration helpers for 20260626133809_add_currency_cost_basis_effective_to.up.sql and .down.sql, seeds a representative row, runs the up/down path, and asserts the row still exists with the expected renamed currency_id and effective_to values. Keep the current substring assertions if useful, but back them with behavior coverage in TestCurrencyCostBasisEffectiveToMigrationPreservesRows or a new migration test.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@api/spec/packages/aip/src/currencies/cost-bases/cost-basis.tsp`:
- Around line 34-39: The `effective_to` field documentation in `cost-basis.tsp`
only mentions the open-ended case and should also document the ordering
constraint enforced by the service. Update the doc comment on `effective_to`
near the `@visibility(Lifecycle.Create, Lifecycle.Read)` declaration so it
clearly states that, when provided, it must be later than `effective_from`,
while still noting that it is otherwise open-ended until superseded. Keep the
wording aligned with the existing `effective_from`/`effective_to` contract so
generated client docs match the actual behavior.
In `@api/v3/handlers/currencies/convert_test.go`:
- Around line 14-42: The ToAPIBillingCostBasis test only covers a populated
EffectiveTo pointer, so add a second case in
TestToAPIBillingCostBasisExposesIDAndEffectiveTo that passes a
currencies.CostBasis with EffectiveTo set to nil and asserts the API model keeps
EffectiveTo nil. Keep the existing checks for Id, FiatCode, Rate, EffectiveFrom,
and CreatedAt, and use the same ToAPIBillingCostBasis conversion path to verify
both the open-ended and bounded cost basis behaviors.
In `@openmeter/currencies/service/service_test.go`:
- Around line 317-319: In the validation test for the effective_to case in
service_test.go, tighten the assertion so it checks the returned error still
uses the validation-error wrapper, not just the message substrings. Update the
test around the existing assertions on err to verify the error type/contract
exposed by the currencies service validation path, alongside the current
effective_to message checks.
In `@pkg/currencyx/currency.go`:
- Around line 78-87: NewFiatCurrency is currently treating any currency returned
by currency.Get as fiat, including non-ISO definitions. Update NewFiatCurrency
to only accept currencies whose definition has a non-empty ISONumeric field,
using the existing currency.Get lookup and the currency.Code/code symbols to
locate the definition. If the lookup is missing or ISONumeric is empty, return
the fiat currency definition error; otherwise keep the current validation and
return the code as fiat.
In `@pkg/currencyx/validation.go`:
- Around line 68-84: Wrap the aggregated result from CustomCurrency.Validate
with the repo’s standard validation wrapper instead of returning raw
errors.Join(errs...). Keep collecting all issues in errs, then return
models.NewNillableGenericValidationError(errors.Join(errs...)) so this
Validate() method matches the shared validation-error contract; apply the same
pattern to the other new Validate() methods in currencyx.
---
Nitpick comments:
In `@tools/migrate/currency_cost_basis_test.go`:
- Around line 11-23: The current test only validates migration SQL text, so it
can miss real execution failures or row-loss regressions. Add a DB-backed
migration test around the currency cost basis migration that uses the existing
migration helpers for 20260626133809_add_currency_cost_basis_effective_to.up.sql
and .down.sql, seeds a representative row, runs the up/down path, and asserts
the row still exists with the expected renamed currency_id and effective_to
values. Keep the current substring assertions if useful, but back them with
behavior coverage in TestCurrencyCostBasisEffectiveToMigrationPreservesRows or a
new migration test.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: adacf1c3-2ec4-4080-a380-6f82599c71f4
⛔ Files ignored due to path filters (12)
api/v3/openapi.yamlis excluded by!**/openapi.yamlopenmeter/ent/db/currencycostbasis.gois excluded by!**/ent/db/**openmeter/ent/db/currencycostbasis/currencycostbasis.gois excluded by!**/ent/db/**openmeter/ent/db/currencycostbasis/where.gois excluded by!**/ent/db/**openmeter/ent/db/currencycostbasis_create.gois excluded by!**/ent/db/**openmeter/ent/db/currencycostbasis_query.gois excluded by!**/ent/db/**openmeter/ent/db/currencycostbasis_update.gois excluded by!**/ent/db/**openmeter/ent/db/customcurrency/customcurrency.gois excluded by!**/ent/db/**openmeter/ent/db/customcurrency_query.gois excluded by!**/ent/db/**openmeter/ent/db/migrate/schema.gois excluded by!**/ent/db/**openmeter/ent/db/mutation.gois excluded by!**/ent/db/**tools/migrate/migrations/atlas.sumis excluded by!**/*.sum,!**/*.sum
📒 Files selected for processing (34)
.agents/skills/currencyx/SKILL.md.agents/skills/currencyx/agents/openai.yamlapi/spec/packages/aip-client-javascript/src/models/schemas.tsapi/spec/packages/aip-client-javascript/src/models/types.tsapi/spec/packages/aip/src/currencies/cost-bases/cost-basis.tspapi/v3/api.gen.goapi/v3/handlers/currencies/convert.goapi/v3/handlers/currencies/convert_test.goapi/v3/handlers/currencies/create.goapi/v3/handlers/currencies/create_cost_basis.goapi/v3/handlers/currencies/get_cost_bases.goapi/v3/handlers/currencies/list.goapi/v3/handlers/currencies/request.goopenmeter/billing/worker/subscriptionsync/service/reconciler/patchcharge_test.goopenmeter/billing/worker/subscriptionsync/service/reconciler/patchchargeflatfee.goopenmeter/billing/worker/subscriptionsync/service/reconciler/patchchargeusagebased.goopenmeter/billing/worker/subscriptionsync/service/targetstate/targetstateitem.goopenmeter/currencies/adapter.goopenmeter/currencies/adapter/adapter.goopenmeter/currencies/adapter/currencies.goopenmeter/currencies/models.goopenmeter/currencies/service/service.goopenmeter/currencies/service/service_test.goopenmeter/ent/schema/custom_currencies.gopkg/currencyx/README.mdpkg/currencyx/allocation.gopkg/currencyx/allocation_test.gopkg/currencyx/currency.gopkg/currencyx/currency_test.gopkg/currencyx/fiat.gopkg/currencyx/validation.gotools/migrate/currency_cost_basis_test.gotools/migrate/migrations/20260626133809_add_currency_cost_basis_effective_to.down.sqltools/migrate/migrations/20260626133809_add_currency_cost_basis_effective_to.up.sql
💤 Files with no reviewable changes (1)
- openmeter/currencies/adapter.go
mark-vass-konghq
force-pushed
the
feat/reactor-currencies-api
branch
from
6a2acf8 to
d7e9b9a
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
openmeter/billing/rating/service/rate/tieredgraduated.go (1)
112-137: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winFold the new currency check into aggregated validation.
Since this
Validate() errorwas touched, please collect all validation failures inerrs []errorinstead of adding another fail-fast return, so callers can see every invalid input at once. As per coding guidelines, "**/*.go: For GoValidate() errormethods, collect all validation issues intovar errs []errorand returnmodels.NewNillableGenericValidationError(errors.Join(errs...))instead of failing fast."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@openmeter/billing/rating/service/rate/tieredgraduated.go` around lines 112 - 137, The TieredPriceCalculatorInput.Validate method currently returns on the first error, but it should aggregate all validation failures. Change the validation flow to collect issues in a var errs []error, keep the existing checks for TieredPrice.Validate, GraduatedTieredPrice mode, FromQty, ToQty, and Currency, append each failure instead of returning immediately, and finish by returning models.NewNillableGenericValidationError(errors.Join(errs...)) when any errors are present.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@pkg/currencyx/fiat.go`:
- Around line 59-62: NewCalculator and Calculator.Validate() are accepting
non-ISO currency definitions, unlike NewFiatCurrency. Update the fiat checks in
those paths to require a valid definition with a non-empty ISONumeric before
treating a currency as fiat, using the existing currency.Get/currency.Code
lookup in NewCalculator and the validation logic in Calculator.Validate(). Keep
the same error style as the current fiat validation messages so all fiat
creation and validation paths enforce the same contract.
---
Nitpick comments:
In `@openmeter/billing/rating/service/rate/tieredgraduated.go`:
- Around line 112-137: The TieredPriceCalculatorInput.Validate method currently
returns on the first error, but it should aggregate all validation failures.
Change the validation flow to collect issues in a var errs []error, keep the
existing checks for TieredPrice.Validate, GraduatedTieredPrice mode, FromQty,
ToQty, and Currency, append each failure instead of returning immediately, and
finish by returning
models.NewNillableGenericValidationError(errors.Join(errs...)) when any errors
are present.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 4b2e0dba-69b7-4eab-9804-a35a4479b851
⛔ Files ignored due to path filters (12)
api/v3/openapi.yamlis excluded by!**/openapi.yamlopenmeter/ent/db/currencycostbasis.gois excluded by!**/ent/db/**openmeter/ent/db/currencycostbasis/currencycostbasis.gois excluded by!**/ent/db/**openmeter/ent/db/currencycostbasis/where.gois excluded by!**/ent/db/**openmeter/ent/db/currencycostbasis_create.gois excluded by!**/ent/db/**openmeter/ent/db/currencycostbasis_query.gois excluded by!**/ent/db/**openmeter/ent/db/currencycostbasis_update.gois excluded by!**/ent/db/**openmeter/ent/db/customcurrency/customcurrency.gois excluded by!**/ent/db/**openmeter/ent/db/customcurrency_query.gois excluded by!**/ent/db/**openmeter/ent/db/migrate/schema.gois excluded by!**/ent/db/**openmeter/ent/db/mutation.gois excluded by!**/ent/db/**tools/migrate/migrations/atlas.sumis excluded by!**/*.sum,!**/*.sum
📒 Files selected for processing (38)
.agents/skills/currencyx/SKILL.md.agents/skills/currencyx/agents/openai.yamlapi/spec/packages/aip-client-javascript/src/models/schemas.tsapi/spec/packages/aip-client-javascript/src/models/types.tsapi/spec/packages/aip/src/currencies/cost-bases/cost-basis.tspapi/v3/api.gen.goapi/v3/handlers/currencies/convert.goapi/v3/handlers/currencies/convert_test.goapi/v3/handlers/currencies/create.goapi/v3/handlers/currencies/create_cost_basis.goapi/v3/handlers/currencies/get_cost_bases.goapi/v3/handlers/currencies/handler.goapi/v3/handlers/currencies/list.goapi/v3/server/server.goopenmeter/app/stripe/calculator.goopenmeter/billing/rating/service/rate/tieredgraduated.goopenmeter/billing/worker/subscriptionsync/service/reconciler/patchcharge_test.goopenmeter/billing/worker/subscriptionsync/service/reconciler/patchchargeflatfee.goopenmeter/billing/worker/subscriptionsync/service/reconciler/patchchargeusagebased.goopenmeter/billing/worker/subscriptionsync/service/targetstate/targetstateitem.goopenmeter/currencies/adapter.goopenmeter/currencies/adapter/adapter.goopenmeter/currencies/adapter/currencies.goopenmeter/currencies/models.goopenmeter/currencies/service/service.goopenmeter/currencies/service/service_test.goopenmeter/ent/schema/custom_currencies.goopenmeter/server/router/router.gopkg/currencyx/README.mdpkg/currencyx/allocation.gopkg/currencyx/allocation_test.gopkg/currencyx/currency.gopkg/currencyx/currency_test.gopkg/currencyx/fiat.gopkg/currencyx/validation.gotools/migrate/currency_cost_basis_test.gotools/migrate/migrations/20260626133809_add_currency_cost_basis_effective_to.down.sqltools/migrate/migrations/20260626133809_add_currency_cost_basis_effective_to.up.sql
💤 Files with no reviewable changes (1)
- openmeter/currencies/adapter.go
✅ Files skipped from review due to trivial changes (2)
- .agents/skills/currencyx/SKILL.md
- pkg/currencyx/README.md
🚧 Files skipped from review as they are similar to previous changes (24)
- api/spec/packages/aip/src/currencies/cost-bases/cost-basis.tsp
- openmeter/billing/worker/subscriptionsync/service/reconciler/patchchargeflatfee.go
- api/v3/handlers/currencies/create.go
- api/v3/handlers/currencies/convert_test.go
- .agents/skills/currencyx/agents/openai.yaml
- openmeter/currencies/models.go
- openmeter/billing/worker/subscriptionsync/service/reconciler/patchchargeusagebased.go
- api/spec/packages/aip-client-javascript/src/models/schemas.ts
- openmeter/billing/worker/subscriptionsync/service/reconciler/patchcharge_test.go
- api/v3/handlers/currencies/create_cost_basis.go
- openmeter/billing/worker/subscriptionsync/service/targetstate/targetstateitem.go
- openmeter/currencies/service/service.go
- api/spec/packages/aip-client-javascript/src/models/types.ts
- tools/migrate/migrations/20260626133809_add_currency_cost_basis_effective_to.down.sql
- openmeter/currencies/adapter/adapter.go
- tools/migrate/migrations/20260626133809_add_currency_cost_basis_effective_to.up.sql
- api/v3/handlers/currencies/convert.go
- pkg/currencyx/allocation_test.go
- openmeter/ent/schema/custom_currencies.go
- pkg/currencyx/allocation.go
- openmeter/currencies/adapter/currencies.go
- openmeter/currencies/service/service_test.go
- pkg/currencyx/validation.go
- pkg/currencyx/currency_test.go
mark-vass-konghq
force-pushed
the
feat/reactor-currencies-api
branch
from
d7e9b9a to
ca7eb79
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@pkg/currencyx/currency.go`:
- Around line 16-23: CurrencyType.Validate currently returns a raw fmt.Errorf on
invalid values, which breaks the package’s standard validation contract. Update
CurrencyType.Validate to follow the usual validation pattern used elsewhere in
the repo: collect issues in a var errs []error, append the invalid-type error
for the default case, and return
models.NewNillableGenericValidationError(errors.Join(errs...)). Keep the
existing CurrencyType.Validate symbol and make sure it matches the repo’s
validation shape consistently.
In `@pkg/currencyx/fiat.go`:
- Around line 20-27: The RoundingMode.Validate method currently returns a raw
fmt.Errorf on invalid input, unlike the package’s standard aggregated validation
pattern. Update RoundingMode.Validate to collect issues in a var errs []error,
append the invalid-mode error when needed, and return
models.NewNillableGenericValidationError(errors.Join(errs...)) instead of
returning a direct error; keep the valid cases returning nil.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 357f10e1-b7d8-4370-a718-6ced72938f7a
⛔ Files ignored due to path filters (12)
api/v3/openapi.yamlis excluded by!**/openapi.yamlopenmeter/ent/db/currencycostbasis.gois excluded by!**/ent/db/**openmeter/ent/db/currencycostbasis/currencycostbasis.gois excluded by!**/ent/db/**openmeter/ent/db/currencycostbasis/where.gois excluded by!**/ent/db/**openmeter/ent/db/currencycostbasis_create.gois excluded by!**/ent/db/**openmeter/ent/db/currencycostbasis_query.gois excluded by!**/ent/db/**openmeter/ent/db/currencycostbasis_update.gois excluded by!**/ent/db/**openmeter/ent/db/customcurrency/customcurrency.gois excluded by!**/ent/db/**openmeter/ent/db/customcurrency_query.gois excluded by!**/ent/db/**openmeter/ent/db/migrate/schema.gois excluded by!**/ent/db/**openmeter/ent/db/mutation.gois excluded by!**/ent/db/**tools/migrate/migrations/atlas.sumis excluded by!**/*.sum,!**/*.sum
📒 Files selected for processing (38)
.agents/skills/currencyx/SKILL.md.agents/skills/currencyx/agents/openai.yamlapi/spec/packages/aip-client-javascript/src/models/schemas.tsapi/spec/packages/aip-client-javascript/src/models/types.tsapi/spec/packages/aip/src/currencies/cost-bases/cost-basis.tspapi/v3/api.gen.goapi/v3/handlers/currencies/convert.goapi/v3/handlers/currencies/convert_test.goapi/v3/handlers/currencies/create.goapi/v3/handlers/currencies/create_cost_basis.goapi/v3/handlers/currencies/get_cost_bases.goapi/v3/handlers/currencies/handler.goapi/v3/handlers/currencies/list.goapi/v3/server/server.goopenmeter/app/stripe/calculator.goopenmeter/billing/rating/service/rate/tieredgraduated.goopenmeter/billing/worker/subscriptionsync/service/reconciler/patchcharge_test.goopenmeter/billing/worker/subscriptionsync/service/reconciler/patchchargeflatfee.goopenmeter/billing/worker/subscriptionsync/service/reconciler/patchchargeusagebased.goopenmeter/billing/worker/subscriptionsync/service/targetstate/targetstateitem.goopenmeter/currencies/adapter.goopenmeter/currencies/adapter/adapter.goopenmeter/currencies/adapter/currencies.goopenmeter/currencies/models.goopenmeter/currencies/service/service.goopenmeter/currencies/service/service_test.goopenmeter/ent/schema/custom_currencies.goopenmeter/server/router/router.gopkg/currencyx/README.mdpkg/currencyx/allocation.gopkg/currencyx/allocation_test.gopkg/currencyx/currency.gopkg/currencyx/currency_test.gopkg/currencyx/fiat.gopkg/currencyx/validation.gotools/migrate/currency_cost_basis_test.gotools/migrate/migrations/20260626133809_add_currency_cost_basis_effective_to.down.sqltools/migrate/migrations/20260626133809_add_currency_cost_basis_effective_to.up.sql
💤 Files with no reviewable changes (1)
- openmeter/currencies/adapter.go
✅ Files skipped from review due to trivial changes (3)
- .agents/skills/currencyx/SKILL.md
- pkg/currencyx/README.md
- openmeter/billing/rating/service/rate/tieredgraduated.go
🚧 Files skipped from review as they are similar to previous changes (30)
- api/spec/packages/aip-client-javascript/src/models/schemas.ts
- openmeter/billing/worker/subscriptionsync/service/reconciler/patchchargeusagebased.go
- openmeter/server/router/router.go
- tools/migrate/migrations/20260626133809_add_currency_cost_basis_effective_to.down.sql
- openmeter/billing/worker/subscriptionsync/service/reconciler/patchchargeflatfee.go
- openmeter/billing/worker/subscriptionsync/service/targetstate/targetstateitem.go
- api/spec/packages/aip-client-javascript/src/models/types.ts
- .agents/skills/currencyx/agents/openai.yaml
- api/v3/server/server.go
- api/v3/handlers/currencies/create.go
- api/v3/handlers/currencies/handler.go
- api/v3/handlers/currencies/list.go
- openmeter/billing/worker/subscriptionsync/service/reconciler/patchcharge_test.go
- tools/migrate/migrations/20260626133809_add_currency_cost_basis_effective_to.up.sql
- api/spec/packages/aip/src/currencies/cost-bases/cost-basis.tsp
- pkg/currencyx/allocation_test.go
- api/v3/handlers/currencies/convert_test.go
- pkg/currencyx/allocation.go
- tools/migrate/currency_cost_basis_test.go
- openmeter/currencies/service/service.go
- api/v3/handlers/currencies/convert.go
- api/v3/handlers/currencies/create_cost_basis.go
- api/v3/handlers/currencies/get_cost_bases.go
- openmeter/currencies/models.go
- pkg/currencyx/validation.go
- openmeter/currencies/adapter/adapter.go
- openmeter/ent/schema/custom_currencies.go
- openmeter/currencies/service/service_test.go
- pkg/currencyx/currency_test.go
- openmeter/currencies/adapter/currencies.go
mark-vass-konghq
force-pushed
the
feat/reactor-currencies-api
branch
4 times, most recently
from
bb0b786 to
38a4902
Compare
mark-vass-konghq
force-pushed
the
feat/reactor-currencies-api
branch
from
38a4902 to
d833b2d
Compare
chrisgacsal
approved these changes
Jul 1, 2026
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
2 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 by CodeRabbit
effective_tosupport for cost-basis records across the API, models, and database migration flow.effective_tois correctly preserved, exposed, and validated (including open-ended ranges).effective_tobehavior, validation, and rounding/precision.