Release v0.1.15 — Phase 15: Conversion Cost Estimator#205
Merged
Conversation
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…st-dto CONV-215: Create CreditCost DTO
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…st-breakdown-dto CONV-216: Create CreditCostBreakdown DTO
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…n-cost-estimator-contract CONV-217: Create ConversionCostEstimator contract
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…iven-conversion-cost-estimator-skeleton CONV-218: Create ConfigDrivenConversionCostEstimator skeleton
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…osts-config CONV-219: Add conversion costs config
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…age-cost CONV-220: Test image to image cost
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…to-image-cost CONV-221: Implement image to image cost
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…f-cost CONV-222: Test image to PDF cost
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…to-pdf-cost CONV-223: Implement image to PDF cost
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…own-shape CONV-224: Test cost breakdown shape
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…reakdown CONV-225: Implement cost breakdown
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…-cost-estimation-is-rejected CONV-226: Test unsupported cost estimation is rejected
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…conversion-cost-action CONV-227: Create EstimateConversionCostAction
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…n-credit-charge-model-and-migration CONV-228: Create ConversionCreditCharge model and migration
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…job-checks-credits-before-queue CONV-229: Test conversion job checks credits before queue
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…in-create-conversion-job-action CONV-230: Enforce credits in CreateConversionJobAction
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…tured-after-successful-conversion CONV-231: Test credits captured after successful conversion
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…on-successful-conversion CONV-232: Capture credits on successful conversion
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ersion-does-not-spend-credits CONV-233: Test failed conversion does not spend credits
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ted-cost-in-settings-step CONV-234: Show real estimated cost in settings step
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…-credits-ui-state CONV-235: Add insufficient credits UI state
📝 WalkthroughWalkthroughThis PR implements a complete conversion credit enforcement system. It adds cost estimation contracts and configuration-driven pricing rules, enforces credit availability before job creation, captures credits on successful conversion, and handles charge failures when jobs fail. The UI shows estimated costs and surfaces insufficient-credit errors. All layers integrate with existing job creation/processing pipelines and CreditLedger infrastructure. ChangesConversion Credit Enforcement System
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
4 issues found across 35 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="app/Actions/Conversions/CreateConversionJobAction.php">
<violation number="1" location="app/Actions/Conversions/CreateConversionJobAction.php:61">
P2: Credit enforcement uses a non-atomic balance pre-check, so concurrent requests can over-queue jobs and fail later during capture.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
|
|
||
| $balance = $this->creditLedger->balance($user); | ||
|
|
||
| if ($balance < $cost->amount) { |
There was a problem hiding this comment.
P2: Credit enforcement uses a non-atomic balance pre-check, so concurrent requests can over-queue jobs and fail later during capture.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At app/Actions/Conversions/CreateConversionJobAction.php, line 61:
<comment>Credit enforcement uses a non-atomic balance pre-check, so concurrent requests can over-queue jobs and fail later during capture.</comment>
<file context>
@@ -48,6 +54,17 @@ public function handle(
+
+ $balance = $this->creditLedger->balance($user);
+
+ if ($balance < $cost->amount) {
+ throw InsufficientCreditsException::make(
+ required: $cost->amount,
</file context>
Owner
Author
|
@CodeRabbit review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
app/Actions/Conversions/CreateConversionJobAction.php (1)
57-92:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftMake credit admission atomic with job creation.
balance()is only a read, so two concurrent requests can both pass the check and both create jobs against the same credits. The same gap also leaves a persistedConversionJobbehind if the charge insert fails after Line 68. Reserve/spend the estimated credits and create the job/charge inside one transaction, then enqueue only after the write commits.🤖 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 `@app/Actions/Conversions/CreateConversionJobAction.php` around lines 57 - 92, Balance check and job creation are not atomic: calls to $this->estimateCost->handle and $this->creditLedger->balance allow race conditions and can leave a ConversionJob if the ConversionCreditCharge insert fails. Wrap the creation of ConversionJob and ConversionCreditCharge plus the credit reservation call in a single DB transaction, and perform the credit reservation/spend via the credit ledger API (use the ledger's reserve/debit method rather than separate read) inside that transaction; only call ProcessConversionJob::dispatch($job->id) after the transaction has committed (e.g., dispatch in an after-commit callback) so the enqueue happens only when job and charge are persisted.app/Jobs/ProcessConversionJob.php (1)
65-72:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftDon't persist
Completedbefore the debit succeeds.If the worker dies after Line 70 but before
captureCredits()commits, a retry returns at Line 41 because the job is no longerQueued, so the conversion stays completed with no credit spend. The same ordering also means a transient ledger failure turns an already-recorded result intoFailed. Keep the terminal job state change and charge capture in the same transaction, and only writeCompletedafter the debit succeeds.Also applies to: 87-110
🤖 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 `@app/Jobs/ProcessConversionJob.php` around lines 65 - 72, The job persists status = ConversionStatus::Completed before charging credits, which can leave a job marked completed if the debit fails; change the ordering and wrap the charge+terminal state update in a single DB transaction so they commit atomically: inside a transaction (use DB::transaction or equivalent) call captureCredits($job, $creditLedger) first (so debit succeeds) then forceFill the terminal fields ('result_file_id', 'status' => ConversionStatus::Completed, 'progress' => 100, 'completed_at' => now()) and save the $job; apply the same transactional pattern to the other terminal-update block referenced around the 87-110 region to ensure the ledger debit and final job state are saved together.
🧹 Nitpick comments (2)
tests/Unit/Contracts/ConversionCostEstimatorContractTest.php (1)
7-9: ⚡ Quick winThis test never exercises contract resolution.
interface_exists()only proves the interface file is autoloadable. It will still pass if the container binding is missing or incorrect, so it does not cover what the test name and PR behavior promise. ResolveConversionCostEstimator::classfrom the container and assert the concrete service instead.Suggested rewrite
+use App\Services\Billing\ConfigDrivenConversionCostEstimator; + -it('can resolve conversion cost estimator contract', function () { - expect(interface_exists(ConversionCostEstimator::class))->toBeTrue(); +it('resolves the conversion cost estimator contract from the container', function () { + $estimator = app(ConversionCostEstimator::class); + + expect($estimator)->toBeInstanceOf(ConfigDrivenConversionCostEstimator::class); });🤖 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 `@tests/Unit/Contracts/ConversionCostEstimatorContractTest.php` around lines 7 - 9, The test currently only calls interface_exists(ConversionCostEstimator::class) which only checks autoloading; instead resolve the contract from the container in the "can resolve conversion cost estimator contract" test (use app()->make(ConversionCostEstimator::class) or resolve(ConversionCostEstimator::class)) and assert the resolved value is an object implementing the ConversionCostEstimator contract (e.g. assertInstanceOf(ConversionCostEstimator::class, $service)); optionally also assert it is the expected concrete class if you have a known implementation class to validate.tests/Feature/Actions/CreateConversionJobActionCreditTest.php (1)
17-18: ⚡ Quick winSeed balances through
CreditLedgerinstead offorceFill().These tests bypass the same abstraction production uses to read balances. If the ledger stops treating the account row as source of truth, the tests will keep passing while exercising a different path. Prefer a ledger helper or factory state that drives
CreditLedgersemantics for the insufficient-credit setup too.Also applies to: 33-34
🤖 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 `@tests/Feature/Actions/CreateConversionJobActionCreditTest.php` around lines 17 - 18, The test currently sets a user's balance by calling User::factory()->create() then mutating $user->creditAccount->forceFill(['balance' => 1])->save(); instead seed the balance via the same ledger abstraction used in production: create CreditLedger entries (or use a test helper/factory state that writes to CreditLedger) for the user to produce the desired insufficient balance instead of forceFill on creditAccount; update both occurrences in CreateConversionJobActionCreditTest (and the similar block at lines 33-34) to use the CreditLedger-writing helper or factory state so tests exercise the same read/write semantics as production.
🤖 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 `@app/Data/Credits/CreditCostBreakdown.php`:
- Around line 9-28: The constructor of CreditCostBreakdown currently allows
inconsistent data (base, size, features vs total); update
CreditCostBreakdown::__construct to enforce consistency by either (a) computing
$total = $base + $size + $features and assigning that value instead of accepting
an external total, or (b) validating that $total === ($base + $size + $features)
and throwing an \InvalidArgumentException on mismatch; implement the chosen
approach consistently and keep the existing negative-value checks for base,
size, features, and total.
In `@app/Jobs/ProcessConversionJob.php`:
- Around line 95-109: Inside the DB::transaction where you call
$creditLedger->spend(...) and update $charge, reload and lock the charge row
first (e.g., re-query the Charge model with lockForUpdate()) and check its
status; if the current status is not ConversionCreditChargeStatus::Estimated,
abort the transaction (do not call spend() or update captured_amount/status).
This ensures spend() is only called once and makes the capture idempotent under
duplicate execution.
In `@app/Livewire/Dashboard/DashboardConverter.php`:
- Around line 197-206: Replace the broad catch(Throwable) in the
DashboardConverter Livewire flow around the EstimateConversionCostAction::handle
call with specific billing/pricing exceptions (e.g., BillingException,
PricingUnsupportedException or whatever domain exceptions your app defines) so
only expected pricing failures are handled; when caught, set a targeted state
(for example set $this->estimatedCreditCost = null and a new flag like
$this->pricingUnsupported = true or $this->pricingErrorMessage) so the UI can
render a proper pricing error, and allow any other unexpected exceptions to
bubble up (rethrow) rather than being swallowed; keep the rest of the call using
$this->currentFile, $converter and $this->options unchanged.
In `@app/Models/ConversionCreditCharge.php`:
- Around line 28-31: The model ConversionCreditCharge currently only casts
'status' and 'breakdown_json', causing numeric DB columns to be hydrated as
strings and potentially raising a TypeError when
ProcessConversionJob::captureCredits passes $charge->estimated_amount into
CreditLedger::spend(int $amount) or assigns to captured_amount/refunded_amount.
Update the ConversionCreditCharge::$casts to include 'estimated_amount',
'captured_amount', and 'refunded_amount' as integers so these attributes are
strictly typed to int at retrieval and assignment.
In `@app/Services/Billing/ConfigDrivenConversionCostEstimator.php`:
- Line 37: The code in ConfigDrivenConversionCostEstimator.php silently defaults
$base to 1 when config("conversion_costs.rules.{$rule}.base") is missing or
malformed; change it to validate the setting and fail fast: in the method where
$base is assigned (the conversion cost estimation method using $rule and $base),
first check that the config key exists and is a valid integer/number (e.g.,
config()->has("conversion_costs.rules.{$rule}.base") and is_numeric on the raw
value), and if not throw a clear exception (InvalidArgumentException or a
domain-specific exception) indicating the missing/invalid pricing rule instead
of casting to 1. Ensure the thrown error includes the rule name to aid
debugging.
In
`@database/migrations/2026_06_02_123052_create_conversion_credit_charges_table.php`:
- Around line 14-23: The migration creates a non-unique index on
conversion_job_id but ConversionJob::creditCharge() expects a one-to-one
relation; change the schema so conversion_job_id is unique (either add
->unique() on the conversion_job_id column or replace the index with a unique
constraint) so only one charge row can reference a given conversion_job_id;
update the migration that defines the conversion_job_id column (and remove the
plain ->index('conversion_job_id')) to enforce uniqueness.
---
Outside diff comments:
In `@app/Actions/Conversions/CreateConversionJobAction.php`:
- Around line 57-92: Balance check and job creation are not atomic: calls to
$this->estimateCost->handle and $this->creditLedger->balance allow race
conditions and can leave a ConversionJob if the ConversionCreditCharge insert
fails. Wrap the creation of ConversionJob and ConversionCreditCharge plus the
credit reservation call in a single DB transaction, and perform the credit
reservation/spend via the credit ledger API (use the ledger's reserve/debit
method rather than separate read) inside that transaction; only call
ProcessConversionJob::dispatch($job->id) after the transaction has committed
(e.g., dispatch in an after-commit callback) so the enqueue happens only when
job and charge are persisted.
In `@app/Jobs/ProcessConversionJob.php`:
- Around line 65-72: The job persists status = ConversionStatus::Completed
before charging credits, which can leave a job marked completed if the debit
fails; change the ordering and wrap the charge+terminal state update in a single
DB transaction so they commit atomically: inside a transaction (use
DB::transaction or equivalent) call captureCredits($job, $creditLedger) first
(so debit succeeds) then forceFill the terminal fields ('result_file_id',
'status' => ConversionStatus::Completed, 'progress' => 100, 'completed_at' =>
now()) and save the $job; apply the same transactional pattern to the other
terminal-update block referenced around the 87-110 region to ensure the ledger
debit and final job state are saved together.
---
Nitpick comments:
In `@tests/Feature/Actions/CreateConversionJobActionCreditTest.php`:
- Around line 17-18: The test currently sets a user's balance by calling
User::factory()->create() then mutating
$user->creditAccount->forceFill(['balance' => 1])->save(); instead seed the
balance via the same ledger abstraction used in production: create CreditLedger
entries (or use a test helper/factory state that writes to CreditLedger) for the
user to produce the desired insufficient balance instead of forceFill on
creditAccount; update both occurrences in CreateConversionJobActionCreditTest
(and the similar block at lines 33-34) to use the CreditLedger-writing helper or
factory state so tests exercise the same read/write semantics as production.
In `@tests/Unit/Contracts/ConversionCostEstimatorContractTest.php`:
- Around line 7-9: The test currently only calls
interface_exists(ConversionCostEstimator::class) which only checks autoloading;
instead resolve the contract from the container in the "can resolve conversion
cost estimator contract" test (use app()->make(ConversionCostEstimator::class)
or resolve(ConversionCostEstimator::class)) and assert the resolved value is an
object implementing the ConversionCostEstimator contract (e.g.
assertInstanceOf(ConversionCostEstimator::class, $service)); optionally also
assert it is the expected concrete class if you have a known implementation
class to validate.
🪄 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: 8a99d59d-070e-411b-8139-e2f610422855
📒 Files selected for processing (35)
app/Actions/Conversions/CreateConversionJobAction.phpapp/Actions/Conversions/EstimateConversionCostAction.phpapp/Contracts/Billing/ConversionCostEstimator.phpapp/Data/Credits/CreditCost.phpapp/Data/Credits/CreditCostBreakdown.phpapp/Enums/ConversionCreditChargeStatus.phpapp/Exceptions/Billing/UnsupportedConversionCostException.phpapp/Jobs/ProcessConversionJob.phpapp/Livewire/Dashboard/DashboardConverter.phpapp/Models/ConversionCreditCharge.phpapp/Models/ConversionJob.phpapp/Providers/AppServiceProvider.phpapp/Services/Billing/ConfigDrivenConversionCostEstimator.phpconfig/conversion_costs.phpdatabase/factories/ConversionCreditChargeFactory.phpdatabase/factories/ConversionJobFactory.phpdatabase/factories/FileRecordFactory.phpdatabase/migrations/2026_06_02_123052_create_conversion_credit_charges_table.phpresources/views/livewire/dashboard/dashboard-converter.blade.phptests/Fakes/Converters/FakeUnsupportedConverter.phptests/Feature/Actions/CreateConversionJobActionCreditTest.phptests/Feature/Actions/EstimateConversionCostActionTest.phptests/Feature/Conversion/ProcessConversionJobRealDriverTest.phptests/Feature/ConversionFlowTest.phptests/Feature/Jobs/ProcessConversionJobCreditTest.phptests/Feature/Jobs/ProcessConversionJobTest.phptests/Feature/Livewire/DashboardConverterCostTest.phptests/Feature/Livewire/DashboardConverterSettingsStepTest.phptests/Feature/Models/ConversionCreditChargeTest.phptests/Unit/Config/ConversionCostsConfigTest.phptests/Unit/Contracts/ConversionCostEstimatorContractTest.phptests/Unit/Data/CreditCostBreakdownTest.phptests/Unit/Data/CreditCostTest.phptests/Unit/Services/ConversionCostEstimatorResolutionTest.phptests/Unit/Services/ConversionCostEstimatorTest.php
Comment on lines
+197
to
+206
| try { | ||
| $cost = app(EstimateConversionCostAction::class)->handle( | ||
| $this->currentFile, | ||
| $converter, | ||
| $this->options, | ||
| ); | ||
| $this->estimatedCreditCost = $cost->amount; | ||
| } catch (Throwable) { | ||
| $this->estimatedCreditCost = null; | ||
| } |
There was a problem hiding this comment.
Don't swallow every estimation failure and continue to settings.
Line 204 catches all Throwable, clears the estimate, and still moves the user to settings. If pricing is unsupported or the config is broken, the UI presents a normal conversion flow and only fails later with a generic error or ? credits. Catch the expected billing exception(s) here and surface a targeted state instead; let unexpected failures bubble. As per coding guidelines, "Controllers and Livewire components must stay thin - business logic goes into app/Actions".
Proposed fix
+use App\Exceptions\Billing\UnsupportedConversionCostException;
+
try {
$cost = app(EstimateConversionCostAction::class)->handle(
$this->currentFile,
$converter,
$this->options,
);
$this->estimatedCreditCost = $cost->amount;
- } catch (Throwable) {
+ } catch (UnsupportedConversionCostException) {
$this->estimatedCreditCost = null;
+ $this->targetFormatError = 'We could not estimate credits for this conversion yet.';
+ $this->step = 'format';
+
+ return;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| try { | |
| $cost = app(EstimateConversionCostAction::class)->handle( | |
| $this->currentFile, | |
| $converter, | |
| $this->options, | |
| ); | |
| $this->estimatedCreditCost = $cost->amount; | |
| } catch (Throwable) { | |
| $this->estimatedCreditCost = null; | |
| } | |
| use App\Exceptions\Billing\UnsupportedConversionCostException; | |
| try { | |
| $cost = app(EstimateConversionCostAction::class)->handle( | |
| $this->currentFile, | |
| $converter, | |
| $this->options, | |
| ); | |
| $this->estimatedCreditCost = $cost->amount; | |
| } catch (UnsupportedConversionCostException) { | |
| $this->estimatedCreditCost = null; | |
| $this->targetFormatError = 'We could not estimate credits for this conversion yet.'; | |
| $this->step = 'format'; | |
| return; | |
| } |
🤖 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 `@app/Livewire/Dashboard/DashboardConverter.php` around lines 197 - 206,
Replace the broad catch(Throwable) in the DashboardConverter Livewire flow
around the EstimateConversionCostAction::handle call with specific
billing/pricing exceptions (e.g., BillingException, PricingUnsupportedException
or whatever domain exceptions your app defines) so only expected pricing
failures are handled; when caught, set a targeted state (for example set
$this->estimatedCreditCost = null and a new flag like $this->pricingUnsupported
= true or $this->pricingErrorMessage) so the UI can render a proper pricing
error, and allow any other unexpected exceptions to bubble up (rethrow) rather
than being swallowed; keep the rest of the call using $this->currentFile,
$converter and $this->options unchanged.
Comment on lines
+14
to
+23
| $table->foreignId('conversion_job_id')->nullable()->constrained()->nullOnDelete(); | ||
| $table->unsignedInteger('estimated_amount'); | ||
| $table->unsignedInteger('captured_amount')->default(0); | ||
| $table->unsignedInteger('refunded_amount')->default(0); | ||
| $table->string('status'); | ||
| $table->json('breakdown_json')->nullable(); | ||
| $table->timestamps(); | ||
|
|
||
| $table->index(['user_id', 'status']); | ||
| $table->index('conversion_job_id'); |
There was a problem hiding this comment.
Enforce one charge row per conversion job.
ConversionJob::creditCharge() is modeled as HasOne, but this table only adds a non-unique index on conversion_job_id. That lets multiple charge rows point at the same job, so capture/failure reads can pick the wrong record and drift billing state. Make conversion_job_id unique instead of plain indexed.
Suggested schema fix
- $table->foreignId('conversion_job_id')->nullable()->constrained()->nullOnDelete();
+ $table->foreignId('conversion_job_id')->nullable()->unique()->constrained()->nullOnDelete();
...
- $table->index('conversion_job_id');📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| $table->foreignId('conversion_job_id')->nullable()->constrained()->nullOnDelete(); | |
| $table->unsignedInteger('estimated_amount'); | |
| $table->unsignedInteger('captured_amount')->default(0); | |
| $table->unsignedInteger('refunded_amount')->default(0); | |
| $table->string('status'); | |
| $table->json('breakdown_json')->nullable(); | |
| $table->timestamps(); | |
| $table->index(['user_id', 'status']); | |
| $table->index('conversion_job_id'); | |
| $table->foreignId('conversion_job_id')->nullable()->unique()->constrained()->nullOnDelete(); | |
| $table->unsignedInteger('estimated_amount'); | |
| $table->unsignedInteger('captured_amount')->default(0); | |
| $table->unsignedInteger('refunded_amount')->default(0); | |
| $table->string('status'); | |
| $table->json('breakdown_json')->nullable(); | |
| $table->timestamps(); | |
| $table->index(['user_id', 'status']); |
🤖 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
`@database/migrations/2026_06_02_123052_create_conversion_credit_charges_table.php`
around lines 14 - 23, The migration creates a non-unique index on
conversion_job_id but ConversionJob::creditCharge() expects a one-to-one
relation; change the schema so conversion_job_id is unique (either add
->unique() on the conversion_job_id column or replace the index with a unique
constraint) so only one charge row can reference a given conversion_job_id;
update the migration that defines the conversion_job_id column (and remove the
plain ->index('conversion_job_id')) to enforce uniqueness.
- Add integer casts for amount fields in ConversionCreditCharge to prevent TypeError - Validate CreditCostBreakdown total consistency (must equal base + size + features) - Fail fast in ConfigDrivenConversionCostEstimator when pricing rule is missing or non-numeric - Add unique constraint migration for conversion_credit_charges.conversion_job_id - Fix P1: isolate captureCredits so a billing failure cannot override a successful job status - Make captureCredits idempotent via lockForUpdate + status pre-check - Wrap ConversionJob + ConversionCreditCharge creation in DB::transaction - Replace broad Throwable catch in DashboardConverter with specific billing exceptions Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This was referenced Jun 2, 2026
Merged
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
1 participant
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.
Phase 15 — Conversion Cost Estimator (CONV-215–CONV-235)
Summary
This release adds the conversion cost estimation system and credit enforcement to the FileConverter application.
What's new
config/conversion_costs.phpTest coverage
composer lintpassesnpm run buildpasses🤖 Generated with Claude Code
Summary by cubic
Adds a config‑driven conversion cost estimator with enforced credit checks and post‑success capture. Shows real costs in settings, blocks low‑balance jobs, and hardens billing with idempotent capture and transaction‑safe job/charge creation (CONV-215–CONV-235).
New Features
config/conversion_costs.php(image→image = 1 credit, image→PDF = 2 credits).EstimateConversionCostActionfor app/UI use.ConversionCreditChargewith breakdown.lockForUpdate+ status check); job + charge creation wrapped in a DB transaction; strict validation for missing/non‑numeric pricing rules.Migration
conversion_credit_chargesand the uniqueconversion_job_idconstraint.config/conversion_costs.phpgroups/pricing as needed; uses the existingCreditLedger.Written for commit 89413c3. Summary will update on new commits.
Summary by CodeRabbit
Release Notes