Keep each collection's response history to itself - #87
Merged
Conversation
Hunting the same shape as the selection bug — an identifier assumed unique that isn't — with probes that assert the correct behaviour and let the failures name the bugs. Five written, two failed, and both were the same one: history is bucketed by request id alone, and a loaded endpoint's id is `METHOD /path` in every collection describing that API. So staging and production shared one list. Opening either showed whichever had been sent last, and clearing one deleted both — including from disk, since `clear_request` matched on request_id. The fix was mostly already there: the database has stored `section_id` since the column was added, and simply never returned it. Selecting it and putting it on `HistoryRecord` is enough for the window to tell the two apart. `clear_request` takes an optional section, and unscoped still means everywhere, which is what scratch needs. Entries recorded before this match any collection rather than vanishing — nothing knows where they came from, and dropping them would read as history lost. A scoped clear takes those too: they are the same request's older entries, and leaving them would look like the clear half-worked. The three probes that passed are kept. They cost nothing to run and they are the ones that would notice if overlay bodies, endpoint-forgetting or schema lookups ever stopped being section-scoped. Two earlier drafts of them passed by taking an `if` around a menu item that is not there for loaded rows; a test that asserts nothing when its branch is not taken is worse than no test, so both were rewritten to be unconditional. Verified the way the last one had to be: reverting the filter fails the probe.
Merged
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.
Hunting the same shape as the selection bug — an identifier assumed unique that isn't — by writing probes that assert the correct behaviour and letting the failures name the bugs.
Five probes over two collections describing one API. Two failed, and both were the same bug.
Found: history is shared between collections
Bucketed by request id alone, and a loaded endpoint's id is
METHOD /pathin every collection describing that API. So:clear_requestmatched onrequest_id.Fix
Most of it was already there: the database has stored
section_idsince the column was added and simply never returned it. Selecting it ontoHistoryRecordis enough for the window to tell the two apart.clear_requesttakes an optional section; unscoped still means everywhere, which is what scratch needs.Old entries match any collection rather than vanishing — nothing knows where they came from, and dropping them would read as history lost. A scoped clear takes them too: they're the same request's older entries, and leaving them behind would look like the clear half-worked.
Passed, and kept
They cost nothing and they're what would notice if any of those stopped being section-scoped.
On the probes themselves
Two early drafts passed by taking an
ifaround a context-menu item that doesn't exist for loaded rows — a test that asserts nothing when its branch isn't taken is worse than no test. Both were rewritten to be unconditional, and there are no skips.Verified the way the last one had to be: reverting the filter fails the probe.
Testing
cargo fmt --check,cargo clippy --no-default-features --all-targets -- -D warnings,cargo test(137 passed),pnpm check(0 errors),pnpm test:e2e(328 passed, 5 new).