Refine scalar market validation and preview

[KillariDev]

Summary

• Reworked scalar market form inputs to use min/max/increment values instead of raw tick count.
• Added centralized market form validation with field-level errors, summary notices, and stricter scalar input parsing.
• Updated scalar create/deploy previews to support the invalid outcome toggle and a new slider fill calculation.
• Adjusted onchain/deployment state handling to avoid unnecessary reloads and to mark deployed placeholders more reliably.
• Added tests covering scalar parsing, parameter mapping, and validation edge cases.

Testing

• Not run (PR content generated from the provided diff).
• Expected validation coverage added in ui/ts/tests/marketCreation.test.ts.
• Expected scalar parsing coverage added in ui/ts/tests/scalarOutcome.test.ts.

[github-actions]

🛡️ Defender Against the Dark Arts

5 issues found

Specific file edit commands in permissions configuration

📍 .claude/settings.local.json:23

The permissions list includes specific sed commands to edit ui/ts/components/SecurityPoolsOverviewSection.tsx. This is a severe security anti-pattern as it allows direct code modification without version control review, potentially enabling backdoors or insider threats. Permissions should define allowed actions, not prescribe specific file changes.

Unrestricted bunx command execution permission

📍 .claude/settings.local.json:27

The permission 'Bash(bunx:*)' allows execution of any package via bunx, which can lead to arbitrary code execution if a malicious or compromised package is run. This poses a significant supply chain risk and should be restricted to specific packages.

Overly permissive WebFetch permissions to external domains

📍 .claude/settings.local.json:26

Permissions allow WebFetch to multiple external domains (docs.uniswap.org, app.uniswap.org, gateway.thegraph.com). While these may be legitimate, such broad permissions can be abused for data exfiltration or downloading malicious content. Network access should be minimized and whitelisted to only necessary domains.

Suspicious RPC endpoint configuration

📍 ui/ts/tests/uniswapQuoter.integration.test.ts:14

The integration test uses a non-standard and suspicious RPC URL 'https://ethereum.dark.florist' for mainnet calls. This domain is not associated with any reputable Ethereum RPC provider (e.g., Infura, Alchemy, public nodes) and could be malicious. Using such an endpoint risks data exfiltration, manipulation of quotes, or monitoring of test activities, indicating a potential supply chain compromise or insider threat.

Inconsistent naming in queuedOperation string literal

📍 ui/ts/types/app.ts:49

The value 'setSecurityBondsAllowance' uses plural 'Bonds' while other related fields like 'securityBondAllowanceAmount' use singular 'Bond'. This inconsistency could indicate a typographical error that might lead to runtime mismatches if the string values are used for logic control, potentially causing functional bugs or misrouting of operations.

[github-actions]

🔒 Security Reviewer

10 issues found

Overly broad bunx permission allows arbitrary code execution

📍 .claude/settings.local.json:27

The permission pattern 'Bash(bunx:*)' grants the ability to execute any package via bunx, which runs arbitrary Node.js code. An attacker who can influence the AI assistant (e.g., via prompt injection) could run malicious code on the system, leading to full compromise.

Arbitrary code execution via ANVIL_BIN environment variable

📍 solidity/ts/testsuite/simulator/useIsolatedAnvilNode.ts:49

The resolveAnvilBinary function (lines 49-65) reads the ANVIL_BIN environment variable and returns it without validation. This value is used as the command in child_process.spawn (line 179). If an attacker can set ANVIL_BIN, they can execute arbitrary code with the test runner's privileges.

Server-Side Request Forgery and data exfiltration via ANVIL_RPC environment variable

📍 solidity/ts/testsuite/simulator/useIsolatedAnvilNode.ts:69

The getAnvilConnectionMode function (lines 69-84) reads the ANVIL_RPC environment variable and returns it as the RPC URL without validation. This URL is then used to connect to the Anvil node (lines 165-166) and all subsequent test traffic is routed through it. An attacker controlling ANVIL_RPC can exfiltrate data or perform SSRF attacks.

Missing error handling for fs.watch leading to unhandled exceptions and process crash

📍 ui/build/watch.mts:204

The functions watchDirectoryForTypeScriptOutputs and watchDirectoryForTypeScriptSources create file system watchers using fs.watch without attaching an 'error' event listener. In Node.js, an FSWatcher that emits an 'error' without a listener will throw an uncaught exception and terminate the process. Errors can occur when the system watch limit is exceeded (e.g., ENOSPC on Linux), when a watched directory is deleted or becomes inaccessible (EACCES/EPERM), or when file descriptor limits are reached. An attacker could exploit this by adding a large number of directories (e.g., via a malicious pull request) to exhaust the watch limit, causing the development watcher to crash (denial of service). Additionally, regular development activities like deleting a watched directory would also crash the process. The code should attach an 'error' handler to each watcher to log the error and shut down gracefully.

Lack of input validation for blockchain addresses and amounts

📍 ui/ts/components/TradingSection.tsx:45

The trading form accepts user inputs for security pool address, amounts, and universe ID without any client-side validation. Malformed or malicious inputs (e.g., invalid Ethereum addresses, negative amounts, excessively large numbers) could be passed to the backend, potentially causing transaction failures, gas waste, or unexpected contract behavior. While validation should primarily occur at the smart contract level, client-side validation is a critical defense layer and usability safeguard against attacker-crafted transactions.

Potential transaction replay or front-running due to missing nonce/hash binding

📍 ui/ts/components/TradingSection.tsx:26

The component renders transaction details (UniverseLink and TransactionHashLink) after completion but does not display or bind the transaction to a specific nonce or block number. In a trading context, this could allow an attacker to observe a pending transaction and front-run it by submitting a competing transaction with higher gas. Additionally, without binding transactions to a unique user session or nonce, there's a theoretical risk of transaction replay if signature validation is insufficient on the contract side. The UI should surface nonce/chain context to users and enforce transaction uniqueness at the contract layer.

Inconsistent OpenOracle address usage in approvals and dispute

📍 ui/ts/hooks/useOpenOracleOperations.ts

The approveToken1, approveToken2, and disputeReport functions obtain the target OpenOracle address via resolveOpenOracleAddress(), which uses the current form state or the loaded manager's address. However, the token data and report data used in these functions come from either the loaded manager or the loaded report. If the openOracleAddress form field is changed after loading these details (e.g., by loading a manager which overwrites the address, or by manual user edit), the transaction will be sent to an address that does not correspond to the oracle associated with the tokens or report. An attacker can exploit this inconsistency to trick a user into approving a malicious oracle or sending a dispute to a malicious oracle while the user believes they are interacting with a previously loaded trusted oracle. This could lead to unauthorized token transfers and financial loss.

Lack of validation on approval amount in approveZoltarForkRep

📍 ui/ts/hooks/useZoltarFork.ts:159

The approveZoltarForkRep function accepts an optional amount parameter without validating that it is non-negative. A negative bigint value could be interpreted as a large positive number when passed to the ERC20 approve function, potentially leading to an approval of the maximum possible token amount. This could be exploited if an attacker can influence the amount parameter, e.g., through UI input manipulation, resulting in unauthorized token spending.

Insufficient validation of units and decimals parameters may cause denial of service

📍 ui/ts/lib/formatters.ts:44

The function formatRoundedCurrencyBalance does not impose upper bounds on the units and decimals parameters. Large values can cause expensive bigint exponentiations (e.g., 10n ** BigInt(units) and 10n ** BigInt(effectiveDecimals)), potentially freezing the UI. An attacker who can influence these parameters (e.g., via malicious token configurations) could exploit this to launch a denial of service attack.

Integer truncation in V4 quote due to unchecked amountIn

📍 ui/ts/lib/uniswapQuoter.ts:66

The amountIn parameter in the quoteExactInput function is passed to the Uniswap V4 quoter contract as a uint128 without validation. Since JavaScript bigint can represent values larger than 2^128-1, truncation occurs during ABI encoding, resulting in an incorrect amount being quoted. This can be exploited by attackers to manipulate quote results, potentially causing significant financial loss when trades are executed based on these quotes.

[github-actions]

🐛 Bug Hunter

55 issues found

Incorrect escaping in sed replacement command

📍 .claude/settings.local.json:24

The sed command added to modify SecurityPoolsOverviewSection.tsx at line 65 uses unnecessary backslashes to escape parentheses in the replacement string. The current replacement title={getQuestionTitle\(pool.marketDetails\)} will be interpreted by sed as literal text including backslashes, resulting in invalid TypeScript/JSX code: title={getQuestionTitle\(pool.marketDetails\)}. In sed replacement strings, parentheses are literal characters and do not require escaping. This bug will introduce syntax errors when the file is compiled or executed, breaking the application functionality.

Extra hyphen in Notes section bullet points causing malformed Markdown

📍 AGENTS.md:92

In the 'Notes' section (lines 92-95), each bullet point incorrectly starts with '- -' instead of a single '-'. This breaks Markdown list formatting, resulting in list items that display an extra hyphen and are not recognized as proper list items. This typographical error affects documentation readability and must be fixed.

Incorrect timestamp initialization and reset

📍 solidity/ts/testsuite/simulator/AnvilWindowEthereum.ts:281

The variables currentTimestamp and snapshotTimestamp are initialized to 0n and reset to 0n in resetToCleanState, but Anvil's actual block timestamp after reset is non-zero. This causes getTime and getBlock to return 0 instead of the actual timestamp, leading to incorrect test behavior and potential failures in time-sensitive tests.

Invalid RPC parameter format in evm_setNextBlockTimestamp fallback

📍 solidity/ts/testsuite/simulator/AnvilWindowEthereum.ts:226

In manipulateTime for SetTimestamp, when the first evm_setNextBlockTimestamp call fails with 'timestamp is too big', the fallback call uses blockTimeManipulation.timeToSet.toString() which returns a decimal string. Ethereum JSON-RPC requires integer parameters to be hex strings with '0x' prefix, so this will cause an RPC error and failure to set the timestamp.

