Repository navigation
Conversation
![propr-dev[bot]](https://github.githubassets.com/images/gravatars/gravatar-user-420.png){kind=avatar}
…ow be scrolled fully into view with a real mouse wheel, touch drags or keyboard focus. Nothing was run on the signed build or a real Android device, so your native check and the Astra review are still the real tests. Cause. The creation dialog's short-window layout, where the whole dialog scrolls under a sticky header, only switched on while a phone keyboard was open. In a short desktop window the dialog kept its normal layout: the dialog itself can't scroll, and only the form area above the buttons does. At 370px wide the header wraps to 125px and the buttons wrap to two rows (133px). Together they're taller than the dialog, so Run task's bottom was cut off and no wheel or touch could reach it. The existing native-titlebar test missed this because Playwright's `scrollIntoViewIfNeeded` can scroll a box that a user can't. Fix: - `CreationDialog.tsx`: the short-window layout now applies to any short window, not just under a keyboard. The full-width, square-cornered look stays keyboard-only, so desktop dialogs keep their rounded corners and borders. As a fallback, the dialog itself scrolls whenever its contents don't fit, so nothing is ever clipped at its edge. - `NewTaskPage.tsx` and `GoalsPage.tsx`: the form area keeps at least 6rem of height. In a window that can't fit that plus the header and buttons, the whole dialog scrolls instead of the prompt shrinking to nothing. The 40px close button, 44px action buttons, all creation features, and the existing 65px Follow Up and 89px phone-keyboard layouts are unchanged. Tests. The new `e2e/creation-short-window.pw.ts` covers New Task and New Goal in five windows: - the issue's Linux window (restored and maximized), at 370×345 with device pixel ratio 2.378. - web 370×345 and web 260×420. - Linux 420×480 with enlarged text. Each test keeps an unsent draft with the repository selected. It then uses real mouse-wheel and touch input to bring each whole button into view. It checks the top and bottom edges plus nine tap points inset from the rounded corners. It also confirms the caret stays visible, keyboard focus reaches Run task, the draft and repository survive, nothing is sent, and the titlebar and window controls stay usable. On the unfixed code, New Task fails in three windows, as expected: Run task's bottom is cut off by 10px at the issue's geometry (restored and maximized) and by 57px with enlarged text. The other two web sizes don't clip on the old code, so they only guard against future regressions. All 10 pass with the fix. Results: - Existing browser suites: these cover native titlebar, New Task, goals, phone-keyboard bands and dialogs. No test failed. The first combined run was stopped by its time limit after 90 passing tests, so I re-ran the two files it hadn't finished (27 passed). - Other checks: 250 unit tests, eslint with zero warnings, and tsc all pass. Two existing-test edits: - One assertion in `creation-keyboard-viewport.pw.ts` checked that the dialog had `overflow:hidden` in portrait. It now checks that the dialog doesn't scroll there, which was the original intent. - `stubGoals` moved into `new-task.fixture.ts` so both test files share it. I didn't touch any #3251 files, dependencies or permissions.
|
/review gpt-6-astra |
|
✅ AI Code Review Complete requested by @integry Posted 1 review: |
🔍 AI Code Review — gpt-6-astraOverall EvaluationThe PR addresses the reported clipping cause by enabling whole-dialog scrolling outside keyboard mode and preserving a minimum form-body height. It needs a localized correction before merge: the sticky-header threshold still assumes the compact keyboard header, which can leave desktop/web controls impossible to reveal fully. ✅ Targeted layout change — Both creation forms receive the same minimum body height, and the shared shell permits overflow to scroll. ✅ Useful regression coverage — The new tests exercise wheel input, Chromium touch drags, action hit testing, draft preservation, and native-chrome clearance. This review is static; no commands were run. The supplied issue description is truncated, so unseen acceptance details were not assessed. Current checks show no failures and six pending checks. Merge blockersEvery finding below was introduced by this PR and must be resolved before merging. F1: 🔴 Account for the taller non-keyboard header
SuggestionsThese are optional follow-ups and are not sent to S1: 🟢 Assert return-scroll destinationsIn ScoreThe implementation is focused and adds useful interaction coverage, but a newly exposed short-window boundary still prevents full action visibility. A localized header-threshold correction is needed; current checks have no reported failures. Score: 6/10 🤖 Review Details
Copy and edit to choose records from this review: 🤖 Review by ProPR |
|
/fix F1 S1 |
|
✅ Applied the requested follow-up changes in commit eb98095
Summary of ChangesI fixed F1 and added S1. In short windows without a keyboard, both New Task and Start a goal now let every whole action button scroll fully into view. The new boundary tests fail on the old code and pass with the fix, but nothing was run on a native build or a real device. Cause. Without a keyboard, the header is 73px tall, not 41px, but the PR made it sticky at the keyboard's 4.8rem threshold. At 370×130 that left only 31px below it for 44px buttons. Fix (
Tests (
Results:
I captured previews of both forms at 370×130 and 370×144. Visual previewStart a goal, 370×130 web window
The same header-scrolls-away layout lets Start goal come wholly into view. Original unavailable: managed storage is unavailable or the upload has expired. Start a goal, 370×144 web window
Sticky header with the whole Start goal action visible beneath it. Original unavailable: managed storage is unavailable or the upload has expired. New Task, 370×130 web window
Below the 7.5rem sticky band the header scrolls away with the form, so Run task is wholly visible after scrolling down. Original unavailable: managed storage is unavailable or the upload has expired. New Task, 370×144 web window
At the 7.5rem threshold the 73px header stays sticky, with the whole 44px Run task visible beneath it. Original unavailable: managed storage is unavailable or the upload has expired. 🤖 Implementation Details
Undo Changes • View Task Execution 💡 ProPR Slash Commands
🤖 Generated by ProPR |
|
Evidence for the existing I independently reproduced F1 on exact head b23ee74 using pinned full Chromium151.0.7922.34, source-owned API fixtures, no credentials or writes, and real wheel input. After waiting for the form/repository/model setup and enabling submit, resize370x345 to370x130 with keyboard closed, then18 real wheel notches in the pane gutter. Both New Task and Start a goal have a106px pane,73px sticky header, and31px available below it for the44px enabled action. At maximum scroll only15px of the action is visible; its upper part is under the header. I directly inspected the OS-independent screenshot confirming the clipped action. Please use this boundary in the current F1 regression and preserve normal keyboard/native-titlebar behavior. The initial operator probe had a fixture-readiness failure (disabled submit); corrected probe waited for field focus and sequential input, then reproduced both forms. This is now executed evidence, rather than only a static CSS inference. Files retained locally at |
…d, both New Task and Start a goal now let every whole action button scroll fully into view. The new boundary tests fail on the old code and pass with the fix, but nothing was run on a native build or a real device. I fixed F1 and added S1. In short windows without a keyboard, both New Task and Start a goal now let every whole action button scroll fully into view. The new boundary tests fail on the old code and pass with the fix, but nothing was run on a native build or a real device. **Cause.** Without a keyboard, the header is 73px tall, not 41px, but the PR made it sticky at the keyboard's 4.8rem threshold. At 370×130 that left only 31px below it for 44px buttons. **Fix (`propr-ui/src/components/CreationDialog.tsx`):** - **Without a keyboard:** the header becomes sticky only when the space inside the layer's padding is at least 7.5rem (120px). That fits the 73px header, a 44px button and the pane's borders. Below that, the header scrolls away with the form. - **With a keyboard:** the 4.8rem threshold and 41px header are unchanged, but the threshold now applies only while a keyboard is open. - **Second instance of the same assumption:** the scroll padding was still sized for the 41px header (44px). At 370×144, keyboard focus left Run task 14px under the 73px header on the old code. Without a keyboard, the scroll padding now matches the 73px header; with a keyboard it stays 44px. **Tests (`propr-ui/e2e/creation-short-window.pw.ts`):** - **New windows:** two new web window sizes, 370×130 (header scrolls away) and 370×144 (header sticky), each covering both forms. They also check whether the header is sticky. - **On the old code:** all four fail. At 370×130 the header is still sticky and Cancel stays cut off by 13px. At 370×144 Run task and Start goal end up 14px under the header after keyboard focus. - **S1:** after wheeling back to the Prompt label and touch-dragging back to the repository, the test now checks that each one is fully in view and that a tap at its centre lands on it. - **Helper changes:** three were needed to drive these short windows: - Wheel steps now shrink as the target gets close, because 40px steps can't land a 44px button in a 45px gap. - Touch drags now stay inside the visible area. At 144 they started on the pane's bottom edge and didn't scroll. - The scroll starting point no longer goes off-screen once the header has scrolled away. **Results:** - **Browser suites:** all passed: - short-window, creation-keyboard, short-band and creation native-chrome: 62 - New Task, New Task layout, goals, dialog keyboard, Follow Up short band and task-dialog native-chrome: 42 - **Static checks:** eslint with zero warnings and tsc (which includes `e2e/`) both pass. I captured previews of both forms at 370×130 and 370×144. PR: #3256 Comment by: @integry (ID: 6260375728) Model: Claude Opus 5.5
|
✅ Applied the requested follow-up changes in commit 1acb20d Summary of ChangesI added your 370×345 → 370×130 boundary to the F1 regression as a new test. It fails on b23ee74 with the same clipping you measured, and passes on the current fix. Only the test file changed; the F1 fix in New test (
Results:
I didn't make new previews, because this change only adds a test and nothing on screen differs. The 370×130 and 370×144 previews from the previous commit still show the fix. The Gmail, Google Calendar, Google Drive and ProPR connectors in this session need authorising in your claude.ai connector settings before they can be used. None of them were needed for this change. 🤖 Implementation Details
Undo Changes • View Task Execution 💡 ProPR Slash Commands
🤖 Generated by ProPR |
|
/merge Please integrate current main (including the already reviewed and merged planner header fix124acbe17451373ebe984481956577ccd0c34aea) after the current evidence follow-up finishes. Preserve the F1/S1 fix and its negative controls. The separate evidence comment6260610810 documented the old b23 defect; eb98095 appears to address it and does not need a second redesign solely from that old-head evidence. Finish with one stable head for independent operator validation and a fresh Astra review. Do not merge this PR into main or publish anything. |
|
Executed correction follow-up on exact eb98095: the fixed7.5rem sticky threshold still assumes a one-line header and action. Enlarged text makes both wrap, so the actions remain obscured. Please address this within the existing shared creation-shell fix after current queued work, preserving the main integration and existing keyboard/native behavior. No separate overlapping writer is requested. Independent production build, pinned full Chromium151.0.7922.34, source-owned API fixtures, no real credentials/writes, keyboard closed, and real wheel input (up to50 measured corrections, each in the pane gutter):
I directly inspected both370x288 screenshots: Goal is almost entirely a sticky wrapped title with its action obscured; Task action is partly under its header. This is actual layout evidence, not a hypothetical or font-container-only test failure. All ten cases completed, source remained clean and exact, and no real API writes occurred. The first probe had a repository-catalog readiness error; v2 explicitly waited for repository setup and enabled submit before measuring, and retains those earlier operator results separately. Please make the sticky/scroll behavior accommodate the actual wrapped header and actions, including150/200% text at these widths/heights, rather than moving a single fixed threshold that assumes one-line contents. Keep the user-visible title, full action labels, readable prompts, native titlebar and actual keyboard/caret behavior; do not solve it by clipping labels or reducing user text. Add focused regression/negative-control evidence for the failing Goal and Task cases. Whole-button visibility/hit testing and return to prompt/repository remain required. Operator will retest the stable integrated head before fresh Astra review. Evidence retained locally at |
…n as a new test. It fails on b23ee74 with the same clipping you measured, and passes on the current fix. Only the test file changed; the F1 fix in `CreationDialog.tsx` is the same as the previous commit. Nothing was run on a native build or a real device. I added your 370×345 → 370×130 boundary to the F1 regression as a new test. It fails on b23ee74 with the same clipping you measured, and passes on the current fix. Only the test file changed; the F1 fix in `CreationDialog.tsx` is the same as the previous commit. Nothing was run on a native build or a real device. **New test** (`propr-ui/e2e/creation-short-window.pw.ts`, "web 370x345 resized to 370x130"). It runs for both New Task and Start a goal and follows your probe: - **Setup:** it opens the form at 370×345 with no keyboard and waits for the Prompt field to have focus. It then picks the acme/billing repository and types into the field key by key, and only goes on once the submit action is enabled. - **Resize:** it shrinks the window to 370×130. It checks that the dialog is 106px tall and its header 73px, the numbers from your probe. - **Scroll:** it sends 18 real 100px wheel notches in the dialog's side margin and checks that the dialog has reached its maximum scroll. - **What it checks:** that the whole 44px action is visible and takes a click at every sampled point, that the header ends above it, and that the header is not sticky. - **Then:** it wheels back up to the close button and checks that it is clear of the window controls, and that nothing was sent. **Results:** - **On b23ee74's `CreationDialog.tsx`:** both forms fail with the action 29px under the header, so 15 of its 44px show. That matches your executed evidence. - **On the current fix:** both pass. - **Keyboard and native-titlebar behaviour:** the short-window file and the creation keyboard, short band, dialog keyboard, creation native-chrome and task-dialog native-chrome suites all pass, 86 tests in total. - **Static checks:** eslint with zero warnings and tsc both pass. I didn't make new previews, because this change only adds a test and nothing on screen differs. The 370×130 and 370×144 previews from the previous commit still show the fix. The Gmail, Google Calendar, Google Drive and ProPR connectors in this session need authorising in your claude.ai connector settings before they can be used. None of them were needed for this change. PR: #3256 Comment by: @integry (ID: 6260610810) Model: Claude Opus 5.5
|
Warning This work may be incomplete. The agent stopped producing output and the stall watchdog ended the run before it could confirm that all requested work was complete. The available changes were committed instead of discarded. Last Agent UpdateThe related browser suites are running in the background. I'll check the result when they finish. Remaining WorkThe agent stopped before validating every request. Review this partial commit against the follow-up instructions and complete any unaddressed implementation, tests, or documentation. 🤖 Implementation Details
Undo Changes • View Task Execution 💡 ProPR Slash Commands
🤖 Generated by ProPR |
|
/merge After the current implementation follow-up6260790735 finishes, integrate latest main, now including the independently reviewed/tested diff fix756dabadcd22ec1ec072f89abeef04c51f336c26 (PR3254). Preserve the current layout correction and all its regressions. This is a new main update since the prior integration, and must not overlap the active writer. Keep integration bounded with relevant static checks; operator will independently test the final stable combined head and request fresh Astra review. Do not merge this PR into main or publish. |
…'ll check the result when they finish. The related browser suites are running in the background. I'll check the result when they finish. PR: #3256 Comment by: @integry (ID: 6260790735) Model: Claude Opus 5.5 Partial execution: The agent stopped producing output and the stall watchdog ended the run before it could confirm that all requested work was complete.
|
🔀 Auto-merged No conflicts were found — the merge was verified by an AI agent. 🤖 Verification Details
System-triggered merge conflict resolution |
…ude-opus-5-5-keep-the-full-new-task-su-20261019-0444-5kp
|
/review gpt-6-astra |
|
✅ AI Code Review Complete requested by @integry Posted 1 review: |
🔍 AI Code Review — gpt-6-astraOverall EvaluationThe layout fix appears sound, but the new regression suite contains an assertion that contradicts the intended behavior of taller windows. The PR needs a small test correction before merge. ✅ Content-aware header behavior — The shared shell measures the header and action heights, allowing the header to scroll away when wrapped controls cannot fit beneath it. ✅ Interaction-focused coverage — The added tests exercise wheel input, touch drags, action hit testing, draft preservation, and native chrome boundaries for both creation forms. This is a static review; no commands or tests were run. The supplied issue description is truncated, so assessment is limited to its visible requirements and the changed behavior. Current checks show no failures and six pending checks. Merge blockersEvery finding below was introduced by this PR and must be resolved before merging. F2: 🔴 Header assertion rejects valid taller layouts
SuggestionsThese are optional follow-ups and are not sent to S2: 🟢 Exercise actual Tab navigationThe new keyboard portion uses ScoreThe implementation directly addresses the clipping mechanism and adds substantial regression coverage. One deterministic contradiction in the new tests needs correction; current checks otherwise report no failures. Score: 6/10 🤖 Review Details
Copy and edit to choose records from this review: 🤖 Review by ProPR |
|
/fix F2 S2 Keep this correction focused on the tests. The content-aware production behavior should not be changed merely to make a header sticky in a taller layout. Preserve the explicit sticky/static boundary checks and whole action/caret/draft/repository assertions. Add real Tab or Shift+Tab traversal at a representative short geometry for S2. The operator's independent full creation/keyboard matrix and original ten-case enlarged-text probe are already running on2163d307fc0009bf86ff28e664a50a60d05a8f82. Please use bounded affected-case/static validation here rather than another long background suite that outlives the task; report precise completed commands/results. After the stable corrected head, operator will validate it and request fresh Astra review. No merge or publication from implementation. |
|
✅ Applied the requested follow-up changes in commit 2c19661
Summary of ChangesI fixed F2 and added the Tab traversal for S2, all within F2 (header assertion). The test required the header to be sticky exactly where an action fits beneath it. That wrongly fails taller layouts, where the header stays static even though an action would fit. It now only checks one direction: if the header is sticky, the tallest action must fit whole beneath it.
S2 (real Tab traversal). I added a test for both New Task and Start a goal at the issue's geometry (Linux window, zoomed to 370×345 at scale 2.378). It first confirms the header is sticky there, so the test really checks focus landing below a fixed header. Then, using only real Tab and Shift+Tab key presses:
Validation (all in
Between the cut-off full run and the follow-up run, every case in the file passed in local Chromium. No suite is still running in the background. Separate observation, not changed: other keyboard tests ( This change only touches tests, so there's nothing visual to preview. 🤖 Implementation Details
Undo Changes • View Task Execution 💡 ProPR Slash Commands
🤖 Generated by ProPR |
…e you listed. In my headless Chromium runs (stubbed backend, no credentials), new and existing plans showed no sideways scroll at 320, 390, 768, 842, 880, 1280 and 1390px with 100%, 150% and 200% text. All labels and controls are whole and on screen, and your scope-estimate fix is unchanged. The planner setup canvas now fits horizontally in every case you listed. In my headless Chromium runs (stubbed backend, no credentials), new and existing plans showed no sideways scroll at 320, 390, 768, 842, 880, 1280 and 1390px with 100%, 150% and 200% text. All labels and controls are whole and on screen, and your scope-estimate fix is unchanged. **What was overflowing besides the scope estimate:** - **Header:** the phase pill couldn't wrap, so with enlarged text it ran under the right pane and pushed the repo name out of view. The repo and branch now take the first line and the pill wraps under them. The wrapping is opt-in (`wrap` prop on `StudioPhaseSwitcher`), so the other studio headers render exactly as before. When I first changed the pill for every header, the execution-header test at 768px failed, which is why it's opt-in. - **Footer:** the row was forced onto one line. The model picker got squeezed and the Granular pill sat inside a hidden sideways scroller. This was already broken at normal text size: at 1280px/100% the 401 build cut the model name to 61 of its 144px. Break plan, model and Export/Generate now move to new lines when they don't fit. The model name wraps instead of being cut off, and the Generate label can wrap. - **Right pane:** "(optional)" now flows with its "Additional Context Repositories" title, and the long word breaks rather than running past the pane. - **Phone settings:** the "None indexed" picker drops under its "Context repos" label. The settings rows can wrap. The `propr/main` repo pill wraps before `/main` instead of being cut off. - **Context Refresh tooltip** (hover only): kept within the screen width. At 320/200% it used to open off the left edge. The 65/35 split between the panes is unchanged, and the test checks that. Nothing is hidden and no text is made smaller. Calculations, generation, permissions, drafts and the below-header error handling from #3255 are untouched, and no diff (#3254) or creation-dialog (#3256) files were changed. **Tests:** - **New `e2e/planner-whole-canvas.pw.ts`:** covers both modes at 100/150/200% across all seven widths. At each width it checks: - nothing in the document scrolls sideways. - no visible element reaches past its pane or the screen, and no text is cut short. - every label and control can be scrolled fully into view. - the prompt is kept and nothing is written. - **Model name:** each run waits for "Claude Opus 5.5 (Default)" to appear before measuring, so a loading spinner never counts as a layout result. - **Measuring after resize:** each width is measured only after the layout has switched between phone and desktop. Without that wait, the probe picked up a half-switched layout. - **Probe self-test:** a separate test injects an element too wide for its pane, a cut-off label and a sideways scroller, and checks that the probe reports all three. - **Exemption removed:** `planner-scope-estimate.pw.ts` now checks the whole canvas up to the document with enlarged text too. The shared setup moved to `e2e/planner-setup.fixture.ts`. **Proof the 401 layout fails:** I built 401b341 with one layout-neutral test id added so the probe could find the left pane, and ran the new specs against it: - All 6 whole-canvas layout tests failed, and so did all 4 enlarged-text scope tests. That includes the 390/200% phone case, which measured 390/404. - Measured canvas widths matched yours: 768/772 at 150%, 768/862 at 200%, 800/883 at 1280px/200%. With the fix, all 42 cases have no sideways scroll at all. **Final run:** - The 9 planner Playwright specs: 70 passed. - Vitest for `TaskPlanner` and `RepositorySelector`: 297 passed. - `tsc --noEmit` and `eslint --max-warnings 0` are clean. Nothing was run in WebKit, headless-shell or the native app. Two things outside the planner canvas, which I left alone: the bottom navigation cuts "Dashboard" at 320px even at 100% text, and the top-bar search box is squeezed at 200%. The previews show before and after at 768px and 1280px with 200% text, and at 390px on a phone. PR: #3258 Comment by: @integry (ID: 6261145049) Model: Claude Opus 5.5
…propr-ui/e2e/creation-short-window.pw.ts`. Production code is unchanged, including `CreationDialog.tsx`. I fixed F2 and added the Tab traversal for S2, all within `propr-ui/e2e/creation-short-window.pw.ts`. Production code is unchanged, including `CreationDialog.tsx`. **F2 (header assertion).** The test required the header to be sticky exactly where an action fits beneath it. That wrongly fails taller layouts, where the header stays static even though an action would fit. It now only checks one direction: if the header is sticky, the tallest action must fit whole beneath it. - The explicit sticky/static boundary cases are unchanged (`web 370x130` scrolls, `web 370x144` sticks, and the 370×345→370×130 resize stays static). - The whole-action, caret, draft, repository and "nothing sent" assertions are unchanged. - I checked the other e2e files, and no other test makes the same sticky-equals-fits assumption. **S2 (real Tab traversal).** I added a test for both New Task and Start a goal at the issue's geometry (Linux window, zoomed to 370×345 at scale 2.378). It first confirms the header is sticky there, so the test really checks focus landing below a fixed header. Then, using only real Tab and Shift+Tab key presses: - Tab from the prompt reaches each action in order, and each one scrolls wholly into view and takes a press. - Tab past the last control wraps to the close button, which is in view and clear of the window's title bar. - Shift+Tab from the close button wraps back to the submit action, which is whole again. - Shift+Tab back through the actions returns to the prompt, and the caret line is in view. - The draft and selected repository are kept, the window controls stay usable, and no request other than GETs was sent. **Validation (all in `propr-ui`, all finished):** | Command | Result | |---|---| | `npx playwright test e2e/creation-short-window.pw.ts -g "Tab traversal\|web 370x345: \|web 260x420"` | 6 passed (38s) | | Same four `web 370x345` / `web 260x420` cases on HEAD's copy of the test file | 4 failed, at the old equality assertion (sticky was false, fits was true), which confirms F2 | | `timeout 540 npx playwright test e2e/creation-short-window.pw.ts` | Stopped by my 540s limit (exit 124) after 21 cases, all passed, none failed. The enlarged-text cases take about 40–46s each. | | `-g "web 370x345 200% text: Start a goal\|resized to 370x130\|Tab traversal"` (the cases not reached above) | 5 passed (58s) | | `-g "Tab traversal"` after adding the sticky-header check | 2 passed (8.5s) | | `npx eslint --max-warnings 0 e2e/creation-short-window.pw.ts` | exit 0 | | `npm run typecheck` (`tsc --noEmit`) | exit 0 | Between the cut-off full run and the follow-up run, every case in the file passed in local Chromium. No suite is still running in the background. **Separate observation, not changed:** other keyboard tests (`creation-keyboard-viewport.pw.ts`, `dialog-keyboard-viewport.pw.ts`) also call `.focus()` directly. That's outside this fix. This change only touches tests, so there's nothing visual to preview. PR: #3256 Comment by: @integry (ID: 6261494561) Model: Claude Opus 5.5
|
/review gpt-6-astra |
|
✅ AI Code Review Complete requested by @integry Posted 1 review: |
🔍 AI Code Review — gpt-6-astraOverall EvaluationThe PR addresses short-window action clipping by allowing the pane to scroll and measuring whether the header leaves enough room for the tallest action. No merge-blocking defect was identified in the supplied diff. It is merge-ready within the reviewed scope, conditional on pending checks completing successfully. ✅ Content-aware header behavior — The shared shell measures actual header and action heights, accounts for enlarged-text wrapping, and disconnects its observer on unmount. ✅ Meaningful regression assertions — Tests retain explicit sticky/static boundaries, whole-action geometry and hit testing, caret visibility, draft/repository preservation, and native-chrome checks. The added Tab/Shift+Tab cases exercise sequential navigation and focus wrapping. This was a static review; no commands or tests were run. The original issue body is explicitly truncated, so acceptance details beyond the supplied text could not be assessed. The supplied diff was reviewed as complete. Merge blockersNo merge blockers. SuggestionsThese are optional follow-ups and are not sent to No suggestions. ScoreThe implementation is focused and backed by substantial regression coverage. The supplied current-head status reports no failures, with six checks still pending; runtime results were not independently verified in this review. Score: 8/10 🤖 Review Details
🤖 Review by ProPR |
|
/review gpt-6-astra |
|
✅ AI Code Review Complete requested by @integry Posted 1 review: |
🔍 AI Code Review — gpt-6-astraOverall EvaluationThe PR addresses short-window submit clipping through whole-pane scrolling and a measured sticky-header fit check. No merge-blocking defect is demonstrated by the supplied code. It is merge-ready within the reviewed scope, conditional on pending checks completing successfully. ✅ Content-aware header behavior — ✅ Meaningful interaction coverage — The added tests exercise real wheel input, Chromium touch drags, and Tab/Shift+Tab traversal while checking whole-action hit targets, caret visibility, draft preservation, and native chrome. ✅ Font-independent wrapping baseline — The log test uses the longer conversation filename to establish wrapping and verifies its single-line keyboard layout and restored wrapping afterward. This was a static review; no commands or tests were run. The supplied issue description is truncated, so requirements beyond its visible portion could not be assessed. The authoritative checks report no failures and 11 pending checks. Merge blockersNo merge blockers. SuggestionsThese are optional follow-ups and are not sent to S3: 🟢 Make font coverage explicitIn ScoreThe production change is focused and supported by substantial interaction and boundary coverage. Static review found no actionable regression; runtime qualification remains conditional on the pending checks. Score: 8/10 🤖 Review Details
Copy and edit to choose records from this review: 🤖 Review by ProPR |
|
CI failed: Full Test Suite Shard 4/4 Please investigate and fix this CI failure.
Failure evidence
|
|
✅ Applied the requested follow-up changes in commit 7dbb612 Summary of ChangesI fixed the Shard 4/4 failure. It was a flaw in a CI test that leaked state from the running job, not something your UI change caused. Only Cause. Shard 4 had one failure among its 228 test runs, in The classifier also reads the pull request's labels from the running job's event file ( Fix. In Checks I ran:
I didn't re-run the full shard. This change has nothing visible, so there are no previews. 🤖 Implementation Details
Undo Changes • View Task Execution 💡 ProPR Slash Commands
🤖 Generated by ProPR |
…hat leaked state from the running job, not something your UI change caused. Only `test/ciFullSuiteSelection.test.mjs` changed. I fixed the Shard 4/4 failure. It was a flaw in a CI test that leaked state from the running job, not something your UI change caused. Only `test/ciFullSuiteSelection.test.mjs` changed. **Cause.** Shard 4 had one failure among its 228 test runs, in `test/ciFullSuiteSelection.test.mjs`. The test "PR #2513 runs all four backend shards and skips docs and native Electron" builds a small throwaway git repo with only review-job files. It then runs the change classifier on it, which decides which CI checks a change needs, and expects the desktop checks to be off. In CI, it reported them as on. The classifier also reads the pull request's labels from the running job's event file (`GITHUB_EVENT_PATH`), and the test passed the job's whole environment through. This PR got the `desktop-ci` label at 07:54, before this run. So the label switched on the desktop checks inside the test's throwaway repo too. That is why it failed only in this PR's CI and passed locally. **Fix.** In `classifyWithCli` (`test/ciFullSuiteSelection.test.mjs:229`), the test now removes `GITHUB_EVENT_PATH` before starting the classifier, so the CI job's labels no longer leak into its result. The classifier, the workflow and all product code are unchanged. **Checks I ran:** - **Reproduced the failure:** I ran the test with a fake event file carrying a `desktop-ci` label. Before the fix: 77 pass, 1 fail (the same test that failed in CI). - **After the fix:** 78/78 pass with that fake event file, and 78/78 without it. - **Lint:** `npx eslint --max-warnings 0 test/ciFullSuiteSelection.test.mjs` passes. I didn't re-run the full shard. This change has nothing visible, so there are no previews. PR: #3256 Comment by: @propr-dev[bot] (ID: 6263344092) Model: Claude Opus 5.5
|
/review gpt-6-astra |
|
✅ AI Code Review Complete requested by @integry Posted 1 review: |
🔍 AI Code Review — gpt-6-astraOverall EvaluationThe PR addresses short-window clipping by allowing the creation pane to scroll and disabling sticky headers when measured content leaves insufficient room for a complete action. It is merge-ready within the reviewed scope, conditional on the pending checks completing successfully. ✅ Content-aware header behavior — ✅ Meaningful interaction coverage — The new regressions cover both creation forms, enlarged text, native chrome, wheel and touch scrolling, real Tab traversal, whole-action hit testing, and draft preservation. ✅ Improved wrapping baseline — The Logs regression now uses the longer conversation filename to establish wrapping and checks restoration after keyboard dismissal, including explicitly blocked web fonts. This was a static review of the supplied code; no commands or tests were run. The original issue excerpt is truncated, so requirements beyond its visible contents could not be assessed. The supplied current status reports no failures and seven pending checks. Merge blockersNo merge blockers. SuggestionsThese are optional follow-ups and are not sent to S4: 🟢 Make font coverage deterministicIn ScoreThe changed layout logic and regression coverage address the visible objective without a verified introduced defect. Pending checks still need to finish; runtime behavior was not independently executed during this review. Score: 8/10 🤖 Review Details
Copy and edit to choose records from this review: 🤖 Review by ProPR |
|
CI failed: Full Test Suite Shard 1/4 Please investigate and fix this CI failure.
Failure evidence
|
|
✅ Applied the requested follow-up changes in commit 3a58c46 Summary of ChangesI fixed the Shard 1/4 failure. It's the same CI test flaw as the earlier Shard 4/4 fix, this time in a second test file. Your UI change didn't cause it, and the only file changed is Cause. Shard 1 had one failure. In The test runs the classifier as a separate process with the CI job's environment. It already removed Fix. I added The only other test that starts the classifier is Checks I ran:
I didn't re-run the full shard. Nothing in this change is visible, so there are no previews. 🤖 Implementation Details
Undo Changes • View Task Execution 💡 ProPR Slash Commands
🤖 Generated by ProPR |
…s the earlier Shard 4/4 fix, this time in a second test file. Your UI change didn't cause it, and the only file changed is `test/ciChangeClassification.test.mjs`. I fixed the Shard 1/4 failure. It's the same CI test flaw as the earlier Shard 4/4 fix, this time in a second test file. Your UI change didn't cause it, and the only file changed is `test/ciChangeClassification.test.mjs`. **Cause.** Shard 1 had one failure. In `test/ciChangeClassification.test.mjs`, the test "writes one output per surface, the file lists and a readable summary" (line 640) changes one API file. It then expects the change classifier (the script that decides which CI checks a change needs) to leave the desktop checks off. In CI it switched them on (`'true' !== 'false'` at line 650). The test runs the classifier as a separate process with the CI job's environment. It already removed `GITHUB_EVENT_NAME` and `PROPR_CLASSIFY_*` from that environment, but not `GITHUB_EVENT_PATH`. That variable points to the job's event file, and the classifier reads pull request labels from it. This PR has the `desktop-ci` label, so the label switched on the desktop checks inside the test's throwaway repo. **Fix.** I added `GITHUB_EVENT_PATH` to the list of variables the test removes before starting the classifier (the `AMBIENT` pattern near line 614), and updated the comment above it. The classifier, the workflows and the product code are unchanged. The only other test that starts the classifier is `test/ciFullSuiteSelection.test.mjs`, which the earlier commit already fixed. The other CI tests start their processes with a minimal environment, so they can't pick up the label. **Checks I ran:** - **Reproduced the failure:** with a fake event file carrying a `desktop-ci` label, the unfixed file gives 95 pass, 1 fail (the same test that failed in CI). - **After the fix:** 96/96 pass with that event file, and 96/96 without it. - **Lint:** `npx eslint --max-warnings 0 test/ciChangeClassification.test.mjs` passes. I didn't re-run the full shard. Nothing in this change is visible, so there are no previews. PR: #3256 Comment by: @propr-dev[bot] (ID: 6263601181) Model: Claude Opus 5.5
|
/review gpt-6-astra |
integry
changed the title
|
Please correct the regression input helper in Independent full Chromium validation on combined checkout I independently reproduced the cause with the same immutable production UI and source-owned API fixture, in a separate full Chromium151 container. I replayed the public Inter CSS/woff2 already captured in the original failed trace and held the font until after the helper calculated its gutter point. Initially the inner form's visible vertical band is138..255 and the fixed pointer is abouty235. After Inter loads, the action footer wraps and the form band shrinks to138..234. The old pointer is now over the footer, not the label's scroll frame. All40 original trusted upward wheel notches, and100 additional trusted notches at that same point, leave inner scrollTop436 and Prompt top-304 unchanged. Merely increasing the loop count cannot correct that setup. Moving the real pointer to(21,186), inside the visible form scroll frame, then eight trusted upward wheel notches brings the whole Prompt label into view. There is no DOM scrollTop assignment, Please recompute input geometry against the current visible scroll frame appropriate to the target while scrolling, rather than caching a point that can become a footer hit as layout changes. Retain the whole-label/repository hit tests, whole44px action/40px Close checks, draft/repository/caret preservation, real wheel/touch input, and actual Tab traversal. Add a deterministic layout/font-transition guard using the existing test infrastructure; do not fix this by skipping cases, weakening assertions, a blanket sleep, or an increased notch budget alone. Verify the existing native, short-band, enlarged-text, and shared Goal cases too. Local diagnosis is |
|
✅ Applied the requested follow-up changes in commit 5aee3af Summary of ChangesI fixed the stale wheel point in the regression helper and added a guard that changes the layout mid-scroll. Only Cause. The old helper picked one point in the pane's side gutter and reused it for every notch. If the layout changed after that, for example the actions wrapping when Inter loads, the point could end up over the action footer. Wheeling there never scrolls the form body, so more notches can't help. Helper changes
New guard ("web 370x345 late font", New Task and Start a goal)
All whole-label and repository hit tests, the 44px action and 40px Close checks, the real wheel, touch and Tab input, and the sticky/static checks are unchanged. Nothing was skipped, slowed down or loosened. Validation (local production build, bundled Playwright Chromium; Inter loaded in the default runs)
I couldn't reproduce your exact geometry with real Inter. In this container Inter loads early, and the actions already wrap before the action notches, so the old helper happened to pass. That's why the guard uses a deterministic style change instead of timing a real font load. Running it on your Chromium 151 setup would confirm it there. There are no preview images because the change is test-only. 🤖 Implementation Details
Undo Changes • View Task Execution 💡 ProPR Slash Commands
🤖 Generated by ProPR |
…added a guard that changes the layout mid-scroll. Only `propr-ui/e2e/creation-short-window.pw.ts` changed. Production code is untouched, and nothing showed a product defect.
I fixed the stale wheel point in the regression helper and added a guard that changes the layout mid-scroll. Only `propr-ui/e2e/creation-short-window.pw.ts` changed. Production code is untouched, and nothing showed a product defect.
**Cause.** The old helper picked one point in the pane's side gutter and reused it for every notch. If the layout changed after that, for example the actions wrapping when Inter loads, the point could end up over the action footer. Wheeling there never scrolls the form body, so more notches can't help.
**Helper changes**
- **Recomputed every step:** a new `scrollPoint(target, up)` runs before every wheel notch and touch drag. It finds the innermost frame around the target that can still scroll that way (the form body, otherwise the whole pane).
- **Hit-tested:** it takes the part of that frame the reader can see, below any sticky header, and searches outward from the middle. It only accepts a point that hit-tests inside that frame and isn't over a control or a nested scroller, so a footer outside the body is never used. If no point qualifies it stops, and the existing visibility assertions report the failure.
- `wheelToShow` and `dragToShow` now use it. So does the "resized to 370x130" test, which still runs up to 18 notches and asserts it reached the end. The notch budgets are unchanged.
- The draft and repository setup, repeated in three tests, is now one `writeDraft` helper. This keeps the file under the 400-line lint limit that CI enforces with zero warnings.
**New guard ("web 370x345 late font", New Task and Start a goal)**
- **Why styles, not fonts:** which fonts load, and how wide they set, differs by host. So the guard sets the layout with stylesheets instead.
- **Before:** a stylesheet keeps the actions on one row.
- **Transition:** after `wheelToShow` has moved the pointer for its first notch toward Prompt, and before the notch, a late stylesheet stands in for Inter arriving. It makes the header 7.5rem taller and spaces out the action labels until they wrap.
- **Checks that the transition really happened:** the pointer started over the body, the footer grew, the body is no longer under the pointer, and the pane has nothing above to scroll to.
- **After:** Prompt must come wholly into view. Then the same checks as the main test run again: every whole action by wheel, touch to the repository and Run task, keyboard focus on Run task, the caret, draft and repository unchanged, and no writes.
- **Negative control:** I temporarily changed `wheelToShow` back to keeping its first point. The guard then failed with `Prompt top = -304` (New Task) and `-284` (Start a goal), matching your failure. I then restored the fix.
All whole-label and repository hit tests, the 44px action and 40px Close checks, the real wheel, touch and Tab input, and the sticky/static checks are unchanged. Nothing was skipped, slowed down or loosened.
**Validation** (local production build, bundled Playwright Chromium; Inter loaded in the default runs)
- `tsc --noEmit` and `eslint --max-warnings 0` on the file both pass.
- The full file (28 tests: native, short-band, enlarged-text, Goal, resize, Tab traversal and the new guard) passed 28/28 with the container's default fonts. A run of the earlier version also passed 28/28 in one go.
- It passed 28/28 with an Arial-metric fallback (Liberation-only fontconfig) standing in for your hosts' fallback.
- The guard alone passed 2/2 under both font setups.
I couldn't reproduce your exact geometry with real Inter. In this container Inter loads early, and the actions already wrap before the action notches, so the old helper happened to pass. That's why the guard uses a deterministic style change instead of timing a real font load. Running it on your Chromium 151 setup would confirm it there.
There are no preview images because the change is test-only.
PR: #3256
Comment by: @integry (ID: 6264709582)
Model: Claude Opus 5.5
|
/review gpt-6-astra |
|
✅ AI Code Review Complete requested by @integry Posted 1 review: |
🔍 AI Code Review — gpt-6-astraOverall EvaluationThe PR addresses short-window clipping by allowing the complete creation pane to scroll and measuring whether the header leaves enough space for a whole action. The implementation appears merge-ready within the supplied scope, conditional on pending checks completing successfully. ✅ Content-aware header behavior — ✅ Reachability assertions preserved — The new regression suite checks whole-action geometry and hit testing, caret visibility, draft preservation, native chrome clearance, and real wheel, touch, and Tab/Shift+Tab navigation. ✅ Late-layout recovery covered — The input helper recalculates its scroll target before each input iteration, and the added regression deliberately moves the body after pointer placement while retaining the reachability assertions. This was a static review of the supplied diff and context; no commands or tests were run. The linked issue text is explicitly truncated, so requirements beyond its visible portion could not be assessed. The supplied current-head status reports no failures and 16 pending checks. Merge blockersNo merge blockers. SuggestionsThese are optional follow-ups and are not sent to S5: 🟢 Make loaded-font coverage explicitIn ScoreThe scoped layout correction and regression assertions are strong, with no verified code blocker. Pending checks still need to finish; runtime behavior was not independently executed during this review. Score: 9/10 🤖 Review Details
Copy and edit to choose records from this review: 🤖 Review by ProPR |
|
Please investigate and correct two newly observed independent failures at head The actual checkout is GitHub's proposed merge Environment:
Original traces, error contexts and last rendered frames are preserved locally under If the cause is test setup or event stabilization, make the necessary state explicit while retaining the deterministic late-layout test and a meaningful negative control. Do not skip cases, weaken whole-area/caret assertions, reduce target sizes, remove input methods, increase budgets alone, or add a blanket sleep. If legitimate user input instead exposes an actual product reachability defect, demonstrate it and fix the shared product path. Keep the change scoped to what the evidence requires. Astra's static 9/10 review is not being used to override these executed failures. No merge, publication, permission grant, or runtime deployment is requested by this comment. |
|
Warning This work may be incomplete. The agent stopped producing output and the stall watchdog ended the run before it could confirm that all requested work was complete. The available changes were committed instead of discarded. Last Agent UpdateWaiting for the repeat and headless-shell runs. Remaining WorkThe agent stopped before validating every request. Review this partial commit against the follow-up instructions and complete any unaddressed implementation, tests, or documentation. 🤖 Implementation Details
Undo Changes • View Task Execution 💡 ProPR Slash Commands
🤖 Generated by ProPR |
|
At partial commit 0488886, the operator inspected the preserved change against 5aee3af: only the creation test/helper and two temporary Playwright configs changed; no product implementation files changed. The previous task is terminal (watchdog partial handoff); its container is gone. Please complete this narrowly scoped cleanup: remove The operator observed full Chromium 28/28, WebKit 28/28, and the originally failing WebKit case 10/10 in the task before termination. The final headless-shell run was still live when the watchdog ended the task; do not report it as passed. No need to repeat long browser runs in this cleanup: the operator will independently execute the complete affected inventory across all three engines on the finalized head and obtain a fresh Astra review. A brief diff/cleanliness check is sufficient for this cleanup; let ProPR finalize the commit. |
|
✅ Applied the requested follow-up changes in commit c54e080 Summary of ChangesI removed the two temporary Playwright configs, Check against 5aee3af: the PR now differs from it in only two files:
Both are exactly as they were at 0488886, so the new fixture, its test integration, all the original geometry, hit-test, caret, draft and keyboard assertions, and the stronger caret end-selection check are untouched. No product files, skips, retries or other temporary files were added, and nothing in the repo referenced the removed configs.
As you asked, I didn't run any browser tests. The headless-shell run that was still going when the watchdog ended the last task never finished, so it shouldn't be counted as passing. There are no previews because nothing visible changed. 🤖 Implementation Details
Undo Changes • View Task Execution 💡 ProPR Slash Commands
🤖 Generated by ProPR |
…w-full.tmp.config.ts` and `propr-ui/pw-webkit.tmp.config.ts`. Nothing else changed. I removed the two temporary Playwright configs, `propr-ui/pw-full.tmp.config.ts` and `propr-ui/pw-webkit.tmp.config.ts`. Nothing else changed. **Check against 5aee3af:** the PR now differs from it in only two files: - `propr-ui/e2e/creation-scroll.fixture.ts` (new, 154 lines) - `propr-ui/e2e/creation-short-window.pw.ts` Both are exactly as they were at 0488886, so the new fixture, its test integration, all the original geometry, hit-test, caret, draft and keyboard assertions, and the stronger caret end-selection check are untouched. No product files, skips, retries or other temporary files were added, and nothing in the repo referenced the removed configs. `git rm` couldn't write the git index because the worktree's git directory is read-only here. I deleted the files from the working tree instead, so `git status` shows them as deleted for ProPR to pick up when it finalizes the commit. As you asked, I didn't run any browser tests. The headless-shell run that was still going when the watchdog ended the last task never finished, so it shouldn't be counted as passing. There are no previews because nothing visible changed. PR: #3256 Comment by: @integry (ID: 6267310357) Model: Claude Opus 5.5
|
/review gpt-6-astra |
|
✅ AI Code Review Complete requested by @integry Posted 1 review: |
🔍 AI Code Review — gpt-6-astraOverall EvaluationThe PR addresses clipped creation actions by allowing the entire pane to scroll and disabling sticky headers when measured content cannot fit. No merge-blocking defect is established by the supplied code; it is merge-ready within the reviewed scope, conditional on pending checks completing successfully. ✅ Content-aware header behavior — The shell measures the header and tallest action, accounts for wrapping, and disconnects its resize observer on unmount. ✅ Meaningful reachability coverage — The new tests check whole-control geometry and hit targets, real wheel/touch input, Tab traversal, caret end selection, and draft preservation across short and enlarged layouts. ✅ Isolated CI fixtures — Both classifier test helpers exclude the host event payload, preventing PR labels from changing fixture decisions. This was a static review; no commands or tests were run. The supplied issue description is truncated, so unseen acceptance details could not be assessed. Current checks report zero failures and 22 pending checks. Merge blockersNo merge blockers. SuggestionsThese are optional follow-ups and are not sent to S6: 🟢 Clarify font coverage labelsIn ScoreThe implementation is localized and supported by substantial regression coverage, with no verified code blocker. Merge readiness remains conditional on the pending checks; historical results were not treated as current-head validation. Score: 9/10 🤖 Review Details
Copy and edit to choose records from this review: 🤖 Review by ProPR |
1 participant
New Task could cut off the bottom of Run task in a short, enlarged Linux window. New Task and Start a goal now scroll the complete creation pane when needed. The shell measures its header and tallest action and lets the header scroll away when they cannot fit together, preserving prompt space, 40px close controls, and 44px action controls.
Regression tests exercise trusted wheel/touch input, Tab/Shift+Tab, whole-control bounds and hit targets, caret end selection, and unsent draft retention across short windows, resizing, enlarged text, and late layout changes. The shared test helper waits for scrolling, fonts, and layout to settle before measuring. Logs wrapping fixtures support Inter and fallback fonts. Two CI fixture helpers exclude the host GitHub event payload so PR labels cannot alter their expected classifications.
Validation for head
c54e080and proposed combined sourced386cd4(main366824a):d386cd4checkout/tree were verified. The current Linux package CI job and independently verified runtime/packaging audits also passed.Operator validation of actual signed native behavior remains the premerge gate. Final signed distribution, installation cycles, live task/access/attribution checks, and integrated mobile release acceptance remain separate release gates.
Closes #3252
The original reported-geometry previews below illustrate the initial layout correction; current boundary and device validation is described above.
Visual preview
Start a goal at the same geometry
The shared creation shell for New Goal at the same zoomed window, scrolled by wheel to its actions.
Original unavailable: managed storage is unavailable or the upload has expired.
New Task at the zoomed 370×345 Linux window
Linux desktop shell zoomed to 370×345 CSS px (DPR 2.378), after a real mouse-wheel scroll down to the actions: the whole 44px Run task button sits inside the dialog under its sticky header, and the native titlebar stays clear.
Original unavailable: managed storage is unavailable or the upload has expired.
Operator signed-native premerge validation: passed on 2026-10-19. The private RPM is signed by replacement key
80FE0A7667A92A0AD3CEAE6D4C2FF8C3CC4E1599; both native signatures were independently verified using only that public key. All 78 native file bytes and modes match tested proposed sourced386cd4(prepared package source7eb455a). Actual normally sandboxed native Task/Goal controls passed trusted scrolling, whole-target/nine-point hit tests and keyboard return at 370×345 with native zoom. Drafts remained unsubmitted and original zoom/window geometry were restored; screenshots were directly inspected. This satisfies the retained signed-native premerge gate. Final source freeze, installed-package cycles, intended-user live tests, authenticated mobile checks and official signed publication remain separate release gates.