fix(tax-code): fix tax code delete#4555
Open
borbelyr-kong wants to merge 9 commits into
Open
Conversation
Contributor
📝 WalkthroughWalkthroughAdds tax-code resolver and hook wiring, validates plan tax-code references, filters plans by tax code, removes the delete gate, and rejects soft-deleted tax codes in organization-default updates. ChangesTax code deletion guard, resolver, and plan validation
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
Contributor
Author
|
@greptileai review this draft |
Contributor
Greptile SummaryThis PR enables tax code deletion by adding a
Confidence Score: 4/5The delete guard and publish-time validation work correctly, but the adapter's GetTaxCode now returns soft-deleted records to all callers including the public GET handler, which lacks its own deleted-at guard. The core deletion-block and publish-validation flows are correct and well-tested. The removal of DeletedAtIsNil() from the adapter's GetTaxCode is intentionally noted in code comments as a design trade-off, but the public-facing GetTaxCode service method still passes through without filtering, so GET /taxcodes/{id} on a deleted tax code returns the record with deleted_at populated instead of a 404. This regression was already flagged in a prior review pass and remains unaddressed. openmeter/taxcode/adapter/taxcode.go and openmeter/taxcode/service/taxcode.go — the public GetTaxCode path is missing a deleted-at guard after the DeletedAtIsNil() filter was removed from the adapter Important Files Changed
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![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: 2
🧹 Nitpick comments (2)
openmeter/taxcode/service/hooks/planvalidator.go (1)
26-32: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAlign
Validate()with the repo’s validation aggregation convention.This method returns on first error. In this codebase,
Validate() erroris expected to aggregate errors and returnmodels.NewNillableGenericValidationError(errors.Join(errs...)).♻️ Suggested refactor
import ( "context" + "errors" "fmt" @@ func (e PlanValidatorHookConfig) Validate() error { + var errs []error + if e.PlanService == nil { - return fmt.Errorf("plan service is required") + errs = append(errs, fmt.Errorf("plan service is required")) } - return nil + return models.NewNillableGenericValidationError(errors.Join(errs...)) }As per coding guidelines
For Go Validate() error methods, prefer collecting all validation issues into var errs []error and returning models.NewNillableGenericValidationError(errors.Join(errs...)).🤖 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/taxcode/service/hooks/planvalidator.go` around lines 26 - 32, The Validate() method in PlanValidatorHookConfig currently returns immediately on the first error instead of aggregating all validation errors as per the codebase convention. Refactor the method to collect validation errors in a slice (var errs []error) rather than returning early. When the PlanService field is nil, append the error to the errs slice and continue with any other validations. Finally, return models.NewNillableGenericValidationError(errors.Join(errs...)) to aggregate and return all collected errors together.Source: Coding guidelines
openmeter/taxcode/service/hooks/planvalidator_test.go (1)
61-115: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winNice coverage on the core flow—please add the two missing error-path tests.
Would be great to add subtests for:
NewPlanValidatorHookwithPlanService: nilPreDeletewhenListPlansreturns an errorThat closes coverage for all new branches in this file.
As per path instructions
**/*_test.go: Make sure the tests are comprehensive and cover the changes.🤖 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/taxcode/service/hooks/planvalidator_test.go` around lines 61 - 115, The TestPlanValidatorHook_PreDelete test function is missing coverage for error paths. Add two new subtests: one that calls NewPlanValidatorHook with a nil PlanService value in the PlanValidatorHookConfig to verify it properly handles and returns an error, and another subtest that uses a stubPlanService configured to return an error from ListPlans (by adding an error field to the stub) and verifies that PreDelete properly propagates that error. These error-path tests will close the coverage gaps for all branches in the hook implementation.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 `@openmeter/taxcode/adapter/taxcode.go`:
- Around line 113-116: The GetTaxCode adapter query (lines 113-116) is missing
the DeletedAtIsNil() filter that all other adapter queries like
GetTaxCodeByAppMapping and ListTaxCodes include, allowing it to unexpectedly
return soft-deleted records. Fix this by adding an IncludeDeleted field to the
GetTaxCodeInput struct, then conditionally apply the DeletedAtIsNil() filter in
the GetTaxCode adapter method based on this field (defaulting to false),
ensuring callers can explicitly opt-in to retrieving soft-deleted records.
Alternatively, add DeletedAtIsNil() unconditionally to the GetTaxCode query and
refactor DeleteTaxCode.PostDelete to use a separate internal query method that
explicitly handles soft-deleted records to maintain consistency across all
adapter queries.
In `@openmeter/taxcode/service/hooks/planvalidator.go`:
- Around line 54-59: The pre-delete integrity check for tax codes uses a
hard-coded Status slice that excludes PlanStatusInvalid and will regress when
new plan statuses are introduced. Instead of explicitly listing specific
statuses in the Status field, modify the query to include all possible plan
statuses or query without status filtering (or filter only for deleted plans if
applicable) so that the integrity check catches references in plans with any
status, including invalid and future statuses.
---
Nitpick comments:
In `@openmeter/taxcode/service/hooks/planvalidator_test.go`:
- Around line 61-115: The TestPlanValidatorHook_PreDelete test function is
missing coverage for error paths. Add two new subtests: one that calls
NewPlanValidatorHook with a nil PlanService value in the PlanValidatorHookConfig
to verify it properly handles and returns an error, and another subtest that
uses a stubPlanService configured to return an error from ListPlans (by adding
an error field to the stub) and verifies that PreDelete properly propagates that
error. These error-path tests will close the coverage gaps for all branches in
the hook implementation.
In `@openmeter/taxcode/service/hooks/planvalidator.go`:
- Around line 26-32: The Validate() method in PlanValidatorHookConfig currently
returns immediately on the first error instead of aggregating all validation
errors as per the codebase convention. Refactor the method to collect validation
errors in a slice (var errs []error) rather than returning early. When the
PlanService field is nil, append the error to the errs slice and continue with
any other validations. Finally, return
models.NewNillableGenericValidationError(errors.Join(errs...)) to aggregate and
return all collected errors together.
🪄 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: e988c293-4668-4ac6-b727-1f60ca7ec354
📒 Files selected for processing (16)
api/v3/handlers/taxcodes/delete.goapp/common/taxcode.gocmd/server/wire.gocmd/server/wire_gen.goopenmeter/productcatalog/plan/adapter/adapter_test.goopenmeter/productcatalog/plan/adapter/plan.goopenmeter/productcatalog/plan/service.goopenmeter/productcatalog/tax_test.goopenmeter/server/server_test.goopenmeter/taxcode/adapter/taxcode.goopenmeter/taxcode/errors.goopenmeter/taxcode/service.goopenmeter/taxcode/service/hooks/planvalidator.goopenmeter/taxcode/service/hooks/planvalidator_test.goopenmeter/taxcode/service/service.goopenmeter/taxcode/service/taxcode.go
💤 Files with no reviewable changes (1)
- api/v3/handlers/taxcodes/delete.go
Comment on lines
+113
to
+116
| Where( | ||
| taxcodedb.Namespace(input.Namespace), | ||
| taxcodedb.ID(input.ID), | ||
| ) |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 1) Inspect TaxCode service methods, especially GetTaxCode and delete flow
ast-grep outline openmeter/taxcode/service/taxcode.go --view expanded
# 2) Find all GetTaxCode call sites in taxcode service + v3 taxcode handlers
rg -n --type=go '\bGetTaxCode\s*\(' openmeter/taxcode openmeter/api/v3/handlers/taxcodes
# 3) Check whether deleted-row guards still exist in service/handler paths
rg -n --type=go 'DeletedAt|IncludeDeleted|IsDeleted|NotFound' openmeter/taxcode openmeter/api/v3/handlers/taxcodesRepository: openmeterio/openmeter
Length of output: 2389
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 1) Get the full GetTaxCode adapter implementation (lines 106-130)
sed -n '106,130p' openmeter/taxcode/adapter/taxcode.go
# 2) Get the DeleteTaxCode service implementation, especially the refetch at line 182
sed -n '149,195p' openmeter/taxcode/service/taxcode.go
# 3) Check UpdateTaxCode service method for soft-delete guards
sed -n '34,67p' openmeter/taxcode/service/taxcode.go
# 4) Look for any NotFound or deletion-aware logic in the service
rg -n --type=go 'ErrNotFound|NotFound|soft.*delete' openmeter/taxcode/service/ -A 2Repository: openmeterio/openmeter
Length of output: 6314
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 1) Look at adapter.DeleteTaxCode implementation
sed -n '127,150p' openmeter/taxcode/adapter/taxcode.go
# 2) Check ListTaxCodes for soft-delete filtering
sed -n '60,90p' openmeter/taxcode/adapter/taxcode.go
# 3) Search for test coverage on GetTaxCode post-deletion
rg -n --type=go 'GetTaxCode.*after.*[dD]elete|[dD]elete.*GetTaxCode' openmeter/taxcode -A 5
# 4) Check if taxcodedb schema has DeletedAt and how it's used
rg -n --type=go 'DeletedAt|WithDeletedAt' openmeter/taxcode/adapterRepository: openmeterio/openmeter
Length of output: 3311
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check if GetTaxCodeInput has any IncludeDeleted parameter
rg -n --type=go 'type GetTaxCodeInput' openmeter/taxcode -A 10
# Verify the complete DeleteTaxCode adapter to see the soft-delete operation
sed -n '200,220p' openmeter/taxcode/adapter/taxcode.go
# Check if there are any tests specifically for getting a deleted tax code
rg -n --type=go 'Delete.*GetTaxCode|GetTaxCode.*[Dd]elete' openmeter/taxcode --type=go -B 2 -A 8Repository: openmeterio/openmeter
Length of output: 1235
Add an IncludeDeleted parameter to GetTaxCodeInput or refactor DeleteTaxCode.PostDelete to avoid exposing soft-deleted records.
The GetTaxCode adapter (lines 113–116) queries only by namespace+ID and lacks the DeletedAtIsNil() filter that all other adapter queries use (e.g., GetTaxCodeByAppMapping, UpdateTaxCode, ListTaxCodes). This creates an asymmetry: GetTaxCode can return soft-deleted records, but callers have no way to opt into this behavior since GetTaxCodeInput has no IncludeDeleted field.
The refetch in DeleteTaxCode.PostDelete (service line 182) relies on this unprotected access, but regular reads—like UpdateTaxCode calling GetTaxCode at service line 40—could also unexpectedly receive deleted records. Either add explicit opt-in via IncludeDeleted or refactor the delete hook pattern to preserve consistency with the rest of the adapter.
🤖 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/taxcode/adapter/taxcode.go` around lines 113 - 116, The GetTaxCode
adapter query (lines 113-116) is missing the DeletedAtIsNil() filter that all
other adapter queries like GetTaxCodeByAppMapping and ListTaxCodes include,
allowing it to unexpectedly return soft-deleted records. Fix this by adding an
IncludeDeleted field to the GetTaxCodeInput struct, then conditionally apply the
DeletedAtIsNil() filter in the GetTaxCode adapter method based on this field
(defaulting to false), ensuring callers can explicitly opt-in to retrieving
soft-deleted records. Alternatively, add DeletedAtIsNil() unconditionally to the
GetTaxCode query and refactor DeleteTaxCode.PostDelete to use a separate
internal query method that explicitly handles soft-deleted records to maintain
consistency across all adapter queries.
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
openmeter/taxcode/service/hooks/planhook.go (1)
52-82: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftThe status filtering gap flagged in the previous review is still present.
The previous review correctly identified that the hard-coded status list excludes
PlanStatusInvalid, which means a tax code can be deleted even when invalid plans reference it. Additionally, this explicit list will need manual updates when new plan statuses are introduced.🤖 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/taxcode/service/hooks/planhook.go` around lines 52 - 82, In the PreDelete method of the planHook struct, the Status filter is using a hard-coded list that excludes PlanStatusInvalid and will require manual updates when new statuses are added. Either add PlanStatusInvalid to the Status filter list to include invalid plans in the reference check, or remove the Status filter entirely if the intent is to verify that no plans of any status reference the tax code being deleted. This ensures all referenced plans are detected regardless of their status.
🤖 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.
Duplicate comments:
In `@openmeter/taxcode/service/hooks/planhook.go`:
- Around line 52-82: In the PreDelete method of the planHook struct, the Status
filter is using a hard-coded list that excludes PlanStatusInvalid and will
require manual updates when new statuses are added. Either add PlanStatusInvalid
to the Status filter list to include invalid plans in the reference check, or
remove the Status filter entirely if the intent is to verify that no plans of
any status reference the tax code being deleted. This ensures all referenced
plans are detected regardless of their status.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 9b801067-be76-44bb-94a3-abeb7bee6e98
📒 Files selected for processing (7)
app/common/taxcode.gocmd/server/wire.gocmd/server/wire_gen.goopenmeter/taxcode/service/hooks/planhook.goopenmeter/taxcode/service/hooks/planhook_test.goopenmeter/taxcode/service/organizationdefaulttaxcodes.goopenmeter/taxcode/service/organizationdefaulttaxcodes_test.go
✅ Files skipped from review due to trivial changes (2)
- openmeter/taxcode/service/hooks/planhook_test.go
- cmd/server/wire_gen.go
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
openmeter/taxcode/service/hooks/planhook_test.go (1)
48-137: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueNice, this reads really cleanly! The given/when/then comments make the two scenarios super easy to follow, and reusing the real
pctestutilsenvironment gives you genuine end-to-end confidence in the hook path. 👍One small thought for later: both subtests share the same
nsand run sequentially, so they're correctly relying on not.Parallel(). If anyone adds parallelism down the road, the shared namespace (and the plan created in the first subtest) would leak across cases. A tiny// note: subtests share ns; do not run in parallelwould save a future reader a head-scratch.🤖 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/taxcode/service/hooks/planhook_test.go` around lines 48 - 137, The shared namespace setup in TestPlanHookPreDelete means both subtests depend on the same state and must not be parallelized; add a brief note near the shared ns setup or test body making that constraint explicit. Use the TestPlanHookPreDelete subtests and the shared ns variable as the reference point so future changes don’t introduce t.Parallel() or other parallel execution that would leak plan/tax code state between cases.
🤖 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.
Nitpick comments:
In `@openmeter/taxcode/service/hooks/planhook_test.go`:
- Around line 48-137: The shared namespace setup in TestPlanHookPreDelete means
both subtests depend on the same state and must not be parallelized; add a brief
note near the shared ns setup or test body making that constraint explicit. Use
the TestPlanHookPreDelete subtests and the shared ns variable as the reference
point so future changes don’t introduce t.Parallel() or other parallel execution
that would leak plan/tax code state between cases.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 4b811fdd-7359-453d-9583-eeb5833ae377
📒 Files selected for processing (4)
openmeter/taxcode/service/hooks/planhook.goopenmeter/taxcode/service/hooks/planhook_test.goopenmeter/taxcode/service/organizationdefaulttaxcodes.goopenmeter/taxcode/service/taxcode.go
🚧 Files skipped from review as they are similar to previous changes (2)
- openmeter/taxcode/service/organizationdefaulttaxcodes.go
- openmeter/taxcode/service/hooks/planhook.go
borbelyr-kong
force-pushed
the
fix/tax-code-delete
branch
from
55d7233 to
181cff6
Compare
…on update, and use a service-backed plan hook test
borbelyr-kong
force-pushed
the
fix/tax-code-delete
branch
from
181cff6 to
96f7d97
Compare
chrisgacsal
previously approved these changes
Jul 3, 2026
…date tax codes on plan publish (#4557)
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
openmeter/productcatalog/taxcoderesolver/resolver.go (1)
43-56: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueMinor: guard empty
idfor consistency with thenamespacecheck.
Resolveexplicitly guards against an emptynamespacebut lets an emptyidfall through tor.service.GetTaxCode. The only current caller already filters empty tax code IDs upstream, so this isn't broken today, but sinceTaxCodeResolveris a public interface, it'd be safer/more self-documenting to reject emptyidthe same way as emptynamespace.🛡️ Optional guard
func (r *resolver) Resolve(ctx context.Context, namespace string, id string) (*taxcode.TaxCode, error) { if namespace == "" { return nil, errors.New("namespace is not set") } + + if id == "" { + return nil, errors.New("id is not set") + }🤖 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/productcatalog/taxcoderesolver/resolver.go` around lines 43 - 56, `resolver.Resolve` currently checks `namespace` but allows an empty `id` to reach `r.service.GetTaxCode`, which is inconsistent for a public resolver API. Add the same early validation for `id` near the existing `namespace` guard in `Resolve`, returning an error before building `taxcode.GetTaxCodeInput` when `id` is empty, so the method explicitly rejects invalid input for both fields.openmeter/productcatalog/taxcoderesolver_test.go (1)
83-169: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winNice, solid coverage — consider adding a case for the generic-error branch too.
All the not-found/deleted/skip paths are covered here. One gap: there's no test for a resolver returning a plain (non-not-found) error, which the validator wraps differently (
"failed to resolve tax code for ratecard: %w"instead ofErrCodeRateCardTaxCodeNotFound). Might be worth a quick subtest to lock in that distinction.✅ Suggested additional subtest
t.Run("generic resolver error is wrapped, not classified as not-found", func(t *testing.T) { taxCodeResolver := stubTaxCodeResolver{ namespace: "test", err: errors.New("boom"), } cards := RateCards{makeFlatFeeRateCardWithTaxCodeID("rc-1", taxCodeID)} err := ValidateRateCardsWithTaxCodes(ctx, taxCodeResolver)(cards) require.Error(t, err) var vi models.ValidationIssue require.False(t, errors.As(err, &vi), "expected a plain wrapped error, not a ValidationIssue") })🤖 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/productcatalog/taxcoderesolver_test.go` around lines 83 - 169, Add a new subtest in TestValidateRateCardsWithTaxCodes to cover the generic resolver-error path: when stubTaxCodeResolver returns a plain error (not a taxcode.NewTaxCodeNotFoundError), ValidateRateCardsWithTaxCodes should return the wrapped error from the validator and not a models.ValidationIssue. Use the existing test helpers like stubTaxCodeResolver and makeFlatFeeRateCardWithTaxCodeID, and assert that errors.As does not classify the result as ErrCodeRateCardTaxCodeNotFound.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.
Nitpick comments:
In `@openmeter/productcatalog/taxcoderesolver_test.go`:
- Around line 83-169: Add a new subtest in TestValidateRateCardsWithTaxCodes to
cover the generic resolver-error path: when stubTaxCodeResolver returns a plain
error (not a taxcode.NewTaxCodeNotFoundError), ValidateRateCardsWithTaxCodes
should return the wrapped error from the validator and not a
models.ValidationIssue. Use the existing test helpers like stubTaxCodeResolver
and makeFlatFeeRateCardWithTaxCodeID, and assert that errors.As does not
classify the result as ErrCodeRateCardTaxCodeNotFound.
In `@openmeter/productcatalog/taxcoderesolver/resolver.go`:
- Around line 43-56: `resolver.Resolve` currently checks `namespace` but allows
an empty `id` to reach `r.service.GetTaxCode`, which is inconsistent for a
public resolver API. Add the same early validation for `id` near the existing
`namespace` guard in `Resolve`, returning an error before building
`taxcode.GetTaxCodeInput` when `id` is empty, so the method explicitly rejects
invalid input for both fields.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 31b15981-1879-4711-b73e-d2bf797cf169
📒 Files selected for processing (17)
app/common/productcatalog.gocmd/billing-worker/wire_gen.gocmd/jobs/internal/wire_gen.gocmd/server/wire_gen.goopenmeter/productcatalog/errors.goopenmeter/productcatalog/plan.goopenmeter/productcatalog/plan/service/plan.goopenmeter/productcatalog/plan/service/service.goopenmeter/productcatalog/plan/service/taxcode_test.goopenmeter/productcatalog/ratecard.goopenmeter/productcatalog/taxcoderesolver.goopenmeter/productcatalog/taxcoderesolver/resolver.goopenmeter/productcatalog/taxcoderesolver_test.goopenmeter/productcatalog/testutils/env.goopenmeter/subscription/testutils/service.gotest/billing/subscription_suite.gotest/customer/testenv.go
✅ Files skipped from review due to trivial changes (2)
- cmd/jobs/internal/wire_gen.go
- cmd/server/wire_gen.go
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.
@coderabbitai @greptileai review this draft
Summary by CodeRabbit