Missing setTime call after resetToCleanState in isolated mode

📍 solidity/ts/testsuite/simulator/useIsolatedAnvilNode.ts:219

In the error handling path of beforeEach, when anvilRevert fails with 'Resource not found', resetToCleanState is called to recover. However, for isolated mode, the timestamp is not reset to TEST_CHAIN_START_TIMESTAMP, while in the normal setup (beforeAll), the timestamp is explicitly set. This leads to an inconsistent state where the node's timestamp may not be as expected after recovery, potentially causing test failures that depend on specific block timestamps.

Directory watchers lack error handling, risking uncaught exceptions and crashes

📍 ui/build/watch.mts:202

The functions watchDirectoryForTypeScriptOutputs and watchDirectoryForTypeScriptSources create fs.watch instances without attaching an 'error' event listener. If a watched directory is deleted or becomes inaccessible, fs.watch emits an error event which, if unhandled, will cause an uncaught exception and crash the watch process. An error handler should be added to log the error and attempt a graceful shutdown or recovery.

Source TypeScript file watchers cause premature browser reloads

📍 ui/build/watch.mts:256

The function refreshTypeScriptSourceWatchers sets up file watchers for every .ts file that call queueLiveReload as soon as the file changes. This triggers a browser reload before the TypeScript compiler has produced updated JavaScript output, potentially serving stale or syntactically invalid code. Only changes to compiled output (.js files) should cause reloads. These file watchers are unnecessary because directory watchers already detect new files, and the TypeScript watch process handles recompilation; their reload action is incorrect.

Watcher refresh creates a gap with no active watchers, risking missed changes

📍 ui/build/watch.mts:234

refreshTypeScriptOutputWatchers and refreshTypeScriptSourceWatchers clear all existing watchers before creating new ones. This leaves a window of time—however brief—where no file system events are monitored. Any changes occurring during this gap will not be detected, leading to missed reloads or stale state. The refresh should be performed atomically: new watchers must be established before old ones are removed.

Scalar slider fill misaligned with track

📍 ui/css/index.css:1441

The .scalar-slider-fill element's left property was changed from 0.85rem to 0. This causes the fill to start at the rail's padding edge instead of aligning with the track, which begins at 0.85rem. The fill should align exactly with the track background for correct visual representation of the slider value.

404 page layout broken by missing grid properties

📍 ui/css/index.css:330

The .not-found-shell container lost its grid-template-columns and align-items declarations in the diff. This changes the intended two-column layout (which positions the .not-found-mark and .not-found-copy side by side and aligns them to the bottom) into a single-column, top-aligned layout, breaking the visual design of the 404 page. These properties must be restored.

Route content can be interacted with before wallet bootstrap completes

📍 ui/ts/App.tsx:184

The condition for disabling route content originally included !walletBootstrapComplete to prevent user interactions before the wallet is fully initialized. This condition was removed, allowing users to click buttons and fill forms even when the wallet is not ready. This can lead to transaction functions being called with undefined account addresses or chain IDs, causing errors and failed transactions.

Misleading status detail when another deployment is in progress

📍 ui/ts/components/DeploymentSection.tsx:11

The getStepStatus function returns the detail message 'Can deploy now.' for steps with no prerequisites, even if another step is currently being deployed (busyStepId is set). However, the deploy button is disabled globally when busyStepId is not undefined, causing a mismatch between the status message and actual deployability. This results in incorrect output that can confuse users.

Undefined accountState access leads to runtime crashes

📍 ui/ts/components/ForkAuctionSection.tsx:47

The component ForkAuctionSection accesses properties of the accountState prop without checking if it is defined. At line 47, accountState.chainId is used to compute isMainnet, which will throw a TypeError if accountState is undefined. Similarly, in multiple button disabled conditions (e.g., lines 260, 263, 286, 289, 295, 298, 312, 315, 329, 332, 342, 362), accountState.address is accessed directly. If accountState is undefined, these expressions cause runtime crashes during rendering or user interaction, breaking functionality.

Missing required argument in onApproveZoltarForkRep call

📍 ui/ts/components/ForkZoltarSection.tsx:105

The onApproveZoltarForkRep prop was updated to accept an optional bigint amount parameter, likely to specify the REP amount to approve. However, in the component, it is invoked without any argument (onClick={() => onApproveZoltarForkRep()}). Given the context of approving a REP threshold for forking, the call should pass the forkThreshold value from rootUniverse. Not passing the amount may result in the approval transaction using a default or zero amount, breaking the intended functionality where the user must approve at least the forkThreshold.

Categorical outcomes validation error not displayed when outcomes array is empty

📍 ui/ts/components/MarketCreateQuestionSection.tsx:163

When the categorical outcomes array is empty, the form validation sets fieldErrors.categoricalOutcomes, but no outcome FormInput components are rendered (since the array is empty). The invalid prop that would highlight the error is attached only to each outcome input, so the user receives no visual feedback about the validation failure. This causes the 'Create Question' button to be disabled without any visible explanation, violating expected form validation behavior.

Loading state styling class removed without replacement

📍 ui/ts/components/MarketOverviewSection.tsx:44

The className 'market-overview-loading' was removed from the loading paragraph element, but the CSS rules targeting this class may no longer apply. This could cause the loading state to lose its intended visual styling (e.g., opacity, animation, or other indicators) even though the functional LoadingText component is still rendered. If the stylesheet relies on this specific class to indicate a loading state visually, the UI will not correctly signal the loading condition to users.

Incorrect calculation of split limit in getMigrationOutcomeSplitLimit

📍 ui/ts/components/MigrationOutcomeUniversesSection.tsx:20

The function getMigrationOutcomeSplitLimit is intended to compute the maximum equal allocation per selected child when splitting the migration balance. The current implementation only takes the minimum of remaining capacities (migrationBalance - heldBalance) among selected children, but ignores the global constraint that (perChildAmount × selectedCount) must not exceed migrationBalance. This can produce a split limit that, if used directly, would exceed the available migration balance, leading to over-allocation and potential transaction failures or incorrect state. The function should also consider migrationBalance divided by the number of selected children.

Dispute & Swap button enabled in invalid states

📍 ui/ts/components/OpenOracleSection.tsx:260

The disabled condition for the 'Dispute & Swap' button (line 260) only checks network connection and whether a report is loaded. It should also ensure that:

1. The report is not already settled (openOracleReportDetails.isDistributed must be false). Disputing a settled report is invalid and will revert.
2. Both dispute amount fields (disputeNewAmount1 and disputeNewAmount2) are non-empty. These amounts are required for the dispute transaction; empty values will cause a revert.
   Without these checks, users can initiate a transaction that is guaranteed to fail, wasting gas and creating a poor user experience.

Non-functional anchor elements rendered for REP price source indicators when pool URL is unavailable

📍 ui/ts/components/OverviewPanels.tsx:73

The component renders an anchor () element for REP/ETH and REP/USDC price source badges even when no pool URL exists for the given source version. For repEthSource='v4', href is set to undefined; similarly for repUsdcSource='v3'. This results in a non-clickable anchor that misleads users expecting a hyperlink. The element should be rendered as plain text (e.g., ) when no valid URL is available.

Exported helper function may cause naming collision or misuse

📍 ui/ts/components/Question.tsx:12

The function getQuestionTitle was changed from internal to exported. While this enables reuse, it also exposes an implementation detail that could be misused by other modules. The function takes a MarketDetails but doesn't guard against undefined fields; if another module passes an incomplete object, it may throw at runtime (e.g., if question.title is undefined). This is a functional risk because the function's contract isn't fully safe for arbitrary consumption.

Hardcoded badge class for Loaded Escalation Game

📍 ui/ts/components/ReportingSection.tsx:50

The badge class for the 'Loaded Escalation Game' EntityCard is hardcoded as 'ok', but it should dynamically reflect the escalation phase returned by getEscalationPhase. Using 'ok' for all phases misrepresents the state; for example, if the phase is 'Ended', the badge should not have the 'ok' class, which typically indicates an active or positive state.

Hardcoded badge class for Latest Reporting Action

📍 ui/ts/components/ReportingSection.tsx:129

The badge class for the 'Latest Reporting Action' EntityCard is hardcoded as 'ok', but it should vary based on the reporting outcome from reportingResult.outcome. A fixed 'ok' class is inappropriate for outcomes like 'Invalid', which should not be styled as positive, leading to misleading UI.

Hardcoded badge text for Escalation Metrics

📍 ui/ts/components/ReportingSection.tsx:88

The badge text for the 'Escalation Metrics' EntityCard is hardcoded as 'status', which is vague and uninformative. It does not convey meaningful information about the metrics or escalation state, reducing UI clarity and potentially confusing users.

ScalarCreatePreview loses label association for slider input

📍 ui/ts/components/ScalarCreatePreview.tsx:29

The parent element wrapping the range input was changed from a label to a div, removing the implicit association between the 'Scalar Preview' text and the input. This breaks accessibility for screen reader users who rely on proper labeling of form controls.

Min and Max value statistics omitted from ScalarCreatePreview

📍 ui/ts/components/ScalarCreatePreview.tsx:45

The refactored component removed the Min Value and Max Value displays from the statistics grid. Users no longer see the lower and upper bounds of the scalar range, which are essential for understanding the market parameters. This may lead to confusion or incorrect market creations.

Incorrect button label precedence when both duplicate pool and Zoltar fork conditions are true

📍 ui/ts/components/SecurityPoolSection.tsx:51

The create button label logic checks 'duplicateOriginPoolExists' before 'zoltarUniverseHasForked'. When both conditions are true (i.e., a pool with the same details already exists and the universe has forked), the label incorrectly shows 'Pool Already Exists' instead of the more fundamental 'Pool Creation Locked'. This misleads the user about the actual reason pool creation is disabled.

