Repository navigation
Travel sites: pop-ups, calendars, place boxes, click into view - #78
YellowSnnowmann wants to merge 12 commits into
Conversation
A press on a sight mark goes through a selector, and a selector's element that shows at all is not scrolled first: the press lands on its middle whether the window shows that point or not, and reports success. Live, a store's "Add to cart" sat at the window's foot with its middle below it, and every press went nowhere. Before pressing a sight mark, the surface now runs INTO_VIEW_JS: an element whose middle lies outside the window is scrolled to the window's middle, and one in view is left where it is. A ref of the tree is still brought into view by agent-browser itself. The fake's check for stray page scripts (evaluated_besides_every_press) leaves this script out, as it does the one that keeps a press in the tab.
A click listener Preact keeps on an element (`l` once minified, or
`_listeners`, as "click" or "clickfalse") makes it a control, as a React
press handler does. Live, a hotel site's place suggestions were rows with
only such a listener, read as page text, and the place typed was never
chosen. Inside another control, an element with a press handler of its
own is a control too: a sort menu's options sat inside their
pointer-cursor trigger and read as one button naming them all.
A class behind a variant prefix (`placeholder:text-disabled`,
`disabled:opacity-50`) no longer reads as disabled or selected: a flight
site's place box carried the first and was never seen. Words run together
in camel case in an icon's class ("icClose") are read as words, so a
sign-up pop-up whose only way out was such a sprite can be closed.
A calendar drawn without a table, a grid of 28 to 49 cells numbered from
1 below the month and year it shows, is read as one, its days as dates; a
title naming two months names two grids in order. A picker showing two
months side by side is paged by the block that holds both, and a table's
header is read as shown, so a hidden month list no longer hides the month
it shows. A row scrolled out of its own list's view (a popover's airport
rows below its fold) reads offscreen rather than covered, so it is ranked
and offered rather than dropped.
Each has a live sight test against a page in Chrome, in the new
live_controls_tests.rs and live_calendar_tests.rs topics, along with one
that a control below the window's foot is brought into it and pressed.
Live runs on travel sites stalled on a few patterns any site can draw;
each is now handled by what the page shows, not by where it is.
What Jev is shown. Of the 120 elements a state holds, those in view come
first and the rest keep a quarter of the room in page order (seen_first):
a sign-up pop-up drawn at the end of a long page was cut off, and a
calendar in front crowded out the guests button it covers. A knockout
that must drop chunks drops them first from regions with nothing in view,
so the list in front and a dialog's rows are offered whole. A place box's
suggestion is a row in view and in front, never a footer link of the
same name.
Dialogs. Front keeps whose the dialog in front is (Dialog: the page's,
the task's still asking, answered, or served by a step before), in place
of separate flags. A dialog at a run's first look is the task's only when
the task's run before left its own dialog in front, which contract 2.9
carries: RunFlowRequest and FlowRunResult gain `dialog_left_open`, kept
by the task store for a rescue. A sign-up or a menu the page opened at a
rescue's first look no longer refuses every move past it. A calendar the
task opened and has pressed in since no longer blocks a press behind it,
and on a page that draws its pop-up without a dialog's role, a closer
that went with what it was pressed on is the pop-up dismissed.
Calendars and place boxes. An enter step picks each date from a
calendar already showing before it looks for a box, rather than pressing
the calendar's own button closed. A form that draws its place boxes as
buttons ("From DEL", "To BLR") has each slot's opener pressed once, one
slot at a time (enter/open.rs). A date the field's button shows
("Departure Thu, 22 Oct") counts as made. An enter value with no box is
picked only from what the screen shows, a list's option is one option
however long its label, and "to" or "from" alone names only a container
that begins with it. A pick opens only lists whose items hold something
to press, never a strip of bare fares.
RunFlowRequest's new flag sits beside three others that callers set by
name; its struct_excessive_bools allow says why folding them into one
enum is not worth a major bump.
Tiny Sweeper reviewTiny Sweeper reviewed this change across 6 lane(s) and found 20 active actionable finding(s). Detailed lane evidence and any incomplete work are listed below. State: Changes requested Review snapshot
Completeness: Complete What changedThe review could not produce a supported behavioral summary; inspect the cited changed surface and lane details below. FeaturesNone identified with supported citations. TestsNo supported feature-to-test mapping was produced. Test execution is not inferred. Findings
Resolved this pass
Before merge
Agent review detailscritique
security
tests
commits
description
e2e
Evidence and run details
|
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configuration
Comment |
![tinysweeper[bot]](https://avatars.githubusercontent.com/in/4526585?s=60&v=4){kind=small}
There was a problem hiding this comment.
Requesting changes: 2 lane(s) blocking, worst finding is critical.
Fix or reply to the findings below and push. The next review clears this automatically once they are gone — you should not need to dismiss anything by hand.
$0.1267 · 2,592,786 in / 136,056 out · 251,153 cached (10%) · gpt-5.6-luna, glm-5.3-flash
critique: $0.0736 · 1,417,865 in / 84,626 out · 136,534 cached (10%) · gpt-5.6-luna, glm-5.3-flash
security: $0.0514 · 957,898 in / 42,421 out · 110,907 cached (12%) · gpt-5.6-luna
tests: $0.0008 · 105,325 in / 3,155 out · 3,648 cached (3%) · glm-5.3-flash
description: $0.0003 · 52,850 in / 1,980 out · 0 cached (0%) · glm-5.3-flash
|
|
||
| #![allow(clippy::unwrap_used, clippy::expect_used, clippy::panic)] | ||
|
|
||
| mod live_calendar_tests; |
There was a problem hiding this comment.
Add the declared test module files
These declarations require sight_tests/live_calendar_tests.rs and sight_tests/live_controls_tests.rs (or inline modules), but neither file is present in the complete diff or repository search. Rust will emit missing-module errors during compilation, including builds without the agent-browser feature because the declarations are unconditional. Add the files to this pull request or remove the declarations.
[RULE] missing-module ·
| const titleUses = new Map(); | ||
| for (const grid of base.querySelectorAll('div, ul, ol, tbody')) { | ||
| const kids = grid.children; | ||
| if (kids.length < 28 || kids.length > 49 |
There was a problem hiding this comment.
Process every sibling month grid before marking their shared holder
When two month grids share a parent, the first grid passes this check and the code below adds grid.parentElement to calendars. That parent contains the second grid, so the second iteration is skipped here and its days are never added to calendarDays. A user can therefore select dates from the first visible month but cannot select dates from the second month of a two-month picker, despite the new code explicitly claiming to support that layout. Track processed grids separately or defer adding the shared holder until all sibling grids have been processed.
[RULE] incomplete-multi-month-detection ·
| let in_view = !candidate.states.iter().any(|state| { | ||
| state.eq_ignore_ascii_case("offscreen") || state.eq_ignore_ascii_case("covered") | ||
| }); | ||
| let matches = place |
There was a problem hiding this comment.
Apply the visibility guard to newly discovered rows
in_view only affects matches, but the later condition accepts every new candidate regardless of visibility. For example, a new link named hotels in Goa with an offscreen state still satisfies new, passes this filter, and can be pressed. Move the visibility check outside the new || matches branch so offscreen and covered candidates cannot be selected.
[RULE] incomplete-visibility-filter ·
| ]; | ||
|
|
||
| /// Words of a control that closes what it sits on. | ||
| const CLOSERS: &[&str] = &["close", "dismiss", "skip", "later", "decline", "reject"]; |
There was a problem hiding this comment.
Recognize accepting controls as valid closers
The new window-surface path requires the pressed control's name to contain one of these words. A common cookie-banner dismissal such as intent dismiss cookie banner with target name Accept will therefore return None even when the target disappears and the banner is gone, because accept is present in DISMISS_VERBS but absent from CLOSERS. Include accept (and any other supported semantic dismissal labels) so this path does not stall into rescue.
| const CLOSERS: &[&str] = &["close", "dismiss", "skip", "later", "decline", "reject"]; | |
| const CLOSERS: &[&str] = &["close", "dismiss", "skip", "later", "decline", "reject", "accept"]; |
[RULE] incomplete-accepted-input ·
There was a problem hiding this comment.
Resolved — the review agent found this finding fixed in the new code, as of 27c4886.
If this is wrong, reopen the conversation and say so; the finding will be re-raised on the next push if it still reproduces.
There was a problem hiding this comment.
Resolved — the review agent found this finding fixed in the new code, as of eb42bbf.
If this is wrong, reopen the conversation and say so; the finding will be re-raised on the next push if it still reproduces.
| let closer = words(name) | ||
| .iter() | ||
| .any(|word| CLOSERS.contains(&word.as_str())); | ||
| let gone = !screen |
There was a problem hiding this comment.
Verify the pop-up disappeared, not just its close control
This treats the overlay as closed when no candidate with the same role and name remains, but a page can remove or replace a Close control while leaving the pop-up itself visible. For example, pressing Close can rerender the pop-up and replace that button with another control; gone becomes true and the step is marked Done even though the dismissal was not completed. Check evidence for the containing pop-up or otherwise confirm that the overlay disappeared before ending the step.
[RULE] insufficient-state-validation ·
| /// rescue's first look, and every move past them was refused. | ||
| pub(super) fn look(&mut self, screen: &Screen, location: Option<&str>) -> Option<&'static str> { | ||
| let left_open = self.first_look.take() == Some(true); | ||
| self.calendar = holds_calendar(screen); |
There was a problem hiding this comment.
Clear calendar state when the calendar leaves the front
calendar is updated from every screen independently of which surface is actually in front. If a calendar was answered and then the dialog closes or the flow moves to a page that still contains seven uncovered date-like candidates, served_calendar() can continue returning true while the task is no longer blocked by that calendar. Tie the flag to the currently detected front dialog, or clear it whenever front == "window"/the front surface changes away from the calendar.
[RULE] stale-state ·
| const style = getComputedStyle(parent); | ||
| const scrolls = (/(auto|scroll)/.test(style.overflowY) && parent.scrollHeight > parent.clientHeight) | ||
| || (/(auto|scroll)/.test(style.overflowX) && parent.scrollWidth > parent.clientWidth); | ||
| const found = scrolls ? parent : scrollerOf(parent); |
There was a problem hiding this comment.
Exclude fixed-position descendants from scroll clipping
scrollerOf treats every descendant of a scrollable ancestor as clipped by that ancestor. A position: fixed element can remain visible outside the ancestor's scrollport because fixed positioning is normally relative to the viewport and is not clipped by an ordinary overflow: auto ancestor. For example, a fixed suggestion or dialog nested in a scrollable popover will be reported as offscreen whenever its center lies outside the popover's rectangle, even though it is visible and actionable. Account for fixed-position elements (and transformed containing blocks) before applying the scroll-container bounds check.
[RULE] incorrect-visibility-state ·
| .filter(|index| untried.contains(index) && looks_like_date(&slots[*index].text)) | ||
| .collect::<Vec<_>>(); | ||
| let mut picked = false; | ||
| for index in dates { |
There was a problem hiding this comment.
Process only one date from each displayed calendar
When pending contains multiple date slots, dates contains all of them and this loop calls pick_option for each without taking a fresh screen or verifying that the same calendar is still open. The surrounding comment says selecting a date closes the calendar, so after the first successful selection the later calls operate against a changed UI and can fail or select through the wrong control; those slots have already been removed from untried and will not be retried by this path. Select at most one date per displayed calendar, then refresh the screen before handling the next date. The exact pick_option behavior after the calendar closes was not available in the supplied lookup, so this finding is based on the changed loop and its stated calendar-closing contract.
[RULE] stale-ui-state ·
| mod helpers_tests; | ||
| mod journal_tests; | ||
| mod pick_tests; | ||
| mod picker_tests; |
There was a problem hiding this comment.
Add the picker_tests module before declaring it
Rust resolves this declaration to a sibling picker_tests.rs file or directory module, but no such module exists in the reviewed tree. Test compilation therefore fails with a missing module error. Add the missing module file or remove this declaration.
[RULE] missing-module ·
| /// own dialog in front ([`Front::new`]): live, a sign-up the page opened | ||
| /// on load, and a site's menu drawer, were taken for the task's at a | ||
| /// rescue's first look, and every move past them was refused. | ||
| pub(super) fn look(&mut self, screen: &Screen, location: Option<&str>) -> Option<&'static str> { |
There was a problem hiding this comment.
Preserve the look-call contract
Existing callers pass the previous browsing argument to Front::look; removing it here without corresponding updates makes those callers fail to compile. Either retain the parameter or update every caller in the same change.
[RULE] build-break ·
Front took any press in a dialog the task opened for its answer, so a
calendar whose month was turned counted as served, and a covered press
could close it before its day was picked. Pressing a control that only
turns the month ("next month", "Previous month") now leaves the calendar
asking; a day pressed in it still serves its field.
A grid of bare numbers counted as a calendar's days even with no month on
screen, so a seat map pressed in could be closed for a press behind it.
Grid cells named by a number now count only when the screen names a month
somewhere; cells that name or are described with a month count as before.
A run that ended before its first look reported no dialog left open,
dropping the one its run before had left. Front::left_open now hands that
dialog on until the run has looked.
A dismissal step on a pop-up drawn without a dialog's role ends when the control pressed names a closer and went with what it sat on. "Accept all" and "I agree" close a consent bar as surely as "Reject all" does, but were not closer words, so a step accepting cookies on such a bar fell back to a judge that cannot see which button closed it. A simulated cookie bar now checks that accepting it ends the step.
closest kept a long option row by an exact role match, while is_one_option, beside it, compares roles in any case. It now uses that helper, so an "Option" row is kept like an "option" one. The in_region rustdoc now says a placing word's region never holds a control that only names it, which is what the code does on purpose.
The simulated ride form drops a box's unpicked text when a press lands anywhere else, as a page does, except when the press was the other box's "From"/"To" button: that switched boxes and kept the old list on show. It now closes the open list first, so a flow cannot pass by picking from a list a real page would have closed.
Sight took each grid calendar's parent as its month while still looking for grids, so when two months' grids sat side by side in one block, the first claimed the block and the second was skipped: none of its days read as dates. Grids are now all found first, and a block holding two of them is no one month; the picker pass still adds it so its arrows page both. A row's middle outside its scrolling container reads offscreen, but a control fixed to the window moves with no container and none clips it. A fixed button drawn from inside a scrolling list read offscreen wherever it showed. scrollerOf now stops at a fixed element. Live tests in Chrome cover both, and each fails without its fix.
named_opener's rustdoc said a label holding a slot's word anywhere, while it looks only at the first three words; it now says so and why. seen_first said "those in view first" where it keeps them first but lists every kept element in screen order, as jev-harness.md says. The contract spec's 2.9 note broke the sentence it sat in; it now closes the version list and explains dialog_left_open apart.
There was a problem hiding this comment.
Requesting changes: 2 lane(s) blocking, worst finding is critical.
Fix or reply to the findings below and push. The next review clears this automatically once they are gone — you should not need to dismiss anything by hand.
$0.0884 · 1,577,666 in / 126,097 out · 143,701 cached (9%) · gpt-5.6-luna, glm-5.3-flash, gpt-6-luna
critique: $0.0463 · 821,241 in / 66,289 out · 84,583 cached (10%) · gpt-5.6-luna, glm-5.3-flash
security: $0.0375 · 576,708 in / 50,132 out · 59,118 cached (10%) · gpt-5.6-luna
tests: $0.0005 · 57,709 in / 2,204 out · 0 cached (0%) · glm-5.3-flash
description: $0.0006 · 58,271 in / 5,089 out · 0 cached (0%) · glm-5.3-flash
| /// "To BLR" for the slot "to". Further into a label, the slot's word is a | ||
| /// sentence's ("Read our tips to search faster"), and a press there leaves | ||
| /// the form. | ||
| pub(in crate::agentic::flow) fn named_opener( |
There was a problem hiding this comment.
Add tests for named opener selection
This revision adds nontrivial opener-selection behavior, including word normalization, short place leads, label-position filtering, covered-state rejection, and exclusion of editable fields, but the diff contains no declared test module for it. The repository rule requires tests beside the module and at least 90% line coverage; without those tests, regressions in these boundary cases can merge undetected.
[RULE] missing-behavioral-tests ·
| sim.quirks.remove(&Quirk::PromoToast); | ||
| return DesktopResponse::ok("click", json!({})); | ||
| } | ||
| if name == "Accept all" && sim.has(Quirk::CookieBar) { |
There was a problem hiding this comment.
Block clicks behind the cookie bar
CookieBar is documented as sitting over the page, but refused_click has no corresponding covered-click branch. Consequently, while the quirk is active, clicking any underlying control other than the eventual Accept all path succeeds and mutates the simulated page. Tests using this quirk can therefore pass without dismissing the banner and fail to exercise the intended recovery behavior; exempt the accepting control and return a covered response for other clicks while the bar is present.
[RULE] incomplete-overlay-model ·
| let closer = words(name) | ||
| .iter() | ||
| .any(|word| CLOSERS.contains(&word.as_str())); | ||
| let gone = !screen |
There was a problem hiding this comment.
Verify the pressed control disappeared with its overlay
This treats the target as gone whenever no candidate has the same role and name, but it does not verify that the candidate belonged to the dismissed overlay or that the screen actually changed because of the action. A page can rerender or replace a same-named control with a new ref_id, or the target can be absent from the post-action snapshot for unrelated reasons, causing a dismissal step to be marked Done even though the overlay remains. Compare the overlay-specific state (or the target's stable identity/ancestor information) before accepting the action as a successful close.
[RULE] verify-target-disappearance ·
| const [month, year] = title.months[Math.min(used, title.months.length - 1)]; | ||
| const spelled = MONTHS[month]; | ||
| grids.push(grid); | ||
| for (const { cell, day } of run) { |
There was a problem hiding this comment.
Process only one date from each displayed calendar
This still records every day in each discovered grid. A displayed calendar can therefore contribute dozens of date candidates, allowing the observation to expose many mutually exclusive choices instead of one representative date per calendar as required by the existing behavior. Limit the mapping to the intended single date for each displayed calendar, or otherwise preserve the one-date invariant used by the caller.
[RULE] duplicate-candidates ·
| .candidates | ||
| .iter() | ||
| .filter(|candidate| { | ||
| let name = candidate.name.as_deref().unwrap_or_default(); |
There was a problem hiding this comment.
Parse the day from descriptions and non-leading date words
Calendar detection only parses the first whitespace-delimited token of candidate.name. Common accessibility labels such as Thu Oct 01 2026, October 1, 2026, or a candidate named 1 with the full date in description therefore fail the day test, even though the surrounding code explicitly treats descriptions as calendar evidence. With fewer than seven detected cells, calendar remains false, so served_calendar() does not recognize the open picker and later actions can be refused as being behind it. Extract a valid day number from the name and description rather than requiring it to be the first token of the name.
[RULE] incomplete-calendar-detection ·
| .iter() | ||
| .filter(|candidate| { | ||
| let name = candidate.name.as_deref().unwrap_or_default(); | ||
| let day = name |
There was a problem hiding this comment.
Recognize day numbers anywhere in calendar labels
This only accepts a day when it is the first whitespace-delimited token. The detector's documented Thu Oct 01 2026 form therefore fails, as do labels that put weekday or month text before the day, causing a real calendar not to be recognized and its served-state handling to be skipped. Parse the date structure supported by the accessibility label instead of requiring the first token to be numeric.
[RULE] incomplete-input-recognition ·
| let node = grid; | ||
| for (let depth = 0; node && node !== base && depth < 4; depth += 1, node = node.parentElement) { | ||
| let sibling = node.previousElementSibling; | ||
| for (let step = 0; sibling && step < 3; step += 1, sibling = sibling.previousElementSibling) { |
There was a problem hiding this comment.
Ignore hidden siblings when identifying calendar headings
gridTitle accepts any preceding sibling whose text contains a month and year, but it never checks that sibling is visible. A hidden month list or stale accessibility/template node can therefore cause an arbitrary 28-item grid to be treated as a calendar, producing false date candidates and potentially directing clicks to unrelated controls. Require shown(sibling) before using its text as calendar evidence.
[RULE] hidden-content-trust ·
| &form, | ||
| 190.0 + 40.0 * f64::from(u8::try_from(index).unwrap()), | ||
| )); | ||
| if places.door.as_deref() != Some(*name) { |
There was a problem hiding this comment.
Clear the active door when the list closes
When the open list is dismissed by pressing somewhere other than a suggestion or another door, drop_unpicked clears open but does not clear door. This guard therefore continues rendering the old textbox even though its autocomplete is closed, leaving the behind-buttons form in an inconsistent state. Clear door whenever the open list is closed without selecting a place.
[RULE] stale-ui-state ·
| .next() | ||
| .and_then(|word| word.parse::<u8>().ok()) | ||
| .is_some_and(|day| (1..=31).contains(&day)); | ||
| let dated = (month_shown && candidate.role.eq_ignore_ascii_case("gridcell")) |
There was a problem hiding this comment.
Require month evidence for the same calendar grid
A month name anywhere on the screen combined with seven numeric gridcell candidates is enough to classify the screen as a calendar. A page containing a month in unrelated text and a separate seat map or data grid can therefore be marked as a calendar, making later presses behind it subject to the picker-specific flow. Tie the month evidence to the same calendar container or require date-labelled candidates rather than accepting any grid on the screen.
[RULE] insufficient-context-validation ·
| .any(|state| state.eq_ignore_ascii_case("covered")); | ||
| day && dated && !covered | ||
| }) | ||
| .count() |
There was a problem hiding this comment.
Count dates within one displayed calendar
The detector counts all matching candidates on the screen as one calendar. When two calendars are displayed together, or when unrelated numeric grids coexist with a picker, candidates from separate widgets can collectively reach seven and falsely establish calendar state. Group candidates by their calendar container and require one group to supply the threshold.
[RULE] cross-widget-aggregation ·
A pick "closest to 9" was judged item by item, and live, with a store's sizes 9 and 10 sold out, Jev found none of 6, 7, and 8 clearly closest, so the step failed and every rescue went back to the sold-out 9. tinycomputer-core gains closest_to, which reads the number a criterion such as "closest to 9" or "nearest to size 42" names, and rank_closest, which ranks records by the distance of the first number each shows. pick ranks such a criterion within the list from names (asking which when several show, as for "first", through the new named_list), and takes the first ranked item that belongs, as for prices. Both are additive.
Return is refused while a sheet or alert shows, since there it presses the dialog's default button. Live, a store's search opened as a full-window sheet, and the step to press Enter in the box just typed into was refused 154 times; a rescue had to pick a suggestion instead. FlowRun now keeps the field its last action typed into (typed_last, kept through waits), and Return goes through in front of a sheet when that field is a search box: a searchbox, or a box that takes text and names itself for searching. A simulated search that opens as a sheet checks both that Return runs it and that Return stays refused with no search box typed into.
A product grid repeats one "ADD" on every card, and by its name alone
every copy is the same. Live, "add the first Maggi product" pressed the
first card's ADD, on a ramen above the Maggi, and two packets of the wrong
product went into the cart.
Sight now describes a control whose role and name repeat, with no
description of its own, by the name of the card holding it when that
card is a control itself ("in Maggi Double Masala 95 g ₹20"). A live
test in Chrome checks it, and that a control no other card repeats is
left alone.
There was a problem hiding this comment.
Requesting changes: 2 lane(s) blocking, worst finding is high.
Fix or reply to the findings below and push. The next review clears this automatically once they are gone — you should not need to dismiss anything by hand.
$0.0700 · 904,877 in / 75,127 out · 69,977 cached (8%) · gpt-5.6-luna, glm-5.3-flash, gpt-6-luna
critique: $0.0266 · 420,422 in / 35,847 out · 42,663 cached (10%) · gpt-5.6-luna, glm-5.3-flash
security: $0.0380 · 279,388 in / 26,801 out · 27,186 cached (10%) · gpt-5.6-luna
tests: $0.0006 · 66,076 in / 4,821 out · 64 cached (0%) · glm-5.3-flash
description: $0.0006 · 66,551 in / 4,151 out · 64 cached (0%) · glm-5.3-flash
| // item, took none of 6, 7, and 8. | ||
| (Some(target), _) => { | ||
| let groups = self.named_list(log, &screen, &from, &families).await?; | ||
| rank_closest(&records_of(groups), target).map(|order| (groups, order)) |
There was a problem hiding this comment.
Select the number for the requested field
rank_closest ranks by the first number in each Record, while records_of preserves every field in reading order. For a list whose cards show a price before a size, a criterion such as closest to size 42 compares prices instead of sizes; for example, a card priced 42 with size 100 will outrank a card priced 50 with size 42. Extract or rank the field named by the closest-to criterion rather than passing the unqualified records to rank_closest.
[RULE] wrong-ranking ·
| sim.obstacle = false; | ||
| sim.quirks.remove(&Quirk::Drawer); | ||
| drop_unpicked(&mut sim); | ||
| if sim.has(Quirk::CalendarStaysOpen) |
There was a problem hiding this comment.
Close the calendar on Escape outside the obstacle path
This state update is inside the existing obstacle-handling branch, so pressing Escape while only CalendarStaysOpen is active never reaches it: sim.obstacle is false, the calendar remains Some, and subsequent screens still report surface: "dialog" while Find flights remains covered. Move this calendar cleanup into the general Escape handling path, outside the obstacle-only branch, so the documented Escape behavior can actually close the calendar.
[RULE] state-transition-guard ·
| `ctrl+n` on Windows and Linux and the browser sends `Meta+n` or `Control+n`. | ||
| Return is refused while a sheet or alert is showing, because there it presses | ||
| the default button. | ||
| the default button, unless the run's last action typed into a search box, |
There was a problem hiding this comment.
Require the search box to remain active before overriding modal Return
This exception is based only on the run's last action. For example, after typing in a search box and then opening a sheet or alert, the last action still matches even though the modal is now visible; pressing Return will run the search instead of the modal's default button. Tie the exception to the currently active, visible search box (and verify that no sheet or alert is showing), or keep the modal guard authoritative.
[RULE] stale-state ·
| pub fn rank_closest(records: &[Record], target: f64) -> Option<Vec<usize>> { | ||
| let keyed = records | ||
| .iter() | ||
| .map(|record| { |
There was a problem hiding this comment.
Select the number from the intended record field
Record.fields is a BTreeMap, so .values() visits fields in lexicographic key order, not in the order the record displays them or according to the field named by the criterion. For example, a record with price = "$9" and size = "6" will use 9 as its closest-to value because price sorts before size, ranking it incorrectly for a criterion such as "nearest to size 8". Pass the relevant field or otherwise define field-selection semantics instead of taking the first numeric value from the map.
[RULE] incorrect-field-selection ·
| // sits: live, a store's search opened as a full-window | ||
| // sheet, and its step to press Enter in the box just typed | ||
| // into was refused 154 times. | ||
| let searching = self.typed_last.as_ref().is_some_and(is_search_box); |
There was a problem hiding this comment.
Require the typed search field to still be active
typed_last is used as the sole evidence that Return should bypass the non-window dialog guard. If a search field was typed into and a sheet or dialog then appeared before the shortcut is chosen, this stale candidate still makes searching true and Return presses the overlay's default action. Check that the typed field is still present and active in the current screen (or clear this state when the front surface changes) before allowing the exception.
[RULE] stale-state-guard ·
| sim.quirks.remove(&Quirk::PromoToast); | ||
| return DesktopResponse::ok("click", json!({})); | ||
| } | ||
| if name == "Accept all" && sim.has(Quirk::CookieBar) { |
There was a problem hiding this comment.
Block clicks behind the cookie bar
This handles the bar's dismissal button, but the click-refusal path does not include Quirk::CookieBar. While the bar is present, clicks on underlying controls can therefore succeed instead of being reported as covered, allowing tests of obstacle handling to pass incorrectly. Add the cookie-bar quirk to the same refusal logic used by the other overlays, while keeping Accept all as the exception.
[RULE] overlay-click-blocking ·
| // drawn from inside a scrolling list shows wherever it is placed. | ||
| const scrollers = new Map(); | ||
| const scrollerOf = (element) => { | ||
| if (style(element).position === 'fixed') return null; |
There was a problem hiding this comment.
Ignore fixed-position ancestors when finding a scroller
Only the element itself is checked for position: fixed. A fixed-position descendant nested inside a scrolling ancestor will still be assigned that ancestor as its scroller and can be incorrectly marked offscreen, even though it is positioned relative to the viewport. Walk the ancestor chain for a fixed positioning context before applying scroll clipping.
[RULE] fixed-position-clipping ·
| if (kids.length < 28 || kids.length > 49 | ||
| || [...calendars, ...grids].some((calendar) => calendar.contains(grid))) continue; | ||
| const days = [...kids].map((kid) => { | ||
| const leading = /^(\d{1,2})(?:\s|$)/.exec(squash(kid.innerText)); |
There was a problem hiding this comment.
Recognize day numbers anywhere in calendar labels
The non-table calendar path only accepts a day number at the beginning of a cell's text. Cells labelled like Mon 22 or Tue, 23 are visible calendar dates but are discarded, so those calendars produce no usable date controls. Extract a valid day number from the complete shown cell label while retaining the month-range validation.
Additional critique observation
Parse day numbers anywhere in calendar labels
[RULE] incomplete-date-parsing
The non-table calendar path only recognizes a day when it begins the cell's text. Cells such as Choose date 22, Tue 22, or labels with an accessible prefix are skipped, so an otherwise valid 28-day grid is not discovered and none of its dates are offered. Search for a day number at a word boundary within the visible label rather than requiring it to be the first token.
Suggested change for this observation (reference only)
const leading = /(?:^|\s)(\d{1,2})(?=\s|$)/.exec(squash(kid.innerText));
[RULE] calendar-day-parsing ·
| const start = days.findIndex((entry) => entry && entry.day === 1); | ||
| if (start < 0) continue; | ||
| const run = []; | ||
| for (const entry of days.slice(start)) { |
There was a problem hiding this comment.
Limit generated dates to the titled month's length
The new non-table calendar path accepts any sequential run after day 1, including days 32–42 in a six-week grid. It then stores those values as dates for the titled month, so a calendar with trailing cells or a malformed sequential grid can expose controls for dates that do not exist. Stop the run at the last day of the titled month before populating calendarDays.
[RULE] invalid-date-range ·
1 participant
Summary
Fixes for the travel-site rounds after #77 (Goibibo, OYO, MakeMyTrip, Agoda, ixigo). Each one handles a pattern any page can draw; none names a site.
This PR does not depend on tinyhumansai/agent-browser#2. The
vendor/agent-browsergitlink is unchanged (ebf10fb, as on main). The click-into-view fix was first tried in agent-browser'selement.rs. It is done here in tinycomputer instead.Three commits:
INTO_VIEW_JS(surface/uncover.rs) now scrolls such an element to the window's middle before the press and leaves one in view alone.l/_listeners), e.g. OYO's place suggestions.placeholder:text-disabled) are no longer read as disabled or selected.icClose).Frontkeeps whose dialog is in front (Dialog: the page's, or the task's asking, answered, or served). A dialog at a rescue's first look is the task's only when the run before left it open, and contract 2.9 carries that across.Related issue
Part of tinyhumansai/openhuman#7000. Follows #77.
API or behavior changes
RunFlowRequest.dialog_left_openandFlowRunResult.dialog_left_open, both optional and defaulting tofalse. The task controller sets the field for a rescue run itself, so a host needs no change. A host built against 2.8 still binds this module; a host built against 2.9 needs a 2.9 module.Validation
cargo fmt --all -- --check: clean.cargo clippy --all-targets --all-features -- -D warnings: clean on every crate excepttinycomputer-accessibility, which this PR doesn't touch. That crate's macOS-only code (focus.rs,paste.rs,permissions.rs) fails 4 lints under local clippy 0.1.98, and fails the same way on main. CI runs on Ubuntu, where that code isn't compiled.cargo build --all-targets --all-features: not run on its own. The clippy run and the test build above compile every target.cargo test --all-features, run ascargo test --release --workspace --exclude tinycomputer-accessibility --all-features: every suite passes. Re-run after the new tests moved into topic files: engine 452 passed, browser 135 passed.RUSTDOCFLAGS="-D warnings" cargo doc --no-deps --all-features --workspace --exclude tinycomputer-accessibility: clean.Live sight tests in Chrome (
TINYCOMPUTER_LIVE_BROWSER=1): 19 passed.Live site runs through the loaded module (
task_live), all made before the last fixes:enterstep left To and the date to rescues. Search, 4 flights read, the cheapest picked and Book pressed all went through. It ended when the destructive guard refused a "… Pay ₹4,300 fee …" radioFront's flags were folded into one state to satisfy clippy. This doesn't change behavior.live_a_control_whose_middle_is_below_the_window_is_brought_into_it_and_pressed.Tests
flow_tests/picker_tests.rs: a closer that goes with its pop-up; a calendar picked in, closed for a press behind it; a date picked without pressing its button.CalendarStaysOpen, and place boxes drawn as buttons.sight_tests/live_controls_tests.rsandsight_tests/live_calendar_tests.rs, plus the scrolled-list case inlive_tests.rs. These need Chrome andTINYCOMPUTER_LIVE_BROWSER=1, and return early without them.INTO_VIEW_JS's scroll itself runs only in the live test.Documentation
docs/crates/tinycomputer-browser/interacting.md: bringing a sight mark into view.docs/crates/tinycomputer-browser/sight.md: Preact listeners and variant classes.docs/technical/decision-thresholds.md:CALENDAR_DAYS.docs/technical/jev-harness.md: in-view-first order of the 120 elements.docs/technical/specs/desktop-module-contract.md: the 2.9 bump.crates/tinycomputer-bus/src/flow/guide.md: a pop-up that may not show is closed in anifstep.Checklist
#[allow(...)],#[ignore], or relaxed lints: there is one#[allow(clippy::struct_excessive_bools)], onRunFlowRequest. The comment beside it gives the reason: folding its four flags into an enum would renameallow_destructive,include_valuesandtraceon the wire, which is a major bump. The same allow already sits ondescribe.rsandobservation/types.rs..envcontents in the diff or the description.Known gaps, not fixed here
enterstep left To and the date to rescues. A radio worded as a fee ("… Pay ₹4,300 fee …") is refused as irreversible, which is an acceptable end.