gui: typing on the Presets screen no longer works the preset out twice per key - #131
Merged
Merged
Conversation
…e per key Every change of a box settled the form twice - once in refreshLine for the line under the buttons and once in recheck to mark the box - and on the Presets screen each settle expanded the preset. With upload-validation chosen that is image encoding: about 300 ms of the window's thread and 117 MB of garbage for every key, while the seed was what was being typed. recheck now settles once and hands the reading to lineFrom, which says the line from it. refreshLine keeps its shape for the other callers, and its comment no longer says settling costs nothing. The Presets screen remembers the last expansion (lastExpansion), keyed by the preset and a copy of the values it is given. The seed and the output directory are not among them, so typing in them expands nothing. A refusal is kept too, since internal/preset reads no clock, randomness, environment or disk. Every settle is on the window's thread, so there is no lock. The parameters and the defaulted list go to the manifest as copies. Measured in the real window (tools/probes/guilag, main and this branch interleaved, three runs each, median per key): - seed, upload-validation: 303-335 ms / 117 MB -> 0-6 ms / 2 MB; - seed, tabular-import: 70-81 ms / 168 MB -> 0 ms / 1 MB; - the preset's own limit, upload-validation: 295-379 ms -> 149-160 ms. Two optional host interfaces let a guard count readings and expansions (countedSettle, tellExpanding). Nothing in the shipped program implements them. New guards in settleonce_test.go: one change of a box reads the form exactly once on all three screens, typing what a preset is not given does not expand it again, and a changed value or another preset is expanded again with the line following it. Not changed: menus and switches that rebuild the Several batches screen still read the form twice, and a key typed there with a preset switched on still expands the preset once. Co-Authored-By: Claude Opus 5.5 <[email protected]>
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Advanced Run ID: 📒 Files selected for processing (9)
Included review availability: Your plan provides up to 10 included reviews per hour; 7 remain after this review. 📜 Recent review details⏰ Context from checks skipped due to timeout. (15)
🧰 Additional context used📓 Path-based instructions (10)Applies to text shown to the user (labels, buttons, tooltips, placeholders, dialogs, errors, status messages, empty states, translations).⚙️ CodeRabbit configuration file Files:
Verify tests check real behavior and would fail if the implementation were broken.⚙️ CodeRabbit configuration file Files:
Performance is a known weak spot of these projects.⚙️ CodeRabbit configuration file Files:
Applies only to code that builds or styles a GUI.⚙️ CodeRabbit configuration file Files:
User-facing changelog.⚙️ CodeRabbit configuration file Files:
SECURITY, HIGH PRIORITY.⚙️ CodeRabbit configuration file Files:
Go code.⚙️ CodeRabbit configuration file Files:
Check that documentation matches the actual code in this PR: commands, flags, config keys, file paths, build steps and examples must exist.⚙️ CodeRabbit configuration file Files:
All code in this repository is written by an AI coding agent (Claude Code).⚙️ CodeRabbit configuration file Files:
Source excerpt: **Words a user reads are English, with a flat hyphen and no semicolons.**📄 CodeRabbit inference engine (CONTRIBUTING.md) Files:
🔇 Additional comments (1)
📝 WalkthroughWalkthroughThe change caches preset expansions for matching inputs and reuses settled form values during runner rechecks. Tests measure form reads and preset expansions across screens. The changelog records reduced typing delays on the Presets screen. ChangesForm settling and preset expansion
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant PresetsScreen
participant Recheck as runner.recheck
participant PresetSettle as Preset.settle
participant LastExpansion as lastExpansion
PresetsScreen->>Recheck: Field change triggers recheck
Recheck->>PresetSettle: Settle form once
PresetSettle->>LastExpansion: Look up selected preset and arguments
LastExpansion-->>PresetSettle: Return cached or new expansion result
PresetSettle-->>Recheck: Return targets, options, and error
Recheck->>Recheck: Update status line and refusal marks
Suggested labels: 🚥 Pre-merge checks | ✅ 13 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (13 passed)
Full details: Clear User-Facing TextExplanation The PR adds user-facing release-note text in
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
✨ Simplify code
Comment |
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
1 participant
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.
What changes for the user
Typing on the
Presetsscreen no longer lags. Withupload-validationchosen, every key typed into a box held the window for about 0.3 s, because the preset was worked out twice for each key - and working it out encodes images.How
rechecksettled the form for the line (refreshLine) and then again to mark the box. It now settles once and hands the reading tolineFrom. The comment onrefreshLinethat said settling costs nothing is corrected.lastExpansion), keyed by the preset and a copy of the values it is given. The seed and the output directory are not given to a preset, so typing in them expands nothing. A refusal is remembered too -internal/presetreads no clock, randomness, environment or disk. Every settle runs on the window's thread, so there is no lock.ParametersandDefaultedgo to the manifest as copies.countedSettle,tellExpanding) let a guard count readings and expansions. Nothing in the shipped program implements them.Measured
tools/probes/guilag, the real window,mainand this branch interleaved, three runs each, median time the window's thread was busy per key / garbage per key:mainupload-validationtabular-importlimit,upload-validationThe live heap is unchanged (29.3 MB after every phase).
Guards
internal/guard/settleonce_test.go:TestOneChangeOfABoxReadsTheFormOnce- exactly one reading per change on all three screens, an emptied box included. Nought readings fails as "not in the state it asks about".TestTypingWhatAPresetIsNotGivenDoesNotExpandItAgain- the seed and the directory expand nothing, and four keys are checked to have read the form four times.TestAChangedPresetValueIsExpandedAgain- a new value and another preset are expanded again and the line follows. It checks that the two presets are given nothing (nothingGiven) instead of assuming it:size-boundariesarrives with itsformatmenu set, which let the first version pass a mutation that ignored the preset's name.Seven new mutation entries, all caught. Four existing entries whose patterns this change moved were re-pointed and caught again.
Not changed
Several batches, menus and switches that rebuild the screen still read the form twice (counted: batch format, the base switch, the base preset). A key typed there with a preset switched on still expands the preset once per key.🤖 Generated with Claude Code
Summary by CodeRabbit