Crash when selectedPool.marketDetails is undefined due to unguarded getQuestionTitle call

📍 ui/ts/components/SecurityPoolWorkflowSection.tsx:63

The selectedPoolTitle computation calls getQuestionTitle(selectedPool.marketDetails) without verifying that selectedPool.marketDetails exists. If a pool is selected but its marketDetails property is undefined (e.g., metadata failed to load or is still loading), this will throw a runtime error and crash the component. The rest of the component safely uses a fallback marketDetails variable that aggregates from selectedPool, reporting, and forkAuction sources, but the title does not use this safe variable. The title should either use the same fallback marketDetails or include a guard to avoid passing undefined to getQuestionTitle.

Inconsistent showHeader prop handling in SecurityPoolsSection

📍 ui/ts/components/SecurityPoolsSection.tsx:33

The showHeader={false} prop was removed from SecurityPoolsOverviewSection in the 'browse' view, while it remains explicitly set to false for 'create' and 'operate' views. This inconsistency likely causes the 'browse' view to display an unintended header from SecurityPoolsOverviewSection, violating the consistent header-hidden design across all sub-views and resulting in incorrect UI behavior.

Approve button label and tooltip crash when REP allowance is undefined

📍 ui/ts/components/SecurityVaultSection.tsx:63

When the REP allowance (securityVaultRepAllowance) is undefined (e.g., still loading) and the user enters a positive deposit amount, both approveButtonLabel and approveButtonTitle attempt to format approvalShortage (which is undefined) using formatCurrencyBalance, causing a runtime TypeError that crashes the component. The label expression should handle undefined approvalShortage, and the tooltip function should guard against undefined before formatting. Add a check for approvalShortage === undefined and display an appropriate message (e.g., 'Loading Approval...') while avoiding formatCurrencyBalance calls with undefined.

Withdraw amount input lacks validation and balance check

📍 ui/ts/components/SecurityVaultSection.tsx:39

The REP withdraw amount field is not parsed using parseRepAmountInput, so invalid inputs (negative, non-numeric, decimals) are accepted. Additionally, the withdraw button's disabled condition does not verify that the entered amount is positive and does not exceed the available withdrawable balance (withdrawableRepAmount). This permits submitting transactions that will fail. To fix, parse repWithdrawAmount into a bigint, define hasWithdrawAmount = parsed > 0n, and define hasSufficientWithdrawableAmount = hasWithdrawAmount && withdrawableRepAmount !== undefined && parsed <= withdrawableRepAmount. Then update the withdraw button's disabled condition to use hasSufficientWithdrawableAmount. Also remove the old hasWithdrawAmount definition that only checked for non-empty string.

Deposit button does not validate against available REP balance

📍 ui/ts/components/SecurityVaultSection.tsx:60

The deposit button disabled condition only checks for sufficient allowance (approved REP) but does not ensure the deposit amount does not exceed the user's current REP balance (securityVaultRepBalance). This can lead to deposit transactions failing due to insufficient token balance. To fix, introduce a constant hasSufficientDepositBalance = depositAmount !== undefined && securityVaultRepBalance !== undefined && depositAmount <= securityVaultRepBalance, and include it in the disabled condition of the deposit button.

Missing fallback for unknown vault action type in Latest Vault Action display

📍 ui/ts/components/SecurityVaultSection.tsx:82

The latestActionLabel mapping uses an object lookup without a default case. If securityVaultResult.action contains a value not present in the mapping, the lookup returns undefined and the UI renders 'Action: undefined'. This is incorrect. The lookup should provide a fallback using the raw action string, e.g., change line 82 from '}[securityVaultResult.action]' to '}[securityVaultResult.action] ?? securityVaultResult.action'.

Incorrect 'fully split' message when split limit is zero

📍 ui/ts/components/ZoltarMigrationSection.tsx:133

When the migration outcome split limit is zero and outcome indexes are selected, the hint messages incorrectly state that the amount is already fully split across the selected universes. In reality, a split limit of zero means no REP can be allocated to the selected outcomes, so the REP remains in the migration balance and cannot be split. This misleads users into believing the migration is complete, potentially causing them to forgo necessary actions and resulting in failed migrations. The issue affects both the prepare button hint (via getAlreadyPreparedHint) and the split button hint (splitHintMessage).

Incorrect BigInt conversion in loadOpenOracleReportDetails causing TypeError

📍 ui/ts/contracts.ts:1313

The loadOpenOracleReportDetails function incorrectly wraps values that are already bigints (as returned by readContract for uint256 fields) with BigInt(). This causes a runtime TypeError, preventing the function from executing successfully. Affected fields: settlementTime, feePercentage, protocolFee, multiplier, disputeDelay, reportTimestamp, settlementTimestamp, and numReports.

Incorrect repDepositShare calculation in loadSecurityVaultDetails

📍 ui/ts/contracts.ts:1098

loadSecurityVaultDetails incorrectly treats the first element of vaultData (poolOwnership) as repDepositShare without converting it. The correct approach is to use poolOwnershipToRep to obtain the repDepositShare, as done in loadSecurityPoolVaultSummaries. This leads to wrong repDepositShare values in the result.

Incorrect update of augurPlaceHolderDeployed in setDeploymentStatuses

📍 ui/ts/hooks/useOnchainState.ts:74

The setDeploymentStatuses function sets augurPlaceHolderDeployed.value to true when all deployment steps are deployed, but augurPlaceHolderDeployed should reflect the on-chain deployment status from the snapshot (set in refreshState). Deriving it from local deployment statuses can cause it to be out of sync with the actual on-chain state, leading to incorrect UI or behavior that depends on this flag.

loadOracleReport does not set amount2 from report details

📍 ui/ts/hooks/useOpenOracleOperations.ts:68

The loadOracleReport function updates the form's amount1 and stateHash but fails to set amount2 with the token2 amount from the report (e.g., details.exactToken2Report). As a result, after loading a report, the form's amount2 remains empty or stale, causing approveToken2 to fail when parsing the amount. This breaks the ability to approve token2 after loading a report.

Promise.all causes price fetch to fail entirely when USDC price request fails

📍 ui/ts/hooks/useRepPrices.ts:41

The useRepPrices hook fetches REP/ETH and REP/USDC prices in parallel using Promise.all. If quoteRepForUsdcV4 (or quoteRepForEth with its fallback) rejects, the entire Promise.all rejects, triggering the catch block which leaves all prices undefined. This means a successful ETH price is discarded simply because the USDC fetch failed. The intended behavior is to set each price independently; a failure in one should not prevent the other from being set. The fix is to use Promise.allSettled or separate error handling for each fetch so that partial success is preserved.

Incorrect order of pre-creation checks: deployment check before fork check

📍 ui/ts/hooks/useSecurityPoolCreation.ts:121

The createPool function checks for the deployment of SecurityPoolFactory before checking if the universe has forked. When both conditions are false, the user receives the deployment error instead of the fork error, which is misleading because the fork is an absolute barrier regardless of deployment status. The fork check should be performed first.

resetSecurityPoolCreation does not clear poolCreationMarketDetails

📍 ui/ts/hooks/useSecurityPoolCreation.ts:171

The resetSecurityPoolCreation function only resets securityPoolError and securityPoolResult, leaving poolCreationMarketDetails set. This can cause stale market details from a previous creation to persist after reset, leading to incorrect UI state.

Type mismatch in market details reuse condition always forces reload

📍 ui/ts/hooks/useSecurityPoolCreation.ts:140

The condition marketDetails.value?.questionId === parameters.questionId.toString() compares a bigint to a string, which always evaluates to false. As a result, the hook always fetches market details from the network even when the required details are already cached, causing unnecessary network requests and potential performance degradation.

REP balance not refreshed after depositRep

📍 ui/ts/hooks/useSecurityVaultOperations.ts:141

After a successful deposit of REP into the security pool, the securityVaultRepBalance signal is not updated. The UI will continue to display the old balance, potentially confusing users about available funds.

REP balance not refreshed after withdrawRep

📍 ui/ts/hooks/useSecurityVaultOperations.ts:195

After withdrawing REP from the security pool, the securityVaultRepBalance signal is not updated. The displayed balance will remain stale until a manual refresh.

Stale vault details after account change

📍 ui/ts/hooks/useSecurityVaultOperations.ts:199

When the connected wallet account changes, the hook retains the previous account's securityVaultDetails, securityVaultError, securityVaultForm, and securityVaultResult. This leads to inconsistent UI state where old vault data is shown alongside the new balance/allowance. The hook should reset vault-specific state upon accountAddress changes.

Silent failure in initial data loading due to unhandled promise rejections

📍 ui/ts/hooks/useZoltarUniverse.ts:175

The auto-load effect uses Promise.allSettled to call loadZoltarUniverse and loadZoltarQuestionCountData but ignores the results. Both functions can throw errors (e.g., network failures, contract reverts), and these errors are silently discarded. No error state is updated to inform the user, leading to a situation where loading completes with no data and no indication of failure.

Off-by-one error in effectiveDecimals calculation for two significant figures

📍 ui/ts/lib/formatters.ts:55

The formula Math.ceil(-Math.log10(floatValue)) + 1 overestimates the required decimal places by one for token values where rounding to two significant figures causes a carry that shifts the most significant digit left. This occurs for ranges such as approximately [0.995, 1) and [0.0995, 0.1). The output includes an extra trailing zero, displaying three significant figures instead of the intended two. Examples: tokenValue=0.995 produces '1.00' instead of '1.0'; tokenValue=0.0995 produces '0.100' instead of '0.10'.

Validation does not enforce endTime > startTime when startTime is empty (defaults to 0n)

📍 ui/ts/lib/marketCreation.ts:145

createQuestionData defaults startTime to 0n when the input is empty. validateMarketForm only compares endTime and startTime when both fields are provided (parsed). If startTime is empty and endTime is provided but less than or equal to 0 (e.g., '0'), the validation passes because the relational check is skipped. However, createQuestionData then throws 'End time must be after start time' because endTime (0n) <= startTime (0n). This results in an unhandled exception after the form was validated successfully, breaking the submission flow.

parseBigIntInput accepts negative numbers

📍 ui/ts/lib/marketForm.ts:115

The parseBigIntInput function currently accepts negative numbers by using BigInt(trimmed) without checking for a leading minus sign. Since this function is likely used for parsing non-negative integer inputs (amounts, indexes, etc.), accepting negative values leads to invalid data that could cause transaction failures or incorrect state. The function should reject negative numbers explicitly.

parseRepAmountInput accepts '.' as valid input (should be invalid)

📍 ui/ts/lib/marketForm.ts:102

The parseRepAmountInput function normalizes inputs that start or end with a decimal point. However, a single dot '.' becomes '0.' after normalization, which parseUnits accepts as zero. This incorrectly treats an incomplete input as a valid zero, potentially leading to unintended zero-valued transactions. A single dot should be rejected as invalid.

normalizeDecimalInput does not handle negative numbers with leading decimal point

📍 ui/ts/lib/scalarOutcome.ts:21

The function normalizeDecimalInput normalizes positive numbers starting with '.' (e.g., '.5' -> '0.5') but does not handle negative numbers starting with '-.' (e.g., '-.5'). This leads to parseUnits receiving an invalid string and throwing 'must be a decimal number', rejecting valid user input. The function should also convert '-.5' to '-0.5'.

getScalarSliderFillWidth lacks input validation

📍 ui/ts/lib/scalarOutcome.ts:66

The exported function getScalarSliderFillWidth does not validate its parameters, unlike similar utilities (getScalarSliderProgress, clampScalarTickIndex). Invalid arguments (numTicks <= 0, tickIndex out of range) produce invalid CSS values (e.g., 'calc(Infinity% - Infinity rem + 0.5rem)') or incorrect visual output, potentially breaking the UI. The function should include the same validation as other slider-related functions.

parseScalarFormInputs incorrectly rejects valid two-tick configurations

📍 ui/ts/lib/scalarOutcome.ts:92

The validation condition if (numTicks <= 1n) throws an error when the inputs produce exactly two tick positions (numTicks = 1). According to the error message 'Scalar inputs must produce more than 1 tick', two ticks (which is more than one) should be allowed. The condition is too strict; it should only reject when the number of ticks (numTicks + 1) is <= 1, i.e., when numTicks <= 0. This bug prevents creating binary scalar questions (e.g., Yes/No).

Misplaced test suites: getMigrationOutcomeSplitLimit tests nested under wrong describe block

📍 ui/ts/tests/migrationOutcomeUniversesSection.test.ts:36

The two new tests for getMigrationOutcomeSplitLimit are incorrectly placed inside the describe block for getMigrationOutcomeHeldBalance. This misorganization can lead to confusion in test reporting and violates the principle of grouping tests by the unit under test. They should be moved into their own describe('getMigrationOutcomeSplitLimit') block.

Missing fee and tickSpacing parameters in quoteExactInput call

📍 ui/ts/tests/uniswapQuoter.integration.test.ts:68

In the test for REP/USDC with no V4 pool (line 68), quoteExactInput is called without the required fee and tickSpacing parameters. All other calls to quoteExactInput in this file include these parameters, indicating they are necessary. Omitting them may cause the function to throw due to invalid arguments rather than the absence of a pool, leading to a false positive where the test passes for the wrong reason.

Inistent naming between form field and queued operation in OpenOracleFormState

📍 ui/ts/types/app.ts:49

In OpenOracleFormState, the form field 'securityBondAllowanceAmount' uses singular 'Bond', while the queued operation 'setSecurityBondsAllowance' uses plural 'Bonds'. This inconsistency could lead to bugs in code that maps operation names to form fields, causing undefined values or incorrect behavior, especially if dynamic key lookup or string matching is used.

[github-actions]

🏛️ Architect

36 issues found

Hardcoded line-specific source code modifications in configuration file

📍 .claude/settings.local.json:23

The settings.local.json configuration file now includes Bash(sed) commands that modify specific line numbers (57 and 65) in SecurityPoolsOverviewSection.tsx. This violates key architectural principles: configuration should be declarative and environment-specific, not contain imperative code transformation logic; hardcoding line numbers creates brittle coupling to file structure; this approach bypasses proper refactoring workflows and version control; and it mixes abstraction levels by embedding implementation details in configuration. Such patterns lead to significant maintenance debt and should be replaced with proper codemod scripts or manual code reviews.

Local timestamp state causes potential desynchronization with Anvil node

📍 solidity/ts/testsuite/simulator/AnvilWindowEthereum.ts:81

The refactoring introduced local variables 'currentTimestamp' and 'snapshotTimestamp' to cache the blockchain timestamp. However, the 'request' method allows arbitrary RPC calls, including those that modify time (e.g., 'evm_increaseTime', 'evm_setNextBlockTimestamp', 'evm_mine'), bypassing the local state updates. This can lead to 'getTime' and 'getBlock' returning stale values, making tests unreliable. Additionally, snapshot and revert operations rely on this local state, which may not match the node's state after external time changes, violating the mock's purpose as a faithful proxy.

Inefficient global refresh of file watchers

📍 ui/build/watch.mts:202

The refreshTypeScriptWatchers functions (and the directory watchers that trigger them) clear and recreate all file and directory watchers whenever any single file or directory change is detected. This O(n) operation on each change is an anti-pattern that does not scale with project size and can cause high CPU and I/O. Instead, the watcher set should be updated incrementally: add watchers for newly created files/directories and remove watchers for deleted ones.

Removal of TypeScript compiler state awareness introduces reload race conditions

📍 ui/build/watch.mts:162

Previously, live reload was only triggered upon detecting 'Found 0 errors' in the TypeScript compiler output, guaranteeing a successful compilation before reload. The new approach relies solely on filesystem events for both source and output files, which can trigger reloads before the compiler finishes writing files or on partial/broken builds. This leads to inconsistent browser state and potentially two reloads per change. This is a regression in reliability and an architectural regression from event-driven to filesystem-driven change detection without proper synchronization.

Incomplete adoption of new CSS design tokens

📍 ui/css/index.css:491

The diff introduces new CSS custom properties --notice-error-gradient and --danger-text intended for error notice styling but fails to update the .notice.error component to use them. The .notice.error rule continues to use hardcoded gradient and color values, leading to inconsistency, violating DRY principles, and risking visual discrepancies when design tokens evolve.

God Component Anti-Pattern in App.tsx

📍 ui/ts/App.tsx:34

The App component violates the Single Responsibility Principle by orchestrating at least 9 distinct business domains (deployment flow, market creation, security pools, vault operations, oracle operations, reporting, trading, fork auctions, REP prices). It combines routing logic, global transaction state management, wallet connection state, and UI composition for all sections. This creates high coupling between unrelated concerns, makes testing nearly impossible, and will severely hinder future scalability and maintenance. Any change to one domain risks breaking others due to the monolithic structure.

Inconsistent Hook Abstraction Boundaries

📍 ui/ts/App.tsx:55

The codebase shows an inconsistent pattern for hook configuration: most hooks accept a baseHookConfig object, but useMarketCreation and useSecurityPoolCreation require additional domain-specific parameters (activeUniverseId, deploymentStatuses, zoltarUniverseHasForked), while useRepPrices accepts no configuration at all. This violates the Interface Segregation Principle and creates unclear architectural boundaries. The baseHookConfig appears intended as a contract for infrastructure concerns (transactions, refresh), but its application is irregular, suggesting the abstraction was not systematically designed.

Breaking change in exported utility function contract

📍 ui/ts/components/MigrationOutcomeUniversesSection.tsx:15

The function getMigrationOutcomeHeldBalance was modified to return 0n for non-existing child universes instead of undefined. This alters the function's public contract and may cause runtime errors or incorrect behavior in other modules that rely on undefined to indicate non-existence, violating the principle of stable interfaces and backward compatibility.

Component violates Single Responsibility Principle by aggregating multiple independent workflows

📍 ui/ts/components/OpenOracleSection.tsx:19

OpenOracleSection combines seven distinct responsibilities: displaying report details, oracle manager details, latest oracle action, browsing oracle games, submitting initial reports, disputing reports, and managing oracle manager operations. This high coupling between unrelated features makes the component large (300+ lines), hard to test, and difficult to extend. Each workflow should be extracted into its own focused component with dedicated state and handlers.

Shared form state couples independent workflows, creating implicit dependencies

📍 ui/ts/components/OpenOracleSection.tsx:189

The OpenOracleFormState object aggregates input fields from four independent workflows (browse, report submission, dispute, manager operations). This implicit coupling leads to complex validation logic, unnecessary re-renders when unrelated fields change, and high risk of regressions when extending the form. The state should be decomposed per workflow to enforce separation of concerns and improve maintainability.

Single Responsibility Principle Violation - Component Handles Two Distinct Workflows

📍 ui/ts/components/SecurityPoolSection.tsx:70

The SecurityPoolSection component conditionally renders completely separate UI flows: a pool creation form workflow (lines 104-196) and a pool creation result display workflow (lines 70-103). These workflows have different structures, purposes, and user interactions. This violates the Single Responsibility Principle, making the component complex, hard to test, and brittle to change. Any modification risks affecting both workflows unintentionally.

Code Duplication - Question Display and Loading Logic Repeated Across Branches

📍 ui/ts/components/SecurityPoolSection.tsx:72

The 'Question' EntityCard component with its associated 'Load Question' button is duplicated in both the result view (lines 72-82) and form view (lines 108-129). This violates the DRY (Don't Repeat Yourself) principle, creating a maintenance burden where any visual or behavioral change must be applied in two locations, increasing risk of inconsistency and bugs.

Incorrect data source mixing in marketDetails variable

📍 ui/ts/components/SecurityPoolWorkflowSection.tsx:56

marketDetails is defined as selectedPool?.marketDetails ?? reporting.reportingDetails?.marketDetails ?? forkAuction.forkAuctionDetails?.marketDetails, mixing pool-specific data with data from the current reporting/fork contexts. This causes reportingReady and the Question component to potentially use the wrong market data when the selected pool's marketDetails is not yet available. The pool's own marketDetails should be used exclusively for pool-specific information; no fallback to other contexts.

Missing guard for selectedPool.marketDetails in selectedPoolTitle

📍 ui/ts/components/SecurityPoolWorkflowSection.tsx:63

selectedPoolTitle unconditionally calls getQuestionTitle(selectedPool.marketDetails) when selectedPool is defined, without verifying that marketDetails exists. Since selectedPool may be present while its marketDetails is still loading (or missing), getQuestionTitle may receive undefined, causing an error or invalid title. A guard should fallback to displaying the address when marketDetails is unavailable.

Utility function exported from component file violates separation of concerns

📍 ui/ts/components/SecurityPoolsOverviewSection.tsx:6

The function getQuestionTitle is exported from Question.js, which is a UI component file. This introduces tight coupling between components and utilities, violating architectural principles of separation of concerns. In this codebase, utility functions are consistently placed in ../lib/ directories (e.g., isMainnetChain, formatOpenInterestFeePerYearPercent). Mixing utilities with components can hinder scalability, testing, and maintenance by blurring module responsibilities.

Business Logic Mixed with UI Presentation Layer

📍 ui/ts/components/SecurityVaultSection.tsx:33

The component contains extensive business logic including form normalization, currency parsing with error handling, approval calculations, fee eligibility checks, and conditional enable/disable logic embedded directly in the UI component. This violates separation of concerns principles and makes the component responsible for both presentation and business rules. Lines 33-96 demonstrate calculations that should be extracted to a custom hook or business logic layer. The component also decides when to perform side effects (auto-load vault) based on business rules rather than delegating to a controller or container component.

High Coupling to Specific Implementation Details

📍 ui/ts/components/SecurityVaultSection.tsx:6

The component is tightly coupled to specific implementations of parsing functions (parseRepAmountInput), formatting functions (formatCurrencyBalance), and network checks (isMainnetChain). It also has intimate knowledge of the structure of securityVaultDetails and securityVaultForm objects, including nested property access and specific field semantics. This makes the component difficult to test in isolation and impossible to reuse with different implementations or data sources.

Side Effects in Render Path via useEffect with Complex Dependencies

📍 ui/ts/components/SecurityVaultSection.tsx:83

The useEffect at lines 87-96 performs an auto-load action but depends on 9 different dependencies including derived values like autoLoadKey and normalizedSecurityVaultForm.securityPoolAddress. The dependency array includes values that change frequently, potentially causing unintended re-runs of the effect. The guard conditions inside the effect suggest the logic belongs in a state management layer rather than a UI component. The use of a ref to prevent duplicate loads is a code smell indicating procedural thinking in a declarative framework.

Redundant derived state and inconsistent updates for augurPlaceHolderDeployed

📍 ui/ts/hooks/useOnchainState.ts:71

The signal augurPlaceHolderDeployed is redundant because its value can be derived from deploymentStatuses (when all steps are deployed). It is updated in two places: directly in refreshState (line 89) from the snapshot, and indirectly via a side effect in setDeploymentStatuses (line 74). This violates DRY, creates multiple sources of truth, and can lead to state inconsistency. Additionally, setDeploymentStatuses now has an implicit side effect that couples it to another state variable, breaking single responsibility and encapsulation.

Single Responsibility Principle violation in useOpenOracleOperations hook

📍 ui/ts/hooks/useOpenOracleOperations.ts:22

The hook is responsible for an excessive number of distinct concerns: loading oracle manager and report details, managing form state, approving tokens, requesting prices, queuing operations, submitting/settling/disputing reports. This violates the Single Responsibility Principle, leading to high complexity, poor testability, and difficult maintenance. It should be decomposed into smaller, focused units (e.g., separate hooks or services for data loading and actions).

Tight coupling to blockchain contract layer

📍 ui/ts/hooks/useOpenOracleOperations.ts:3

The hook directly imports and invokes contract interaction functions (e.g., loadOracleManagerDetails, approveErc20), creating a hard dependency on the blockchain implementation. This breaks separation of concerns and the dependency inversion principle, making the UI layer difficult to test in isolation and vulnerable to changes in contract interfaces. An abstraction layer (e.g., a service or use-case interactor) should be introduced to decouple UI from contracts.

Inconsistent data access with runtime type checks

📍 ui/ts/hooks/useOpenOracleOperations.ts:120

The code uses runtime type checks like 'token1' in details to differentiate between OracleManagerDetails and OpenOracleReportDetails when accessing token addresses. This duck-typing approach indicates unclear data models and violates type safety, potentially causing runtime errors if object structures change. A unified interface or explicit type handling should be used instead.

Inconsistent memoization of callback functions in useZoltarFork hook

📍 ui/ts/hooks/useZoltarFork.ts:114

The hook defines runZoltarForkAction as a non-memoized function, causing it to be recreated on every render. approveZoltarForkRep is wrapped in useCallback with runZoltarForkAction as a dependency, but since runZoltarForkAction changes on every render, approveZoltarForkRep also changes on every render, making useCallback ineffective. Additionally, this can lead to stale closures if runZoltarForkAction depends on changing props, as approveZoltarForkRep may hold a reference to an outdated runZoltarForkAction. forkZoltar is not memoized at all, creating inconsistency. This violates the React/Preact pattern of stable function references and can cause unnecessary re-renders and functional bugs when these functions are passed to child components or used in dependency arrays.

Inconsistent validation strategy with duplicated logic

📍 ui/ts/lib/marketCreation.ts:56

The file implements two competing validation approaches: createQuestionData uses exception throwing for title and time order validation, while the new validateMarketForm collects errors into a structured result. The validation for 'title required' and 'endTime > startTime' is duplicated (lines 56-57 vs lines 112-115 and 145-149). This creates architectural inconsistency, making it unclear which function should be used and risking divergent rules when changes are made, as both locations must be kept in sync.

Single Responsibility Principle violation: marketCreation.ts mixes multiple concerns

📍 ui/ts/lib/marketCreation.ts:1

The marketCreation.ts module has grown to include market parameter creation, form validation, parsing helpers, utilities, and cryptographic functions. The new validateMarketForm function alone exceeds 100 lines. This bloat violates SRP, making the module difficult to test, understand, and maintain. Changes to validation, parsing, or market creation all affect the same file, increasing coupling and risk of regressions.

Fragile error message parsing in scalar validation

📍 ui/ts/lib/marketCreation.ts:182

validateMarketForm (lines 186-197) determines which fields to flag as invalid by parsing the text of error messages from parseScalarFormInputs using string checks (e.g., message.includes('Scalar increment')). This tightly couples validation to the exact wording of error messages, making the code brittle. Any change to those messages in parseScalarFormInputs will break field error mapping, potentially causing incorrect UI feedback or silent failures.

Single module violates Single Responsibility Principle by containing multiple unrelated form state factories

📍 ui/ts/lib/marketForm.ts:6

The file marketForm.ts contains default state getters for at least eight different form types (Market, SecurityPool, SecurityVault, OpenOracle, Trading, Reporting, ForkAuction, ZoltarMigration). This creates high coupling between unrelated features, violates separation of concerns, and makes the module a central point of change for many different domains. The filename suggests it only handles market forms, but it has become a dumping ground for all form defaults, which hinders scalability, extensibility, and long-term maintenance. Refactoring into feature-specific modules is required to restore architectural boundaries.

Hardcoded Network and Token Configuration

📍 ui/ts/lib/uniswapQuoter.ts:4

Contract addresses (UNISWAP_V4_QUOTER_ADDRESS, UNISWAP_V3_QUOTER_ADDRESS), token addresses (REP_ADDRESS, USDC_ADDRESS), and pool parameters (DEFAULT_POOL_CONFIG, REP_USDC_V4_POOL) are hardcoded directly in the module. This prevents reuse across different networks or environments, forces code changes for configuration updates, and tightly couples the implementation to specific deployments. These values should be externalized (e.g., via environment variables, configuration files, or dependency injection) to adhere to the principle of separating configuration from code.

Mixing Infrastructure Logic with Domain-Specific Tokens and Business Rules

📍 ui/ts/lib/uniswapQuoter.ts:9

The module combines generic Uniswap infrastructure (ABI definitions, viem client calls) with application-specific domain knowledge: hardcoded token addresses (REP_ADDRESS, USDC_ADDRESS) and convenience functions that exclusively support those tokens (quoteRepForEth, quoteEthForRep, quoteRepForEthV3, quoteRepForUsdcV4). This violates layered architecture and the Single Responsibility Principle. Infrastructure should be independent of domain decisions. The current design forces business logic changes (e.g., adding a new token) to modify infrastructure code, hindering testability, reuse, and maintainability.

Exporting Internal Implementation Constants Breaks Encapsulation

📍 ui/ts/lib/uniswapQuoter.ts:4

The module exports UNISWAP_V4_QUOTER_ADDRESS, an internal implementation detail about which contract implements the quoting logic. This exposes infrastructure specifics to consumers, creating a brittle dependency. If the address changes (e.g., due to an upgrade), all importing modules break. Only the functional API needed by consumers should be exported; internal constants like contract addresses should remain private or be provided via an abstracted configuration interface.

Breaking change: onApproveZoltarForkRep callback signature modified

📍 ui/ts/types/components.ts:83

The onApproveZoltarForkRep prop type in MarketRouteContentProps changed from () => void to (amount?: bigint) => void. This is a breaking change: any component implementing this callback without an optional bigint parameter will fail TypeScript's type checking and may cause runtime errors if the argument is passed. All consumers of MarketRouteContentProps must be updated.

Breaking change: onApproveRep callback signature modified

📍 ui/ts/types/components.ts:206

The onApproveRep prop type in SecurityVaultRouteContentProps changed from () => void to (amount?: bigint) => void. This breaks any existing implementation that does not accept an optional bigint argument, causing type errors and potential runtime failures. This requires updating all components that pass this callback.

Breaking change: Removal of onRedeemRep and onUpdateVaultFees from SecurityVaultRouteContentProps

📍 ui/ts/types/components.ts

SecurityVaultRouteContentProps no longer includes onRedeemRep and onUpdateVaultFees callbacks. These were replaced with onSetSecurityBondAllowance and onWithdrawRep, but the semantics have changed. Any component that previously relied on these callbacks will encounter undefined values and runtime errors when attempting to call them. This is a breaking change that requires refactoring dependent components to use the new callbacks, if equivalent functionality still exists.

Breaking change: Removal of lastCreatedQuestionId and onLoadLatestMarket from SecurityPoolRouteContentProps

📍 ui/ts/types/components.ts

SecurityPoolRouteContentProps removed lastCreatedQuestionId (string | undefined) and onLoadLatestMarket (() => void). Components that read lastCreatedQuestionId or call onLoadLatestMarket will break. The removal of these properties reduces the interface's functionality and may require redesign of related components that depend on them.

Breaking change: Removal of showHeader from SecurityPoolsOverviewSectionProps

📍 ui/ts/types/components.ts

SecurityPoolsOverviewSectionProps was simplified by removing the optional showHeader boolean property that previously allowed controlling header visibility. Any component using this prop will either pass an extraneous prop (React warnings) or, if TypeScript strict checking is enabled, fail to compile. More importantly, the component's behavior might change if it relied on showHeader to conditionally render elements.

Inconsistent naming for state hash fields across oracle-related types

📍 ui/ts/types/contracts.ts:140

The types 'OracleManagerDetails' and 'OpenOracleReportDetails' define fields for state hashes with inconsistent names: 'callbackStateHash' in OracleManagerDetails and 'stateHash' in OpenOracleReportDetails. This naming inconsistency can lead to confusion and errors when these types are used interchangeably or when developers assume they represent the same concept. Given the critical nature of state hashes in blockchain applications, such inconsistencies increase the risk of bugs and undermine code clarity.

[github-actions]

🔧 Expensive Linter

24 issues found

Incorrect markdown list item formatting in Notes section

📍 AGENTS.md:92

The Notes section at the end of AGENTS.md uses a double hyphen prefix ('- -') for list items instead of the standard single hyphen ('-') used throughout the rest of the file. This creates inconsistent markdown formatting and will render as nested list items or plain text depending on the parser.

CSS variable --slider-thumb-border placed in incorrect thematic group

📍 ui/css/index.css:96

The variable --slider-thumb-border is added in the :root block between the '/* decorative /' group and the new '/ notice error /' group, but it logically pertains to form controls and should be grouped with other control-related variables (e.g., --control-bg, --control-border) under the '/ controls */' section. All variables in the codebase are organized under clear thematic headers, and this variable's placement breaks that convention, reducing maintainability.

Inconsistent container element for categorical outcomes field

📍 ui/ts/components/MarketCreateQuestionSection.tsx:151

The categorical outcomes field uses a

element with className='field', while all other form fields use a element with the same className. This breaks consistency in the HTML structure for form controls, even though the

may be necessary due to the inclusion of a button element. The prevailing style in the codebase uses for all other field containers, so this deviation is a stylistic inconsistency.

Inconsistent handling of non-existing child universes in held balance display

📍 ui/ts/components/MigrationOutcomeUniversesSection.tsx:54

The function getMigrationOutcomeHeldBalance now returns 0n for non-existing child universes, but the UI always renders a CurrencyValue component showing 0 REP instead of conditionally rendering a 'Not deployed yet' message as in the original code. This leads to a confusing user experience where a non-existent universe displays a zero balance rather than indicating it is not deployed.Although this has functional implications, the inconsistency in UI representation based on the same data (child.exists) versus the balance value constitutes a stylistic and logical deviation from the original conditional rendering approach.

CSS variable usage in inline style inconsistent with repository patterns

📍 ui/ts/components/ScalarDeploymentSection.tsx:69

The new code introduces a CSS custom property '--slider-fill' in an inline style (line 69). Based on the existing codebase, which uses direct inline styles (e.g., original width style), this represents a stylistic shift without clear precedent. The repository appears to avoid CSS variables in inline styles, favoring direct property assignments for dynamic values.

Class name removal breaks consistent styling pattern

📍 ui/ts/components/ScalarDeploymentSection.tsx:99

The class 'market-scalar-deploy-grid' was removed from the grid container (line 99). This class appears to be part of a naming convention for scalar deployment sections, and its removal may cause styling inconsistencies with other similar components that rely on this class for layout.

Import ordering does not follow repository conventions

📍 ui/ts/components/ScalarDeploymentSection.tsx:1

Imports are not grouped by type (value vs. type) or source. Value imports from 'preact/hooks' are split (lines 1 and 5), and type imports are interleaved with value imports (lines 2 and 8). The repository likely follows a standard grouping: standard library, third-party, local, with type imports grouped separately.

Prop destructuring order not alphabetical for newly added onResetSecurityPoolCreation

📍 ui/ts/components/SecurityPoolSection.tsx:20

The newly added prop onResetSecurityPoolCreation is placed after onSecurityPoolFormChange in the function's destructuring pattern. This breaks the alphabetical ordering of event handler props (onC, onL, onR, onS) observed in the original code. The prop should be moved before onSecurityPoolFormChange to maintain consistent alphabetical ordering.

Destructured parameter order: optional parameters placed before required ones

📍 ui/ts/components/SecurityVaultSection.tsx:11

The function signature for SecurityVaultSection interleaves optional parameters (compactLayout, autoLoadVault) before several required ones, breaking the pattern established in the existing codebase where all required parameters are listed first followed by optional ones. This reduces readability and consistency. Move compactLayout and autoLoadVault after all required parameters.

Local component imports not sorted alphabetically

📍 ui/ts/components/TradingSection.tsx:1

The imports from './' are not sorted alphabetically. 'EntityCard' should be imported before 'EnumDropdown' as 'Entity' comes before 'Enum' alphabetically.

Callback prop onApproveZoltarForkRep not grouped with other callback props

📍 ui/ts/components/ZoltarMigrationSection.tsx:37

In the ZoltarMigrationSectionProps interface and the component's destructuring, the newly added callback onApproveZoltarForkRep is placed after all state-related props, while the existing callbacks (onMigrateInternalRep, onPrepareRepForMigration, onZoltarMigrationFormChange) are grouped together near the top. This breaks the established pattern of grouping all callback props together and reduces readability. The prop should be moved to be adjacent to the other callbacks (e.g., after onZoltarMigrationFormChange).

Incorrect alphabetical order of value imports from 'viem'

📍 ui/ts/contracts.ts:1

The import statement from 'viem' currently lists value imports in alphabetical order, but RpcError is placed after zeroAddress. In case-sensitive alphabetical order, uppercase R should come before all lowercase imports. The correct order is: RpcError, encodeAbiParameters, encodeDeployData, getAddress, getContractAddress, getCreate2Address, keccak256, numberToBytes, parseAbiItem, toHex, zeroAddress.

Inconsistent type import for OpenOracleReportDetails

📍 ui/ts/contracts.ts:24

The function loadOpenOracleReportDetails uses an inline type import (import('./types/contracts.js').OpenOracleReportDetails) instead of importing the type via the top-level type import block, which is the pattern used throughout the file. This should be corrected by adding OpenOracleReportDetails to the top-level type import and using it directly.

Setter function has side effects not indicated by its name

📍 ui/ts/hooks/useOnchainState.ts:71

The function setDeploymentStatuses is named as a simple setter for deploymentStatuses, but it also conditionally updates augurPlaceHolderDeployed when all steps are deployed. This is inconsistent with other setter functions in the codebase (e.g., setAccountState, setErrorMessage) that only modify their respective signals without additional logic. The naming violates the principle of least surprise and may confuse consumers of the hook about the function's full behavior.

Inconsistent use of useCallback for hook return functions

📍 ui/ts/hooks/useZoltarFork.ts:179

The function approveZoltarForkRep is wrapped in useCallback for memoization, but forkZoltar is not, breaking consistency in how callback functions are defined within the hook. This can lead to unnecessary re-renders in consuming components and violates the principle of stable references for hook returns.

Inconsistent variable declaration placement

📍 ui/ts/hooks/useZoltarUniverse.ts

The variable 'zoltarDeployed' is declared after the 'createChildUniverse' function definition, while all other variable declarations (useSignal, useRef) are grouped at the top of the function body. This breaks the consistent pattern observed in the file where variables are declared before any function definitions.

Inconsistent array type annotation style

📍 ui/ts/lib/marketCreation.ts:170

The codebase consistently uses the T[] syntax for array type annotations (e.g., DeploymentStatus[], string[]). In validateMarketForm, an array is typed using Array syntax (Array<{ key: 'scalarMin' | 'scalarMax' | 'scalarIncrement'; label: string }>) instead of the preferred { ... }[].

Inconsistent brace usage for single-statement if blocks

📍 ui/ts/lib/scalarOutcome.ts:87

In parseScalarFormInputs, a single-statement if block uses braces (lines 87-89), while existing code in the same file consistently omits braces for similar single-statement if blocks (e.g., lines 83, 84, 92, and throughout the file). This violates the established code style.

Inconsistent object literal formatting for REP_USDC_V4_POOL

📍 ui/ts/lib/uniswapQuoter.ts:162

The REP_USDC_V4_POOL constant is defined on a single line, while similar constants like DEFAULT_POOL_CONFIG use multi-line formatting with each property on a separate line and trailing commas. This reduces readability and violates consistent style within the same file and type.

Inconsistent argument object formatting in quoteV3ExactInput

📍 ui/ts/lib/uniswapQuoter.ts:149

The args object in the simulateContract call within quoteV3ExactInput is formatted on a single line, while similar objects in quoteExactInput use multi-line formatting with each property on a separate line. This inconsistency affects code readability and violates the established pattern for function arguments in this file.

Tests for getMigrationOutcomeSplitLimit are incorrectly nested under describe for getMigrationOutcomeHeldBalance

📍 ui/ts/tests/migrationOutcomeUniversesSection.test.ts:36

The test file contains a single describe block for 'getMigrationOutcomeHeldBalance', but it includes tests for both getMigrationOutcomeHeldBalance and getMigrationOutcomeSplitLimit. This violates best practices for test organization, where each function should have its own describe block. The tests for getMigrationOutcomeSplitLimit should be moved to a separate describe block to improve maintainability and clarity.

Test name does not match implementation

📍 ui/ts/tests/uniswapQuoter.integration.test.ts:71

The test named 'quoteEthForToken(USDC via REP) throws for all standard fee tiers' actually calls quoteExactInput, not quoteEthForToken. This mismatch can cause confusion when reading test output or failure messages, and does not accurately document the behavior under test.

Inconsistent pluralization in property name

📍 ui/ts/types/app.ts:34

The new property 'securityBondAllowanceAmount' in SecurityVaultFormState uses singular 'Bond', while the existing queuedOperation value 'setSecurityBondsAllowance' uses plural 'Bonds'. This naming inconsistency can cause confusion and should be aligned for clarity and maintainability.

Inconsistent naming of security bond allowance

📍 ui/ts/types/contracts.ts:23

The union member 'setSecurityBondsAllowance' in OracleQueueOperation uses plural 'Bonds', while the field 'securityBondAllowance' in SecurityVaultDetails and the newly added action 'queueSetSecurityBondAllowance' use singular 'Bond'. This naming inconsistency should be standardized across the codebase.

[github-actions]

🧠 Wise Man

37 issues found

Typographical formatting error in documentation bullet points

📍 AGENTS.md:92

In the 'Notes' section of AGENTS.md, each bullet point incorrectly begins with '- - ' instead of the proper markdown list format '- '. This introduces an unwanted dash in the rendered content, impairing readability and creating an unprofessional appearance.

Incorrect snapshot timestamp management for multiple or failed snapshots

📍 solidity/ts/testsuite/simulator/AnvilWindowEthereum.ts:81

The mock tracks only a single snapshotTimestamp variable, which is overwritten each time a snapshot is created. This causes (a) reverting to a non-latest snapshot to restore the wrong timestamp, and (b) if anvilSnapshot fails after setting the variable, the stored timestamp does not correspond to any actual snapshot, breaking subsequent reverts. The correct approach is to maintain a mapping from snapshot IDs to timestamps and update it only after successful snapshot creation.

Internal timestamp state may diverge from Anvil when using raw RPC calls

📍 solidity/ts/testsuite/simulator/AnvilWindowEthereum.ts:58

The mock maintains currentTimestamp to answer getBlock and getTime without RPC, but this state is only updated for eth_sendTransaction and the high-level time manipulation methods. If a test calls any other RPC method that affects the chain timestamp directly via request (e.g., evm_mine, evm_increaseTime, evm_setNextBlockTimestamp, anvil_reset), the internal timestamp becomes stale, causing getTime/getBlock to return incorrect values. Moreover, the snapshot timestamp tracking is also fragile. A more robust design is to query Anvil for the latest block whenever the timestamp is needed, eliminating the need for internal state.

Brittle error handling in manipulateTime using error message text

📍 solidity/ts/testsuite/simulator/AnvilWindowEthereum.ts:212

The SetTimestamp branch of manipulateTime catches an error and checks if the message contains 'timestamp is too big' to decide whether to retry with a decimal-formatted timestamp. This couples logic to exact error wording, which may change across Anvil versions, causing the fallback to fail silently. Moreover, the need for a fallback suggests the initial parameter format may be incorrect; a more robust approach is to validate the timestamp value before sending or rely on structured error information.

Fragile error handling in validateLocalhostUrl based on error message content

📍 solidity/ts/testsuite/simulator/AnvilWindowEthereum.ts:85

The function catches any error from new URL() and then checks error.message.includes('unauthorized') to distinguish between an invalid URL and an unauthorized host. This relies on the exact error message text, which may vary across JavaScript runtimes. Instead, handle the TypeError from new URL() explicitly and perform hostname validation separately without a catch for that case.

Excessive code duplication in TypeScript watcher functions

📍 ui/build/watch.mts:202

The functions watchDirectoryForTypeScriptOutputs, watchDirectoryForTypeScriptSources, refreshTypeScriptOutputWatchers, and refreshTypeScriptSourceWatchers contain heavily duplicated logic. This violates the DRY principle, increasing maintenance burden and risk of inconsistencies. The duplication can be eliminated by extracting common functionality into parameterized functions, which would also centralize error handling for fs.watch calls.

Missing error handling for file system watchers

📍 ui/build/watch.mts:212

The fs.watch calls in watchDirectoryForTypeScriptOutputs and watchDirectoryForTypeScriptSources lack error event listeners. If a watched directory becomes inaccessible (e.g., deleted or permission changes), fs.watch emits an 'error' event that, if unhandled, can cause an uncaught exception and crash the process. This is a reliability risk. Adding error handlers to log such errors is a fundamental defensive coding practice.

Missing walletBootstrapComplete guard in route content disabled condition

📍 ui/ts/App.tsx:184

The isRouteContentDisabled variable previously included a check for !walletBootstrapComplete, ensuring the UI was disabled before the wallet was initialized. This check was removed, allowing user interaction with forms and buttons before the wallet is ready, which can lead to transaction failures, confusing errors, and a poor user experience. The condition should be restored to maintain defensive UI gating.

StepStatus.label type is overly permissive

📍 ui/ts/components/DeploymentSection.tsx:4

The label property in the StepStatus type is defined as string | undefined, but the getStepStatus function always returns a string for label. This mismatch reduces type safety and results in a redundant undefined check in the component's JSX (stepStatus.label === undefined ? ...). Updating the type to string clarifies that a label is always present and enables removal of the unnecessary conditional, improving code clarity and maintainability.

Unnecessary arrow function wrapper in onClick handler

📍 ui/ts/components/ForkZoltarSection.tsx:105

The approve button's onClick handler is unnecessarily wrapped in an arrow function that calls the prop without arguments. This creates a new function on every render, causing unnecessary re-renders for any memoized child components and breaking referential equality. The direct function reference onClick={onApproveZoltarForkRep} is simpler, more performant, and clearer.

Prop type includes unused optional parameter

📍 ui/ts/components/ForkZoltarSection.tsx:14

The prop onApproveZoltarForkRep is typed as (amount?: bigint) => void, implying the component may pass an optional REP amount. However, the component never includes an argument when invoking the callback (line 105). This mismatch between the API contract and actual behavior is confusing and violates the principle of least surprise. The type should be () => void to accurately reflect that no argument is ever passed.

Missing validation feedback for answerUnit field

📍 ui/ts/components/MarketCreateQuestionSection.tsx:178

The scalar 'Answer Unit' input does not receive the 'invalid' prop based on validation state, unlike other scalar fields (scalarMin, scalarIncrement, scalarMax). This creates an inconsistent user experience where some fields show validation errors visually while others do not, even if the validation logic includes errors for answerUnit.

Removal of state-specific CSS class reduces styling granularity

📍 ui/ts/components/MarketOverviewSection.tsx:44

The change removes the 'market-overview-loading' class from the loading state paragraph, which previously allowed targeted styling for the loading condition. This reduces CSS specificity and may require more fragile selectors to style loading states differently, potentially leading to higher maintenance overhead and unintended style leakage into other 'detail' paragraphs.

Incorrect remaining capacity calculation in getMigrationOutcomeSplitLimit

📍 ui/ts/components/MigrationOutcomeUniversesSection.tsx:28

The function uses migrationBalance > heldBalance ? migrationBalance - heldBalance : 0n which incorrectly returns 0n when heldBalance exceeds migrationBalance. This masks a negative remaining capacity and causes the split limit to be computed incorrectly when any selected child has a heldBalance greater than migrationBalance. The correct calculation should be migrationBalance - heldBalance (signed subtraction) to allow negative values, letting the min comparison properly identify the binding constraint.

Duplicate zero address magic string

📍 ui/ts/components/OpenOracleSection.tsx:76

The zero address constant '0x0000000000000000000000000000000000000000' is duplicated in two conditional checks (lines 76 and 130). This violates DRY principles and creates maintainability risk if the value ever needs to change. Extract this into a module-level constant named ZERO_ADDRESS and reuse it.

Component exceeds maintainable complexity

📍 ui/ts/components/OpenOracleSection.tsx

OpenOracleSection has grown to over 300 lines and handles rendering of multiple unrelated sections (Report Details, Oracle Manager, Latest Action, Browse, Submit Report, Dispute, Zoltar). This violates Single Responsibility Principle, increases cognitive load, and makes the component hard to test or modify. Extract each logical section into its own focused component (e.g., ReportDetailsCard, OracleManagerCard, BrowseReportForm, SubmitReportForm, DisputeForm, ZoltarForm) either in the same file or separate files. This will improve readability, enable isolated testing, and simplify future changes.

Range input missing associated label

📍 ui/ts/components/ScalarCreatePreview.tsx:30

The range input element lacks an associated label, which is essential for accessibility. The 'Scalar Preview' text should be wrapped in a element or linked via htmlFor and id attributes to ensure screen reader users understand the input's purpose. This regression occurred when the structure changed from a wrapper to a

.

Duplicated conditional logic for invalid state display

📍 ui/ts/components/ScalarCreatePreview.tsx

The condition isInvalid ? 'Invalid' : ... is repeated across multiple locations to conditionally render text or styles (e.g., fill width, aria-valuetext, metric displays). This duplication violates the DRY principle, making the code harder to maintain and increasing the risk of inconsistencies. Extract these into descriptive variables or helper functions to improve readability and reduce future errors.

Component mixes distinct UI responsibilities; should be split into smaller components

📍 ui/ts/components/SecurityPoolSection.tsx

SecurityPoolSection currently handles two very different user flows: pool creation (form, validation, loading states) and post-creation display (result details and reset). This violates the Single Responsibility Principle, resulting in a large (~200 lines) component with complex conditional rendering. The component becomes harder to read, test, and maintain because changes to one flow may inadvertently affect the other. Additionally, similar UI elements (e.g., the Question card) are used in both states but with slight variations, increasing cognitive load. Extract two focused components: SecurityPoolCreator (handling the creation form, its validation, and related UI) and SecurityPoolResult (displaying deployment details and providing a reset action). This will simplify the main component to a straightforward conditional render, improve clarity, and align with long‑term maintainability best practices.

Nested ternary operator reduces code readability

📍 ui/ts/components/SecurityPoolWorkflowSection.tsx:63

The selectedPoolTitle assignment uses a nested ternary which is hard to read and maintain. Consider using an IIFE or if-else statements for clarity, as nested ternions increase cognitive load and are prone to errors during modification.

Duplication of vault details metrics and actions

📍 ui/ts/components/SecurityVaultSection.tsx:105

The JSX for rendering vault metrics (REP Deposit Share, Approved REP, Security Bond Allowance, Unpaid ETH Fees, Locked REP, Total Security Bond Allowance) and the Claim Fees button is duplicated in two places: within the vaultDetails constant (used for compact layout) and directly inside the main layout's JSX. This violates the DRY principle, increasing maintenance burden and risk of inconsistencies. Extract the common content into a reusable component (e.g., VaultDetailsContent) to ensure a single source of truth.

TradingSection component violates Single Responsibility Principle

📍 ui/ts/components/TradingSection.tsx:9

TradingSection combines multiple distinct concerns: rendering a header, displaying the latest trading result, managing a multi-field trading form, showing errors, and controlling UI visibility flags. This is evidenced by its 11 props and monolithic JSX structure (over 90 lines). A component with such broad responsibilities becomes hard to test, understand, and maintain. Consider extracting focused subcomponents like TradingHeader, TradingResultCard, and TradingFormCard, and introduce a thin container to orchestrate them. This decomposition aligns with SRP, improves readability, and reduces cognitive load.

Dead code in prepareHintMessage: unreachable hasSufficientAllowance check

📍 ui/ts/components/ZoltarMigrationSection.tsx:150

The conditional block if (!hasSufficientAllowance) { ... } within the prepareHintMessage IIFE can never be executed. The preceding conditions cover all possible states: if approvalShortage > 0n returns early; if approvalShortage === 0n and missingPreparationAmount > 0n, then hasSufficientAllowance must be true (allowance defined and sufficient). The case missingPreparationAmount === 0n is handled even earlier. Therefore this branch is unreachable and should be removed to reduce complexity and avoid confusion.

Redundant async/await in loadAllZoltarQuestions

📍 ui/ts/contracts.ts:743

The function uses an unnecessary async callback with await inside the map, and also awaits the Promise.all result. Since Promise.all already returns a promise that resolves to an array of results, we can simplify by removing the async keyword and inner await from the map callback, and remove the outer await. This reduces microtask overhead and improves readability.

Unnecessary 'return await' in async functions

📍 ui/ts/contracts.ts:339

Many async functions directly return the result of awaiting another async operation using 'return await'. Since the enclosing function is already async, returning the promise directly (without 'await') is sufficient and avoids an extra microtask, improving performance and preserving stack traces. This pattern appears in numerous wrapper functions and should be removed.

Multiple sources of truth for augurPlaceHolderDeployed state

📍 ui/ts/hooks/useOnchainState.ts:71

The augurPlaceHolderDeployed signal is assigned in two different locations: within setDeploymentStatuses (lines 71–75) and in the loadDeploymentStatusOracleSnapshot promise resolution (line 89). This violates the single source of truth principle, making state changes harder to reason about and increasing the risk of inconsistencies. Derive this value from a single authoritative source (e.g., blockchain snapshot) or centralize updates in one place to improve maintainability.

Duplicated manager details fetching logic in requestPrice and queueOperation

📍 ui/ts/hooks/useOpenOracleOperations.ts:138

The logic for obtaining OracleManagerDetails (either from the cached signal or by fetching from the contract) is duplicated in the requestPrice and queueOperation action creators. This duplication violates the DRY principle, increases maintenance overhead, and creates a risk of inconsistencies if the logic needs to change. Extract this logic into a single reusable helper function (e.g., ensureOracleManagerDetails) to centralize the responsibility and improve readability.

Incomplete state reset in resetSecurityPoolCreation

📍 ui/ts/hooks/useSecurityPoolCreation.ts:171

The resetSecurityPoolCreation function does not reset poolCreationMarketDetails, leaving stale state after reset. This is inconsistent with the reset logic in createPool, which clears poolCreationMarketDetails before starting a new creation. For a clean reset, all creation-related signals should be cleared to avoid displaying outdated information.

useCallback with unstable dependency renders memoization ineffective

📍 ui/ts/hooks/useZoltarFork.ts:158

The approveZoltarForkRep function is wrapped in useCallback but declares runZoltarForkAction as a dependency, which is recreated on every render because it's defined inline without its own memoization. This makes the useCallback wrapper pointless—the returned function will still change on each render, negating any benefit while adding unnecessary complexity. Since the sibling function forkZoltar is not memoized, it appears that stable references are not required here. The simplest fix is to remove useCallback and define approveZoltarForkRep as a regular async function, aligning with the existing pattern and reducing cognitive load.

useLayoutEffect used for data fetching

📍 ui/ts/hooks/useZoltarUniverse.ts:175

The effect loads remote data (Zoltar universe and question count), which is a side effect unrelated to DOM measurements or synchronous layout. According to React/Preact best practices, data fetching should use useEffect to avoid blocking the browser paint and to follow the intended lifecycle. Using useLayoutEffect for this can cause unnecessary synchronous work before paint and misrepresents the intent of the effect.

Decimals=0 not respected for tiny values

📍 ui/ts/lib/formatters.ts:55

When decimals is 0, the function should return an integer string without a decimal point. After the change, for values less than 1 token (floatValue < 1), effectiveDecimals becomes at least 2 due to Math.max(decimals, ...), causing the function to output a fractional string (e.g., "0.50" instead of "1"). This breaks the contract of the decimals parameter and alters behavior for decimals=0.

Fragile error message parsing in scalar validation

📍 ui/ts/lib/marketCreation.ts:187

The validateMarketForm function uses string inclusion checks on error messages from parseScalarFormInputs to assign field errors to specific form fields. This couples validation logic to the exact wording of error messages, making it brittle—any change to those messages will break field error assignment and lead to incorrect user feedback. It also violates the principle of avoiding magic strings for control flow, reducing maintainability and testability.

Incomplete normalization for negative decimal inputs

📍 ui/ts/lib/scalarOutcome.ts:21

The normalizeDecimalInput function does not handle negative decimal inputs with the decimal point immediately after the minus sign (e.g., '-.5'). This leads to parseUnits throwing an error for otherwise valid inputs, causing unnecessary validation failures. The function should normalize such inputs to include a leading zero, e.g., '-.5' to '-0.5'.

Inconsistent test organization for formatRoundedCurrencyBalance

📍 ui/ts/tests/formatters.test.ts:24

The test suite scatters tests for the same function across different levels: existing tests for formatRoundedCurrencyBalance are direct children of the 'formatting helpers' describe, while new tests are added in a nested describe for a specific scenario. This violates the best practice of grouping all tests for a single unit together, making it harder to locate, understand, and maintain all test cases for that function. Consistent organization improves readability and reduces cognitive load when working with the test suite.

Test suite mixes multiple function units under a single describe block

📍 ui/ts/tests/migrationOutcomeUniversesSection.test.ts:8

The describe block is named 'getMigrationOutcomeHeldBalance' but contains tests for both getMigrationOutcomeHeldBalance and getMigrationOutcomeSplitLimit. This reduces clarity and makes it harder to locate tests for a specific function. Separate describe blocks should be used for each unit under test to improve maintainability and readability.

Test name typo: 'quoteEthForToken' should be 'quoteExactInput'

📍 ui/ts/tests/uniswapQuoter.integration.test.ts:71

The test on line 71 is named 'quoteEthForToken(USDC via REP) throws for all standard fee tiers' but it calls quoteExactInput (REP → USDC). This misalignment misdocuments the test's purpose and can confuse maintainers. The test name should match the actual function being tested and follow the naming pattern used elsewhere (e.g., line 67).

Remove redundant double type assertions

📍 ui/ts/tests/uniswapQuoter.test.ts:28

The code uses the 'as unknown as T' pattern in two places, which is an anti-pattern that unnecessarily bypasses TypeScript's type checking. A single assertion ('as T') or direct element access with a single assertion is sufficient and clearer. This improves maintainability and readability.