simplify the run job flow#548
Conversation
dnsi0
requested review from
alexcos20,
andreip136,
bogdanfazakas,
giurgiur99,
mihaisc and
ndrpp
as code owners
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
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:
📝 WalkthroughWalkthroughThe PR updates run-job resource selection to use GPU amounts with derived CPU, RAM, and disk values, carries GPU quantities through hydration and summary rendering, and replaces the split payment steps with a unified pay-session flow and single payment screen. ChangesGPU-based resource flow and unified payment session
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 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 `@src/components/run-job/select-resources.tsx`:
- Around line 127-135: `maxSelectableUnits` in `select-resources.tsx` only
accounts for GPU availability, but the submitted bundle also depends on
`derivedCpu`, `derivedRam`, and `derivedDisk`; update the selection cap so it is
limited by all four resources. Compute the per-unit share using the existing
`perUnitCpu`, `perUnitRam`, and `perUnitDisk` values, then make
`maxSelectableUnits` reflect the minimum units supported by GPU, CPU, RAM, and
disk capacity; apply the same logic in the later duplicated block around the
derived resource calculations so `handleSubmit` can’t send inconsistent
overcommitted GPU unit counts.
In `@src/components/run-job/summary.tsx`:
- Around line 212-214: The GPU label text is being prefixed with the selected
count before being passed into GpuLabel, which prevents vendor detection from
matching the start of the string. Update the rendering in the
selectedResources.gpus.map block in summary.tsx so GpuLabel receives the raw
gpu.description value (and keep the count separate if needed elsewhere),
ensuring the component can correctly pick the vendor icon.
In `@src/context/run-job-context.tsx`:
- Around line 27-32: The GPU count parsing in parseGpuCount is allowing
fractional positive values to be floored to 0, which can produce an invalid zero
GPU selection during hydration. Update parseGpuCount in run-job-context so it
only returns a positive integer when gpuTypeCount is greater than 0, and
otherwise falls back to 0; ensure any parsed value is clamped to at least 1
before returning so the result never becomes 0 for a valid GPU request.
In `@src/lib/use-pay-session.ts`:
- Around line 71-87: In use-pay-session’s EOA flow, a successful deposit can
happen before authorizeTokensEoa fails, leaving escrowBalance stale because
onSuccess is skipped. Update the EOA branch to refresh the escrow/payment state
immediately after depositTokensEoa.wait() succeeds (or otherwise persist the new
escrow balance) before calling authorizeTokensEoa, so a rejected authorization
does not allow a retry to deposit the same amount again. Keep the fix localized
to the existing EOA path inside use-pay-session.
- Around line 142-150: The success UI and analytics in usePaySession are
currently triggered immediately after sendTransaction(calls) without checking
the returned final call status. Update the usePaySession flow to capture the
result from sendTransaction, inspect that status, and only run toast.success,
onSuccess, and the PostHog capture calls when the batch reaches a successful
terminal state.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: cae10514-410e-45d1-a6ba-879d4e87f1aa
📒 Files selected for processing (11)
src/components/run-job/payment-authorize.tsxsrc/components/run-job/payment-deposit.module.csssrc/components/run-job/payment-deposit.tsxsrc/components/run-job/payment-page.tsxsrc/components/run-job/payment.tsxsrc/components/run-job/select-resources.module.csssrc/components/run-job/select-resources.tsxsrc/components/run-job/summary.tsxsrc/context/run-job-context.tsxsrc/lib/use-pay-session.tssrc/types/environments.ts
💤 Files with no reviewable changes (4)
- src/components/run-job/payment-deposit.tsx
- src/components/run-job/payment-deposit.module.css
- src/components/run-job/payment-authorize.tsx
- src/components/run-job/payment-page.tsx
| if (user?.type === 'eoa') { | ||
| // EOA path: no batching, run the deposit (approve + deposit) then the authorize sequentially. | ||
| if (!ocean) throw new Error('Wallet not ready'); | ||
| if (needsDeposit) { | ||
| const depositTx = await ocean.depositTokensEoa({ tokenAddress, amount: depositAmount! }); | ||
| await depositTx.wait(); | ||
| posthog.capture('payment_deposit', { tokenAddress, amount: depositAmount }); | ||
| } | ||
| const authorizeTx = await ocean.authorizeTokensEoa({ | ||
| tokenAddress, | ||
| spender, | ||
| maxLockedAmount, | ||
| maxLockSeconds, | ||
| maxLockCount, | ||
| }); | ||
| await authorizeTx.wait(); | ||
| posthog.capture('payment_authorize'); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Refresh escrow state after a successful EOA deposit if authorization fails.
Line 75 can permanently move funds into escrow before Lines 79-86 authorize. If authorization then fails or is rejected, the catch path skips onSuccess, so the Payment screen keeps the old escrowBalance and a retry can deposit the same amount again.
Proposed fix
+ let eoaDepositCompleted = false;
try {
if (user?.type === 'eoa') {
// EOA path: no batching, run the deposit (approve + deposit) then the authorize sequentially.
if (!ocean) throw new Error('Wallet not ready');
if (needsDeposit) {
const depositTx = await ocean.depositTokensEoa({ tokenAddress, amount: depositAmount! });
await depositTx.wait();
+ eoaDepositCompleted = true;
+ await onSuccess?.();
posthog.capture('payment_deposit', { tokenAddress, amount: depositAmount });
}
const authorizeTx = await ocean.authorizeTokensEoa({
tokenAddress,
spender,
@@
toast.success(paySuccessMessage(spender, peerId));
onSuccess?.();
} catch (err) {
+ if (eoaDepositCompleted) await onSuccess?.();
console.error('Pay session error:', err);🤖 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 `@src/lib/use-pay-session.ts` around lines 71 - 87, In use-pay-session’s EOA
flow, a successful deposit can happen before authorizeTokensEoa fails, leaving
escrowBalance stale because onSuccess is skipped. Update the EOA branch to
refresh the escrow/payment state immediately after depositTokensEoa.wait()
succeeds (or otherwise persist the new escrow balance) before calling
authorizeTokensEoa, so a rejected authorization does not allow a retry to
deposit the same amount again. Keep the fix localized to the existing EOA path
inside use-pay-session.
| await sendTransaction(calls); | ||
|
|
||
| if (needsDeposit) { | ||
| posthog.capture('payment_deposit', { tokenAddress, amount: depositAmount }); | ||
| } | ||
| posthog.capture('payment_authorize'); | ||
| } | ||
|
|
||
| toast.success(paySuccessMessage(spender, peerId)); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Inspect the local sendTransaction wrapper and call-status handling patterns.
rg -n -C4 'waitForCallsStatus|sendTransaction\\(|sendCalls\\(' src/lib src/componentsRepository: oceanprotocol/nodes-dashboard
Length of output: 318
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate the relevant implementation and surrounding call-status handling.
git ls-files 'src/lib/use-pay-session.ts' 'src/lib/*' 'src/components/*' | sed -n '1,120p'
echo '---'
rg -n -C4 -F 'sendTransaction(' src/lib src/components
echo '---'
rg -n -C4 -F 'waitForCallsStatus' src/lib src/components
echo '---'
rg -n -C4 -F 'capture(' src/lib/use-pay-session.tsRepository: oceanprotocol/nodes-dashboard
Length of output: 14756
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Read the relevant file in a bounded way.
wc -l src/lib/use-pay-session.ts
echo '---'
cat -n src/lib/use-pay-session.ts | sed -n '1,240p'Repository: oceanprotocol/nodes-dashboard
Length of output: 7995
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the Alchemy wrapper to see whether failed statuses throw or resolve.
cat -n src/lib/use-alchemy-client.ts | sed -n '1,180p'
echo '---'
rg -n -C3 'waitForCallsStatus|status|failed|success|executionState|userOp' src/lib/use-alchemy-client.ts src/lib src/componentsRepository: oceanprotocol/nodes-dashboard
Length of output: 50385
🌐 Web query:
Alchemy wallet-apis waitForCallsStatus failed status throws resolves status contract
💡 Result:
In the context of the Alchemy SDK and standard Ethereum provider patterns (such as ethers.js), the waitForTransaction method is designed to resolve once a transaction has been mined and included in a block [1][2][3]. It is important to distinguish between the transaction being "mined" and the transaction being "successful" [4]. 1. Method Behavior: The waitForTransaction method waits for the transaction to be included in the blockchain with a specified number of block confirmations [1][5][6]. Once it is mined, it returns the transaction receipt [2][3]. It does not inherently throw or reject if the on-chain execution of the contract call fails (i.e., reverts) [4][7]. 2. Checking Status: The transaction receipt returned by waitForTransaction contains a status field [4]. To determine if your contract call succeeded or failed, you must manually inspect this status property [4][7]: - status: 0x1 indicates the transaction succeeded [4][7]. - status: 0x0 indicates the transaction failed or reverted [4][7]. 3. Handling Reverts: Because waitForTransaction resolves regardless of the transaction status (as long as it was successfully mined/included in a block), you must implement your own logic to check for failure [4][7]. If you rely on the promise resolution alone, your code may proceed as if the operation succeeded even if the contract call reverted [4][7]. Example pattern for checking status: const receipt = await alchemy.core.waitForTransaction(txHash); if (receipt.status === 0 || receipt.status === '0x0') { throw new Error('Transaction reverted on-chain'); } else { console.log('Transaction successful'); } If you are experiencing issues where waitForTransaction seems to "fail" (e.g., timing out), this is typically related to network latency or the confirmation threshold not being met within the provided timeout window, rather than the on-chain status of the contract call itself [8]. Always ensure your RPC connection is stable and consider increasing the timeout if you are waiting for a high number of confirmations [1][5][6].
Citations:
- 1: https://github.com/alchemyplatform/alchemy-sdk-js/blob/main/docs-md/classes/CoreNamespace.md
- 2: https://github.com/alchemyplatform/alchemy-sdk-js
- 3: https://github.com/alchemyplatform/alchemy-sdk-js/blob/main/docs-md/classes/TransactNamespace.md
- 4: https://www.alchemy.com/docs/ethereum-transactions-pending-mined-dropped-replaced
- 5: https://github.com/alchemyplatform/alchemy-sdk-js/blob/main/docs-md/classes/AlchemyProvider.md
- 6: https://github.com/alchemyplatform/alchemy-sdk-js/blob/main/docs-md/classes/AlchemyWebSocketProvider.md
- 7: https://www.alchemy.com/docs/how-to-check-the-status-of-a-transaction-using-its-hash
- 8: WaitForTransaction() incorrectly times out (the transaction was immediately mined) ethers-io/ethers.js#1479
🌐 Web query:
site:alchemy.com waitForCallsStatus wallet-apis calls status returned object success failed
💡 Result:
The waitForCallsStatus function, part of the Alchemy Wallet APIs, is used to poll the status of a call (or bundle of calls) until it reaches a terminal state [1][2]. When called, it returns a promise that resolves to the final status object, typically representing the result of a getCallsStatus operation [1][2]. While the specific object structure can vary by chain, it generally contains a status field that reflects whether the operation succeeded or failed [3][2][4]. For EVM-compatible chains, the status is often indicated by numeric codes or descriptive strings where terminal states like success (e.g., status 200) or failure (e.g., status 400 for offchain failure) are returned [2][4]. The function is designed to handle this polling automatically, including retry logic and exponential backoff, until the terminal state is reached or the timeout is exceeded [1][2][5]. In practice, developers often check the status field of the returned object to confirm the outcome [3][6][2]. For example, after awaiting waitForCallsStatus, the result object will contain the finalized status, which can be checked against known success or failure criteria [2].
Citations:
- 1: https://www.alchemy.com/docs/wallets/reference/wallet-apis/solana/functions/waitForCallsStatus
- 2: https://www.alchemy.com/docs/wallets/reference/wallet-apis/functions/sendCalls.md
- 3: https://www.alchemy.com/docs/wallets/quickstart
- 4: https://www.alchemy.com/docs/wallets/api-reference/smart-wallets/wallet-api-endpoints/wallet-get-calls-status
- 5: https://www.alchemy.com/docs/wallets/reference/account-kit/react/hooks/useWaitForCallsStatus.md
- 6: https://www.alchemy.com/docs/wallets/transactions/send-transactions
🌐 Web query:
"waitForCallsStatus" Alchemy wallet-apis status success failed
💡 Result:
The waitForCallsStatus method in the Alchemy Wallet APIs is a client-side utility function designed to poll the wallet_getCallsStatus endpoint until a transaction or batch of calls reaches a terminal state [1][2]. It is commonly used in the Alchemy Account Kit SDK and viem-based wallet implementations to simplify transaction monitoring [2][3][4]. Key details regarding its behavior: Functionality: It polls the status of a specific call ID [1][2]. It is essentially a wrapper around viem's waitForCallsStatus action [1][2], offering configurable options such as polling intervals, exponential backoff, status predicates, timeout settings, and a flag to throw an error if the transaction fails [1]. Terminal States: The function waits until the call status leaves the "pending" state (typically denoted by 1xx status codes in the underlying JSON-RPC API) [5][6]. Once the status moves to a terminal state (such as "confirmed" or a failed state), the function resolves and returns the final status result [1][2][4]. Usage: In the SDK, you typically call it after initiating a transaction [7][5][8]: const result = await client.waitForCallsStatus({ id: "0x1234..." }); console.log(result.status); // The final resolved status For React applications, there is also a corresponding hook, useWaitForCallsStatus, which manages this polling state automatically within component lifecycles [4]. If you are using the raw JSON-RPC API directly rather than the SDK, you must manually implement the polling loop by calling wallet_getCallsStatus until you receive a non-pending status [5][6]. The SDK's waitForCallsStatus method automates this logic for you [2][4].
Citations:
- 1: https://www.alchemy.com/docs/wallets/reference/wallet-apis/solana/functions/waitForCallsStatus
- 2: https://github.com/alchemyplatform/aa-sdk/blob/main/account-kit/wallet-client/src/client/actions/waitForCallsStatus.ts
- 3: https://www.alchemy.com/docs/wallets/reference/wallet-apis/functions/smartWalletActions
- 4: https://www.alchemy.com/docs/wallets/reference/account-kit/react/hooks/useWaitForCallsStatus
- 5: https://www.alchemy.com/docs/wallets/transactions/send-transactions.md
- 6: https://www.alchemy.com/docs/wallets/transactions/using-api
- 7: https://www.alchemy.com/docs/wallets/quickstart
- 8: https://www.alchemy.com/docs/wallets/transactions/send-batch-transactions
🌐 Web query:
Alchemy wallet-apis waitForCallsStatus documentation status field
💡 Result:
In Alchemy's Wallet APIs SDK, the waitForCallsStatus function is an asynchronous action that polls the getCallsStatus endpoint until a terminal state is reached [1][2][3]. Regarding the status field in the returned result: 1. EVM-based Chains: For standard EVM operations, the result typically includes a status field that represents the state of the call bundle [4][5]. While specific type definitions may vary by SDK version, the result generally resolves to a structure containing the final status (often strings like "success", "failure", or "pending" in simplified SDK wrappers) [6][3][5], alongside detailed receipt information [7][8][9]. 2. Solana: The Solana-specific implementation of waitForCallsStatus explicitly returns a structure where the status field is normalized to "success", "pending", or "failure" [6]. Additionally, the result includes a statusCode field (a number) that provides more granular information, such as specific terminal or pending state indicators (e.g., 200 for success, 1xx for pending) [6][2]. When using the SDK, you can access the status directly from the returned object [4][5]: const status = await client.waitForCallsStatus({ id }); console.log(status.status); // e.g., "success" or "failure" If you are interacting with the underlying REST API (wallet_getCallsStatus), the status is typically returned as a numeric status code (e.g., 200) rather than a string [8]. The SDK's waitForCallsStatus method abstracts this by polling and waiting until these numeric codes transition to a terminal state, returning a more developer-friendly object [1][2][3].
Citations:
- 1: https://www.alchemy.com/docs/wallets/reference/wallet-apis/solana/functions/waitForCallsStatus
- 2: https://github.com/alchemyplatform/aa-sdk/blob/main/packages/wallet-apis/src/actions/solana/waitForCallsStatus.ts
- 3: https://www.alchemy.com/docs/wallets/reference/wallet-apis/functions/sendCalls.md
- 4: https://www.alchemy.com/docs/wallets/quickstart.md
- 5: https://www.alchemy.com/docs/wallets/quickstart
- 6: https://www.alchemy.com/docs/wallets/reference/wallet-apis/solana/type-aliases/SolanaWaitForCallsStatusResult
- 7: https://github.com/alchemyplatform/aa-sdk/blob/main/account-kit/wallet-client/src/client/actions/waitForCallsStatus.ts
- 8: https://www.alchemy.com/docs/wallets/api-reference/smart-wallets/wallet-api-endpoints/wallet-get-calls-status
- 9: https://www.alchemy.com/docs/reference/smart-wallet-quickstart
Gate success UI on the returned call status
sendTransaction(calls) returns the final calls status object, so toast.success(...), onSuccess?.(), and the PostHog events can fire even when the batch ends in a failed terminal state. Check the returned status and only treat it as success when the batch actually succeeds.
🤖 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 `@src/lib/use-pay-session.ts` around lines 142 - 150, The success UI and
analytics in usePaySession are currently triggered immediately after
sendTransaction(calls) without checking the returned final call status. Update
the usePaySession flow to capture the result from sendTransaction, inspect that
status, and only run toast.success, onSuccess, and the PostHog capture calls
when the batch reaches a successful terminal state.
|
Caution Failed to replace (edit) comment. This is likely due to insufficient permissions or the comment being deleted. Error details |
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 `@src/types/environments.ts`:
- Around line 77-81: The SelectedGpu type still allows amount to be omitted,
which lets stale call sites compile and fall back to an implicit default later.
Update the SelectedGpu definition in environments.ts to make amount required,
and then fix any usages of SelectedGpu or related GPU-selection builders so they
always provide an explicit amount before reaching the run-job context.
In `@src/utils/resources.ts`:
- Around line 34-51: The distributeGpus helper is re-reading the same original
capacity for repeated GPU ids, so duplicate entries can be allocated more than
once; update distributeGpus in resources.ts to track and decrement remaining
availability per gpu.id as you iterate. Use the existing distributeGpus loop and
the available/getAvailableAmount lookup to maintain a per-id remaining capacity
map, so repeated entries consume from the updated balance instead of the initial
amount.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 2f17dea7-0c66-4949-8556-e8ef885e89bb
📒 Files selected for processing (5)
src/components/run-job/select-resources.tsxsrc/components/run-job/summary.tsxsrc/context/run-job-context.tsxsrc/types/environments.tssrc/utils/resources.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- src/components/run-job/summary.tsx
- src/context/run-job-context.tsx
- src/components/run-job/select-resources.tsx
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/run-job/select-resources.tsx (1)
170-180: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winClear stale GPU query params when no GPU is selected.
Because
router.queryis spread first, deselecting all GPUs leaves any previousgpus,gpus[], orgpuCountin the URL. A reload/share of that URL can rehydrate GPUs the user removed.Proposed fix
- const query = { - ...router.query, + const query = { ...router.query }; + delete query.gpus; + delete query['gpus[]']; + delete query.gpuCount; + Object.assign(query, { cpu: derivedCpu, ram: derivedRam, disk: derivedDisk, - // Each selected GPU is one unit; encode `id:1` so the selection round-trips losslessly and - // the hydration clamps it to current availability. - ...(values.gpus.length > 0 && { - gpus: values.gpus.map((id) => `${id}:1`), - gpuCount: values.gpus.length, - }), maxJobDuration: values.maxJobDurationSeconds, - }; + }); + if (values.gpus.length > 0) { + Object.assign(query, { + gpus: values.gpus.map((id) => `${id}:1`), + gpuCount: values.gpus.length, + }); + }🤖 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 `@src/components/run-job/select-resources.tsx` around lines 170 - 180, The GPU query builder in select-resources.tsx is preserving stale selection state because it spreads router.query before conditionally adding gpus and gpuCount; when values.gpus is empty, old GPU params remain in the URL. Update the query construction in the relevant selection handler to explicitly remove any existing gpus, gpus[], and gpuCount entries whenever no GPUs are selected, while keeping the current round-trip behavior when GPUs are present. Use the existing derivedCpu/derivedRam/derivedDisk and values.gpus logic as the place to apply the cleanup so the URL always matches the current selection.
♻️ Duplicate comments (2)
src/components/run-job/select-resources.tsx (1)
192-226: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winDon’t clamp derived resources below the selected GPU units’ share.
Current code still allows selecting GPUs while silently reducing CPU/RAM/disk to availability. If availability is below the proportional share but above the resource minimum, the submitted bundle under-allocates and underestimates cost for the selected GPU units.
Proposed fix
- // Derived resource amounts for the chosen number of units, clamped to what's actually available. - const derivedCpu = clamp(Math.round(perUnitCpu * unitCount), cpu?.min ?? 0, Math.max(0, cpuAvailable)); - const derivedRam = clamp(Math.round(perUnitRam * unitCount), ram?.min ?? 0, Math.max(0, ramAvailable)); - const derivedDisk = clamp(Math.round(perUnitDisk * unitCount), disk?.min ?? 0, Math.max(0, diskAvailable)); + // Requested resource amounts for the chosen number of units. + const derivedCpu = Math.max(cpu?.min ?? 0, Math.round(perUnitCpu * unitCount)); + const derivedRam = Math.max(ram?.min ?? 0, Math.round(perUnitRam * unitCount)); + const derivedDisk = Math.max(disk?.min ?? 0, Math.round(perUnitDisk * unitCount)); const resourcesExhausted = gpuExhausted || - (!!cpu && cpuAvailable < (cpu.min ?? 0)) || - (!!ram && ramAvailable < (ram.min ?? 0)) || - (!!disk && diskAvailable < (disk.min ?? 0)); + (!!cpu && cpuAvailable < derivedCpu) || + (!!ram && ramAvailable < derivedRam) || + (!!disk && diskAvailable < derivedDisk);🤖 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 `@src/components/run-job/select-resources.tsx` around lines 192 - 226, The derived resource calculation in select-resources.tsx is clamping CPU/RAM/disk down to availability even when GPU units are selected, which can under-allocate the bundle and misstate cost. Update the resource derivation around unitCount, selectedGpuEntries, and derivedCpu/derivedRam/derivedDisk so GPU selections cannot be submitted unless the proportional per-unit share is fully available; if availability is below the selected share, surface it as exhausted/invalid instead of silently clamping below the selected GPUs’ requirement.src/components/run-job/summary.tsx (1)
214-220: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winKeep the count outside
GpuLabel.
GpuLabeldetects vendor icons from the start of the GPU string, so passing"2 × NVIDIA..."falls back to the generic icon.Proposed fix
- ).map(([desc, count]) => <GpuLabel key={desc} gpu={`${count} × ${desc}`} />) + ).map(([desc, count]) => ( + <div key={desc}> + {count} × <GpuLabel gpu={desc} /> + </div> + ))🤖 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 `@src/components/run-job/summary.tsx` around lines 214 - 220, The GPU summary rendering in the selectedResources.gpus map is prefixing the count onto the value passed to GpuLabel, which prevents vendor detection from matching the GPU string start. Update the summary logic around the GpuLabel usage so the count is rendered separately from the GPU label text, and pass only the raw GPU description/id to GpuLabel while keeping the count in adjacent UI text.
🤖 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.
Outside diff comments:
In `@src/components/run-job/select-resources.tsx`:
- Around line 170-180: The GPU query builder in select-resources.tsx is
preserving stale selection state because it spreads router.query before
conditionally adding gpus and gpuCount; when values.gpus is empty, old GPU
params remain in the URL. Update the query construction in the relevant
selection handler to explicitly remove any existing gpus, gpus[], and gpuCount
entries whenever no GPUs are selected, while keeping the current round-trip
behavior when GPUs are present. Use the existing
derivedCpu/derivedRam/derivedDisk and values.gpus logic as the place to apply
the cleanup so the URL always matches the current selection.
---
Duplicate comments:
In `@src/components/run-job/select-resources.tsx`:
- Around line 192-226: The derived resource calculation in select-resources.tsx
is clamping CPU/RAM/disk down to availability even when GPU units are selected,
which can under-allocate the bundle and misstate cost. Update the resource
derivation around unitCount, selectedGpuEntries, and
derivedCpu/derivedRam/derivedDisk so GPU selections cannot be submitted unless
the proportional per-unit share is fully available; if availability is below the
selected share, surface it as exhausted/invalid instead of silently clamping
below the selected GPUs’ requirement.
In `@src/components/run-job/summary.tsx`:
- Around line 214-220: The GPU summary rendering in the selectedResources.gpus
map is prefixing the count onto the value passed to GpuLabel, which prevents
vendor detection from matching the GPU string start. Update the summary logic
around the GpuLabel usage so the count is rendered separately from the GPU label
text, and pass only the raw GPU description/id to GpuLabel while keeping the
count in adjacent UI text.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: bcdf1dbd-3084-43fe-8ddb-90afdda82ddf
📒 Files selected for processing (2)
src/components/run-job/select-resources.tsxsrc/components/run-job/summary.tsx
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/context/run-job-context.tsx (1)
320-354: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftRecompute CPU/RAM/disk from hydrated GPU totals here.
This layer now treats GPU quantity as the source of truth, but hydration still trusts
cpu,ram, anddiskfrom the URL. A stale or edited link can therefore hydrategpuCount=1together with arbitrary CPU/RAM/disk values, and that inconsistent bundle is what gets passed intofetchEstimatedCost.Please derive those three resources from
gpuCountwith the same helper path used bysrc/components/run-job/select-resources.tsx, and only fall back to query values if you intentionally need backward compatibility for older links.🤖 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 `@src/context/run-job-context.tsx` around lines 320 - 354, Hydration in run-job-context.tsx is still trusting cpu/ram/disk query params even though gpuCount is the source of truth, which can build inconsistent resources for fetchEstimatedCost. Update the resource hydration logic in the same path used by select-resources.tsx so CPU, RAM, and disk are recomputed from the hydrated gpuCount (and resolved GPU selection), and only use the URL values as an explicit backward-compatibility fallback. Keep the fix localized around the resource assembly block that builds the EnvResourcesSelection object.
🤖 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 `@src/context/run-job-context.tsx`:
- Around line 311-319: The GPU hydration logic in the run-job context is summing
duplicate `gpus` query entries after clamping each one independently, which can
overstate `gpuCount` and the cost estimate. Update the `resolvedGpus` flow in
`run-job-context.tsx` to aggregate entries by GPU id before applying
`clampToAvailable`, then produce one combined `SelectedGpu` per id so
`fetchEstimatedCost` receives the true total. Use the existing `queryGpus`,
`findGpuRes`, and `clampToAvailable` logic in that block to locate and refactor
the mapping/reduction path.
---
Outside diff comments:
In `@src/context/run-job-context.tsx`:
- Around line 320-354: Hydration in run-job-context.tsx is still trusting
cpu/ram/disk query params even though gpuCount is the source of truth, which can
build inconsistent resources for fetchEstimatedCost. Update the resource
hydration logic in the same path used by select-resources.tsx so CPU, RAM, and
disk are recomputed from the hydrated gpuCount (and resolved GPU selection), and
only use the URL values as an explicit backward-compatibility fallback. Keep the
fix localized around the resource assembly block that builds the
EnvResourcesSelection object.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: f1abe532-812b-4f79-889d-984567b5045a
📒 Files selected for processing (1)
src/context/run-job-context.tsx
1 participant
Fixes # .
Changes proposed in this PR:
Summary by CodeRabbit