Repository navigation
fix(minimize): address Finding 4 UX issues - #24
Merged
Merged
Conversation
Three fixes from Finding 4:
1. Fix contradictory 'Minimal: No' when trace is already minimal
- Changed is_minimal to always be True on success since a trace
that cannot be reduced further IS minimal
- Updated warning to say 'Trace was already minimal' instead of
the confusing 'already minimal' next to 'Minimal: No'
2. Keep run_end in minimized failed traces
- The CLI was incorrectly reconstructing the minimized subset by
taking the first N events from the original trace by index
- Now uses the actual minimized_events returned by the engine,
which properly preserves run_start and run_end per docs/minimize.md
3. Add -e short flag to replay and export commands
- The -e shorthand for --entrypoint existed on minimize but not
on replay or export, making CLI inconsistent
- Added -e to both commands for consistency
Tests added for each fix:
- test_minimize_already_minimal_shows_minimal_yes
- test_minimize_failed_trace_keeps_run_end
- test_minimize_short_flag_e_for_entrypoint
- test_replay_cli_short_flag_e_for_entrypoint
- test_export_cli_short_flag_e_for_entrypoint
Co-authored-by: Cursor Agent <cursoragent@cursor.com>
Owner
|
Thanks @gary1381 for opening this PR and fixing the three Finding 4 UX issues! Verified and merged. The |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Relates to #17 (Finding 4). Finding 1 stays with you as discussed.
This fixes the three small
minimizeUX issues listed under Finding 4.1.
Minimal: Nonext to "already minimal"is_minimalis nowTrue. A trace that cannot be reduced further is minimal, so the report no longer contradicts itself.2.
run_enddropped from minimized failed tracesrun_end(5 events became 4).MinimizationResult.minimized_events), and the CLI saves those.run_startandrun_endare preserved, matchingdocs/minimize.md.3.
-eshort flagreplayandexportnow accept-efor--entrypoint, likeminimizealready does.Tests
Five regression tests:
test_minimize_already_minimal_shows_minimal_yestest_minimize_failed_trace_keeps_run_endtest_minimize_short_flag_e_for_entrypointtest_replay_cli_short_flag_e_for_entrypointtest_export_cli_short_flag_e_for_entrypointLocally on Python 3.13.5 (Linux), on top of
f6b2241:uv run pytest607 passed;ruff check,ruff format --checkandmypyare clean.Picked up via Agonna (agonna.com), a network where agents and people post needs like this and others take them on. If you have more help-wanted items, feel free to post them there.