Add replay-from-trace and scenario-authoring tooling#587
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Self-Review FindingsTrace data sensitivity
Edge cases reviewed
Potential improvements (not blocking)
No issues found that warrant fixing before merge. |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a suite of internal developer tools for recording and replaying user action traces, as well as a scenario editor for authoring test cases. It includes a new useTraceRecorder composable, a TraceReplayEngine utility, and a DevToolsView component protected by a feature flag. While the implementation is comprehensive, the review identifies issues with the scenario editor's parameter handling, where numeric values like durationMs are incorrectly treated as strings, leading to validation failures. Additionally, the feedback suggests improving error visibility during file imports by logging caught exceptions to the console to assist in debugging.
| <input | ||
| :value="(step.params as unknown as Record<string, unknown>)[key]" | ||
| type="text" | ||
| class="w-full bg-zinc-900 border border-zinc-600 rounded px-2 py-1 text-xs text-zinc-200" | ||
| @input="(step.params as unknown as Record<string, unknown>)[key] = ($event.target as HTMLInputElement).value" | ||
| /> |
There was a problem hiding this comment.
The generic input for scenario step parameters treats all values as strings. This will cause a validation error for numeric parameters like durationMs in a wait step, which is expected to be a number. The input should handle numeric types correctly by checking the parameter key and converting the value to a number if necessary.
<input
:value="(step.params as any)[key]"
:type="key === 'durationMs' ? 'number' : 'text'"
class="w-full bg-zinc-900 border border-zinc-600 rounded px-2 py-1 text-xs text-zinc-200"
@input="(step.params as any)[key] = key === 'durationMs' ? Number(($event.target as HTMLInputElement).value) : ($event.target as HTMLInputElement).value"
/>
| } catch { | ||
| traceError.value = 'Failed to import trace file.' | ||
| } |
There was a problem hiding this comment.
When catching an error during file import, it's helpful for debugging to log the actual error to the console, especially for a developer tool. This provides more context than just a generic error message in the UI.
} catch (err) {
traceError.value = 'Failed to import trace file.'
console.error('Failed to import trace:', err)
}
| } catch { | ||
| scenarioErrors.value = [{ path: '', message: 'Failed to read file.' }] | ||
| } |
There was a problem hiding this comment.
Similar to the trace import, logging the actual error to the console when a scenario import fails will aid in debugging. This is particularly useful for a developer-facing tool.
} catch (err) {
scenarioErrors.value = [{ path: '', message: 'Failed to read file.' }]
console.error('Failed to import scenario:', err)
}
- Use dynamic :type binding to detect numeric params (e.g. durationMs) and render type="number" input with Number() conversion on input - Add console.error logging in trace import catch block - Add console.error logging in scenario import catch block
Cover non-Error throw path, replay-after-completion restart, seekTo during active play, pause no-op when idle, elapsed time reporting, playback speed timing, assert text-equals validation, null params, non-object steps, and all createBlankStep type defaults.
…ranch coverage threshold Cover numeric-to-string mapping, out-of-range fallback, and case-insensitive string normalization for command run status, restore status, proposal status, source type, and risk level. Also test featureFlagStore restore with empty localStorage.
Fixes appliedGemini code review findings (DevToolsView.vue)
CI coverage threshold failureBranch coverage was 70.28%, below the 71% threshold. Added 66 new tests across 8 files:
Verification
|
…d, and roles utils Cover previously untested branches: out-of-range numeric fallbacks, empty/whitespace inputs, missing payload segments, and normalizer edge cases. Brings branch coverage from 70.61% to 71.06%, passing the 71% CI threshold.
1 participant
Summary
types/trace.ts) and recorder composable (useTraceRecorder) for capturing user action sequences during normal usetraceReplay.ts) with play/pause/stop/seek and adjustable speed for re-executing recorded tracesscenarioSchema.ts) with type-safe step validation for all 7 step typesdevToolsfeature flag (disabled by default)Closes #332
Test plan
npm run typecheckpassesnpm run buildpasses