Skip to content

Remove unused code following some UCP removal#4444

Open
jmg-duarte wants to merge 4 commits into
mainfrom
jmgd/remove-ucp/cleanup
Open

Remove unused code following some UCP removal#4444
jmg-duarte wants to merge 4 commits into
mainfrom
jmgd/remove-ucp/cleanup

Conversation

@jmg-duarte

Copy link
Copy Markdown
Contributor

Description

Removes a bunch of "dead code" — follow up to #4370 (comment)

Changes

  • Removes error paths that are no longer triggered

How to test

Existing tests

jmg-duarte added 3 commits May 7, 2026 09:41
Scoring uses auction-level native prices and settlement verification
reads prices from on-chain calldata, so per-solution UCPs are dead
weight. Drop them from the autopilot domain `Solution`, the
`SolutionError::InvalidPrice` variant, and the `proposed_solutions`
price columns (now written as empty arrays).

The `/solve` wire format is preserved for rolling-deploy safety:
- autopilot tolerates a missing `clearingPrices` (`#[serde(default)]`)
- our driver still emits it from existing settlement data

`clearingPrices` is marked `deprecated: true` in the driver and
orderbook OpenAPI specs. A follow-up PR will remove the field, the
deprecation log, and the empty DB columns.
@jmg-duarte jmg-duarte requested a review from a team as a code owner May 26, 2026 10:16
@jmg-duarte jmg-duarte marked this pull request as draft May 26, 2026 10:16

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request simplifies the solution domain conversion logic by removing the ZeroScore and SolutionError types, along with the Score::try_new validation. As a result, into_domain methods now return domain solutions directly instead of wrapping them in a Result. This allows for the removal of error-handling logic, error metrics tracking, and partition-filtering in run_loop.rs and shadow.rs. No critical issues were found, and there is no feedback to provide.

@github-actions

github-actions Bot commented Jun 3, 2026

Copy link
Copy Markdown

This pull request has been marked as stale because it has been inactive a while. Please update this pull request or it will be automatically closed.

@github-actions github-actions Bot added the stale label Jun 3, 2026
Base automatically changed from jmgd/remove-ucp/2 to main June 9, 2026 11:58
@github-actions github-actions Bot removed the stale label Jun 10, 2026
@jmg-duarte jmg-duarte marked this pull request as ready for review June 15, 2026 17:23
@claude

claude Bot commented Jun 15, 2026

Copy link
Copy Markdown

Claude finished @jmg-duarte's task in 2m 24s —— View job


PR Review — Remove unused code following some UCP removal

  • Read PR diff and changed files
  • Verify the removed error paths are truly unreachable
  • Check for any related callers / metrics that might still reference removed types
  • Post review feedback

Summary

Clean, well-scoped follow-up to #4370. The cleanup falls out naturally once Score::try_new's zero-score check and the SolverDenyListed solution-side path are no longer reachable — Response::into_domain always succeeded, so the Result<Solution, SolutionError> wrapper was pure ceremony. Net: Score becomes infallible to construct, into_domain returns Vec<Solution> directly, and the call-site filter / partition_result in run_loop.rs + shadow.rs collapse to a straight map.

Correctness

  • Score::try_new removal — verified no remaining callers in the workspace (grep ZeroScore|SolutionError|competition::Score::try_new → 0 hits). Score is now only constructed via the tuple struct at crates/autopilot/src/domain/competition/winner_selection.rs:80,91,104, where scores originate from the winsel ranking — not raw driver input — so the zero-score guard is moot.
  • Response::into_domain (crates/autopilot/src/infra/solvers/dto/solve.rs:166) — previously always wrapped in Ok(...), so the unwrap is mechanical and safe. Driver-side SolverDenyListed still exists as a SolveError variant short-circuited in try_solve (run_loop.rs:713,721), which is correct — the removed SolutionError::SolverDenyListed was never produced in this codepath anyway.
  • shadow.rspartition_result() and the itertools::Itertools import drop cleanly because into_domain no longer yields a Result iterator. No behavioral loss: the dropped tracing::debug! only logged errors that could never occur.

Minor observations (non-blocking)

  • Metrics::solutions at crates/autopilot/src/run_loop.rs:1003 still declares the ("driver", "result") label dimension, but the only emitted value is now "success". Keeping the dimension is fine for dashboard stability, but worth noting that the counter is now equivalent to a single-label ("driver",) counter — could be flattened in a future cleanup if dashboards don't depend on the label.
  • The new map in solve() (run_loop.rs:670-676) increments solution_ok once per solution before constructing the Bid. Functionally equivalent to the prior Ok branch — no double-counting since the error branch went through flatten() on vec![].

Verdict

LGTM. The diff is purely subtractive on dead branches, no behavior change for the non-error paths, and grep confirms zero dangling references to the removed types.
| Branch

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request simplifies solution handling within the autopilot domain by removing the ZeroScore validation and its associated SolutionError type. Consequently, solutions are now processed directly as Solution rather than being wrapped in a Result, which streamlines the mapping, filtering, and partition logic in run_loop.rs, shadow.rs, and the solver DTOs. Additionally, logging for deprecated clearing prices has been enhanced to include more context, such as the solution ID and submission address. No review comments were provided, and there is no additional feedback to address.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant