refactor(frontend): reduce NewRun prop syncing

[jeffspahr]

Summary

• remove the remaining page-side prop-mirroring effects in NewRunV2
• switch pipeline, experiment, and displayed pipeline-version state to keyed initialization instead of effect-driven syncing
• add rerender coverage for pipeline-name and experiment-name prop changes

Why

NewRunV2 still had two non-toolbar useEffect blocks whose only job was to copy pipeline, experiment, and version display data from props into local state after render. That pattern is the same effect misuse this cleanup series has been removing elsewhere: it adds post-render repair work, makes rerender behavior harder to reason about, and encourages react-hooks/exhaustive-deps suppressions.

This PR keeps the slice narrow:

• the toolbar effect stays, because it is legitimate external synchronization
• NewRunParametersV2 is intentionally unchanged here, because its remaining mount-time propagation behavior still has a broader standalone-test contract and should be handled in its own follow-up if we keep going

What changed

• NewRunV2 now initializes pipelineName, selectedExperiment, and pipelineVersionName with useKeyedState(...) keyed from the corresponding prop identity instead of syncing them in effects
• the latest-version checkbox now only toggles useLatestVersion; blanking the displayed version name is derived from the keyed state initialization rather than an effect or imperative checkbox handler reset
• run submission now reads the selected experiment directly from selectedExperiment
• NewRunV2.test.tsx now verifies that rerendering with a different pipeline prop updates the displayed pipeline name, and rerendering with a different chosen experiment prop updates the displayed experiment name

Verification

• cd frontend && fnm exec --using .nvmrc -- npx prettier@3.8.1 --check src/pages/NewRunV2.tsx src/pages/NewRunV2.test.tsx
• cd frontend && fnm exec --using .nvmrc -- npm run test:ui -- src/pages/NewRunV2.test.tsx
• cd frontend && fnm exec --using .nvmrc -- npm run typecheck

Result: formatter passed, src/pages/NewRunV2.test.tsx passed with 34 tests, and typecheck passed.

[Copilot]

Pull request overview

This PR refactors NewRunV2 to eliminate post-render effect-driven prop syncing by switching to keyed state initialization for pipeline, experiment, and pipeline version selection. The refactoring removes two useEffect blocks that were copying prop values into local state and replaces them with useKeyedState-based initialization that automatically resets state when the keyed prop identity changes.

Changes:

• Replaced effect-driven state syncing with keyed state initialization for pipelineName, selectedExperiment, and pipelineVersionName
• Derived experimentId and experimentName from selectedExperiment instead of maintaining separate state
• Simplified the "Always use latest version" checkbox handler by removing manual state clearing (now automatic via key change)
• Added tests verifying rerender behavior when pipeline and experiment props change

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File	Description
frontend/src/pages/NewRunV2.tsx	Refactored state management to use keyed initialization, removing effect-driven syncing while maintaining correct behavior
frontend/src/pages/NewRunV2.test.tsx	Added tests to verify state updates when pipeline and experiment props change on rerender

[manaswinidas]

Good refactoring — extends the existing useKeyedState pattern to replace two prop-mirroring effects. A few observations:

This is more than a refactor. The old experiment state was initialized with useState(chosenExperiment) but never re-synced when the chosenExperiment prop changed — only experimentName/experimentId were synced via the effect (which depended on local experiment, not the prop). That meant startRun could submit a stale experiment if chosenExperiment changed without user interaction. The keyed-state version fixes this. The new rerender tests correctly cover this previously-untested behavior.

Minor suggestions (non-blocking):

1. pipelineVersionName key expression — the useLatestVersion ? \latest:${...}` : versionIdkey is clever (thelatest:` prefix prevents key collision), but it encodes toggle state into a key string in a way that's non-obvious. A one-line comment explaining the intent would help future readers.

2. Test boilerplate — the two new tests manually spread all props instead of using the existing buildNewRunV2Element helper (line ~225). Using it would cut ~20 lines per test:

   const { rerender } = render(
     <CommonTestWrapper>
       <NewRunV2 {...buildNewRunV2Element({ chosenExperiment: DEFAULT_EXPERIMENT })} />
     </CommonTestWrapper>,
   );

3. handleExperimentChange behavioral delta — the old code guarded setExperimentName behind if (experiment.display_name), preserving a stale name if the new experiment lacked one. The new derivation (selectedExperiment?.display_name ?? '') shows empty instead. This is the better behavior, just noting it's a subtle change.

Overall LGTM — correctness looks solid, test coverage matches the behavioral change, and it removes two eslint-disable candidates.

[manaswinidas]

/lgtm

[manaswinidas]

There's a merge conflict

[jeffspahr]

Addressed the review feedback on the rebased head e83810076:

• Rebasing: rebased onto current master and resolved the NewRunV2 overlap from #13196; the PR is now mergeable again.
• pipelineVersionName key readability: added a short comment in NewRunV2 explaining why the latest:${pipelineId} prefix exists.
• Test boilerplate: rewrote the two new rerender tests to use the existing renderNewRunV2 / buildNewRunV2Element helpers from master.
• Experiment state correctness: kept the keyed selectedExperiment path as-is; that is the intended fix for the stale-submission behavior you called out, and the rerender regression coverage remains in place.
• handleExperimentChange display-name delta: left this as-is intentionally. Showing an empty value for an experiment without display_name is the new behavior, and I agree with your note that it is the better outcome.

Focused verification on the rebased branch passed:

• cd frontend && fnm exec --using .nvmrc -- npx prettier@3.8.1 --check src/pages/NewRunV2.tsx src/pages/NewRunV2.test.tsx
• cd frontend && fnm exec --using .nvmrc -- npm run test:ui -- src/pages/NewRunV2.test.tsx
• cd frontend && fnm exec --using .nvmrc -- npm run typecheck

Result: formatter passed, NewRunV2.test.tsx passed with 36 tests, and typecheck passed.

[manaswinidas]

/lgtm

[zazulam]

/approve

[google-oss-prow]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: zazulam

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

• frontend/OWNERS [zazulam]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment