feat(image): add resolution budget presets to image tools - #1
Closed
YodonTan wants to merge 253 commits into
Closed
Conversation
docs: tighten promotion claims and onboarding
…ndex-recovery fix(session): recover corrupt session indexes
…urity docs: expose quick start, security, and social preview
…guide docs: explain how to verify release downloads
docs: replace unverifiable vibe-coding percentage
Review child sessions are created with agentType=CodeReview (standard) or DeepReview (strict), but resolve_primary_agent_for_turn only accepted Mode-category agents, so session creation failed with "Unknown session mode: CodeReview" and persisted review sessions could be silently rewritten to agentic on restore. Allow the builtin CodeReview/DeepReview agents to resolve as local session primaries while keeping all other subagents restricted, and add diagnostics at the rejection point. Regression introduced by ca94825.
…t ids Introduce CODE_REVIEW_AGENT_TYPE in agent-runtime deep_review constants and use it in registry review-entry detection and session-primary resolution, replacing the scattered "CodeReview" magic string. This keeps review agent ids consistent with the existing DEEP_REVIEW_AGENT_TYPE / REVIEW_JUDGE_AGENT_TYPE constants. This cleanup also served as the verification content for the /review session fix in this PR: it was written into the workspace and reviewed end-to-end with the /review command after installing the fixed build, confirming review sessions now create and run correctly.
…n primaries resolve_primary_agent_for_turn still filtered the ExternalSubagentRoute::Local branch to AgentCategory::Mode, so workspaces whose route table pins CodeReview/DeepReview to the local implementation (same-name conflict resolved to the local candidate) could still fail to create, restore, or start review sessions. Extract a shared is_local_session_primary_entry predicate used by both the explicit Local-route branch and the no-route fallback, and add a diagnostic warn when a registered entry is rejected under a Local route.
- Remove deprecated reasoning fields from the installer AIConfig mapping. - Add a Windows CI compilation check for the standalone installer crate. - Extend GitHub workflow contract tests to enforce the installer check.
add agent/--model/--session
feat(agent): read documents with anydoc
Stop embedding the docx, pdf, pptx, and xlsx skill bundles, clean legacy loose installs, and keep the supported ppt-design workflow. Also restore the Superpowers MIT notice for the retained writing-skills content while dropping its vendored Anthropic documentation copy.
…ltin-skills fix(skills): remove redistribution-restricted built-ins
…-pdf fix(ppt-live): inherit model and preserve PDF text
…on-startup fix(flow-chat): persist exact last request usage for startup display
feat(harmonyos): refine adaptive chat and remote workflows
Three follow-ups to the WebKit lookbehind fix: - Declare MEditorErrorBoundary after the fallback it renders. ESLint's no-use-before-define was failing the web lint job. - Size the fallback textarea. It is a direct flex item of a column container, so `height: 100%` resolved to `auto` whenever the host passed `height="auto"` -- as PlanViewer does -- collapsing the recovered document to the default two rows. It now grows into a definite-height container and falls back to content-sized `rows` otherwise. - Run scripts/verify-webkit-compatibility.test.mjs. Nothing invoked it: both `node --test` steps in ci.yml pass explicit file lists. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…-lookbehind fix(web-ui): support Markdown on older WebKit
The calendar opened but never dismissed after picking a date. It was the browser's own picker, opened with `showPicker()` on a zero-size, `pointer-events: none`, `aria-hidden` anchor input. Chromium ties that popup's open/close lifecycle to a real, hit-testable anchor, so with nothing to return to it stayed on screen. Render the grid in-app instead. A React popover has a dismissal path we own rather than one we have to coax out of a native widget, and it picks up the app's locale and theme for free. It closes on selection, on Escape, on an outside click, and on toggling the button again. Scope stays the date: picking a day keeps the time already in the field rather than rewinding it to midnight, and the text field remains the way to set the time, which is quicker to type than to click.
The popover used `element-bg-subtle`, an element background meant to sit inside a panel and blend with it. Floating over the form it let the fields behind read straight through the grid, so the day numbers collided with the prompt counter and the buttons underneath. Use `color-bg-elevated` with the same border, shadow and stacking the nav workspace menu uses. That token resolves to an opaque colour in every builtin appearance, so the surface stays readable wherever it opens.
…signing ci: sign and notarize macOS releases
…path fix(worktree): normalize Windows managed paths
… rule
Reported as: open a 12-Turn session, switch away, switch back — and the
transcript comes up with Turn 5 pinned to the top instead of at the end
where it was left.
Offset 0 of a history window starting at ordinal 4 *is* Turn 5 at the
top, and the window was one nobody asked for. The list is keyed on the
session, so switching back is a full remount: fresh scroller at offset 0,
empty measurement cache, opening reveal re-armed. 144ms into that reveal,
with the viewport still at 0, the head boundary read as reached — a tail
of three Turns is shorter than one viewport — and paged. The first visit
had taken 674ms to load that window and landed after the reveal had
settled; the second found it cached and answered in 6ms, which is the
whole difference between the two.
What arrived was 22301px of history above a viewport that had not been
placed yet. The compensation for that is deliberately a shift rather than
a write, refused only by an owner that holds a target — and it was
refused, `heldBy: follow-output`. Follow-output was not going to act:
followOutput.enter session-open, viewportId 3, scrollTop 0
followOutput.frameStoodDown not-following, settleFrames 90
prependCompensated shiftedPx 22301, followTargetPx null
openReveal settled false, frames 40, scrollTop 0
The loop stood down on its first frame with its whole 90-frame budget
untouched, and there is no `followOutput.exit` anywhere in the trail
because nothing exited. `isFollowingOutputRef` was assigned from the
`isFollowingOutput` state on every render. Ownership is written
imperatively by enter and exit; the state is only how the rest of the
component hears about it. Between the two, React is free to render the
value from before the update — an update scheduled from a passive effect
sits below a synchronous render — and session-open enters from a mount
effect, so it hits this every time. The register still recorded
follow-output as the holder, which is the state nothing recovers from:
one side says the viewport is owned, the other says it is not following.
The other two entries above are the same open, one and two switches
earlier. Both were clobbered too; both were rescued by a resize callback
restarting the loop before it mattered.
Three fixes, independent on purpose — each one alone leaves the viewport
at the end of the transcript:
- The mirror is gone. Every other ref on that line is a prop, which is
consistent with the render that assigned it by construction; this one
had two writers.
- A refused shift now wakes whoever it was left to. Follow-output is the
only holder that can be asleep — its ownership outlives its frame loop
so streaming can resume without re-entering, and the loop stops once
the transcript settles — so a page landing after that was left to a
writer that would never act, with or without the clobber above.
- Paging is refused while the transcript is still being placed, and the
reveal settling asks again. At offset 0 of an unplaced viewport the head
is trivially reached, and what the page then does is prepend history
above that same placement. Deferred rather than dropped: the boundary
evaluation now re-runs when the reveal ends, which is the only thing a
session shorter than its viewport can wait for — it emits no scroll
events, having nowhere to scroll.
The trail is the other half of this. It said `followOutput.enter` and
then nothing, and the six lines that would have named the cause were all
absent by construction: an entry declined for an inactive viewport
returned silently, an exit was traced only when it had something to give
up, a frame loop that stood down said nothing at all, and no line carried
which transcript it came from. All of those are recorded now, behind the
existing `app.logging.flow_chat_diagnostics` switch — `traceViewport`
returns before evaluating its payload and `traceViewportRepeating` checks
first, and the one probe on the per-frame path guards its own key the way
the register's write already does.
Verified: 198 files / 1660 tests, type-check and lint clean. Each fix has
a test that fails without it — the clobber's reproduces the priority
window with `flushSync`, and the paging tests now drive the reveal to its
end the way half a second of animation frames does in the app.
Two faults on the same correction, found one behind the other by fixing
the first: a reader who paged history in from a short tail was left
sitting in the reserved blank, and once that was fixed, left 364px short
of the content end with a Turn cut off at the top of the viewport.
**The snap back gave up on one refusal.**
It asks from inside the window it is least likely to be granted. The
gesture's claim is a lapse timer — a wheel has no end of its own, so
`notifyUserScrollIntent` holds the viewport for 200ms past each notch —
and this correction is triggered *by* coming to rest, which is inside
that window by construction. Measured: 22301px of history arrived above
a viewport at offset 0, the compensation was refused as it should be
("left the displacement to whoever holds a target"), the real heights
then landed ~1670px shorter, the browser clamped the offset to the end of
the shrunken range, and the snap back that exists for exactly this was
refused `heldBy: user-gesture` 57ms into the hold. Nothing asked again.
The reader kept 841px of blank — the whole spacer — for the four minutes
the recording lasted.
Releasing the hold on the settle was the first attempt and was wrong: a
settle also lands *between* two notches of a gesture still in progress.
Measured on a session of short Turns, a dozen times over: the snap fired
110ms after a notch, the reader wheeled again 40ms later, and the
transcript stuttered under their hands every 800ms.
So the hold stands and the ask repeats. A reader still scrolling refuses
every retry and moves nothing, and their own next settle re-arms the
whole thing; a reader who has stopped is snapped back one window after
their last notch. Five attempts covers the case the retry is actually
for — the reader has stopped while something else is still moving the
transcript under them, where no further scroll event is coming.
**And then it resumed on the offset the animation stopped at.**
Which is not the offset the follow rule owns, and the two differ for the
same reason the snap was needed: a history page whose items are still
measuring. `enterFollowOutput('tail-snap-back')` took `scrollTop` as the
hold rule's memory, and the hold rule *defends* its memory — it tolerates
a gap below the content end of up to 60% of the viewport, so the
difference was not corrected, it was kept.
Measured on a 43-Turn session paged from a three-Turn tail: issued for
12336 while the content end was there, landed 608ms later against a
content end of 11972, and the first follow frame after it read `desired
11972, target 12336, onTarget true` and never moved again.
`resolveFollowState` is `resolveFollowTargetScrollTop` with the mode kept
rather than dropped, so the resume goes through the same rule the snap
back aimed with, evaluated when it lands instead of when it was issued.
Identical when nothing moved in between, which is the ordinary case.
Verified: 445 files / 3161 tests, type-check and lint clean. Three tests,
each red without its fix — the retry's asserts that the first ask *is*
refused, since forcing it through is the fault it replaces.
652 boundary asks in one recording, 648 of them answered `precondition`, and every one of them re-armed the direction so the next scroll event asked again. 664 of the 3077 lines in that log were this. The transcript was a session with every Turn it was ever going to have, and the reader was resting at its head. The precondition guard for "no history window is open" was one `||` covering three unrelated facts — wrong direction, session not partial, catalog not loaded — and answered all of them `cancelled`. Only the last of the three is transient. A session that is not partial *is* the transcript on screen: its head is the first Turn, and nothing will ever be before it. That is `exhausted`, which is the answer that latches the direction quiet until the window moves, and it is also what the ordinal path one step further on would have said — `getSessionCanonicalTailRange` returns `startOrdinal: 0` for such a session, so `resolveHistoryBoundaryTarget` answers `reached-start`. The guard was standing in front of the right answer with a temporary one. `resolveTailBoundaryPrecondition` joins its siblings in `flowChatLiveTailWindow.ts`, where the three outcomes can be stated and tested apart from the component. `after` deliberately keeps answering `cancelled` even though nothing follows the canonical tail: it reaches that branch only through the retained continuous projection, which renders as a history window while the viewport intent is the live tail — and the latch is cleared by the window bounds changing, which that projection has none of. Latching it would be permanent for the rest of the presentation, to save asks that were not the ones being measured. Two probes were lying in the same recording, which is worth more than the noise they made — the next investigation reads these first: - `latched-exhausted-while-partial` fired four times, all `after`. The alarm means "history has gone silently missing", which is a question about the *head*; `after` latches the moment the reader reaches the newest Turn, and a session stays partial throughout the paging that gets them there. It was warning about the ordinary case. `resolveHistoryBoundaryTarget` already draws this line one layer down, between `reached-latest` and `beyond-known-total`. - `turnNavigation.placed.outcome` reported `driftPx: -892.7`, the largest displacement in the log, for a navigation that behaved correctly. The unrendered branch places the Turn instantly and then clamps it out of the reserved blank 20ms later, in the same task and as one visible movement — but it traced them as two placements, so the first one's outcome sample compared where the viewport came to rest against a target this function had itself superseded. The clamp is now part of the placement it belongs to, and reports `clampedToContentEnd` and the offset it clamped from. Verified: 445 files / 3161 tests, type-check and lint clean. The new unit tests cover both directions of the precondition that must not be confused — a catalog still on its way, and a session absent from the store, both of which have to stay `cancelled`.
A focus request — a usage-report entry, a cross-session jump — is resolved against three keys in turn: the Turn's id, its index among the rendered items, and its position in the transcript on screen. The first two placed the viewport instantly. The third, `scrollToTurn`, animated. Which one runs depends only on what the request happened to carry, so the same click animated or jumped depending on whether the report knew the Turn's id. Nothing the reader did chose between them. Instant is the side to settle on: it is what the other two already do, what turn-rail navigation and search navigation do, and the only behaviour `navigateToTurnWithStatus` can honour for a Turn that is not rendered yet — that branch has to read its landing back to clamp it out of the reserved blank, and an animation has not arrived to be read. The header button's jump to the current Turn is unaffected: it asks for an animation itself, over a Turn that is on screen by definition.
…owed Jumping to latest was animated at any distance, and past a few screens that is not an animation. The frame loop stands down for a bounded time and takes the viewport back wherever the animation has got to: measured, a jump issued for 8717px animated 5480 of them and was finished in a single 3290px write. Two thirds of a scroll and then a jump, which is worse than either half on its own — and no yield budget fixes it, since a longer one only makes the reader wait through more of something they cannot read. So `resolveAnimatedJumpBehavior` animates within `FLOWCHAT_ANIMATED_JUMP_MAX_VIEWPORTS` and lands outright past it. Counted in viewports rather than pixels: the question is whether the reader can follow the movement, and what they can follow is a share of what they can see. The same 2000px is two and a half screens on a laptop and most of one on a tall display. Three reasons the far case is not worth animating even when it finishes: - The animation exists for spatial continuity, and three screens on the transcript in between goes past faster than anyone can read it. What is left is a wait where the answer was. - Animating across N screens of a virtualized transcript renders and measures every item passed while the animation runs, and heights are estimates until measured — so the content end moves under an animation aimed at where it used to be, and the follow loop corrects it afterwards as a second, visible movement. - Every other navigation in the transcript is instant already, so the rule is now one sentence rather than a list of entry points. Both jump-to-latest branches go through it, including the one that resumes a pinned Turn — that one is under a viewport by construction and keeps its animation. The post-streaming settle is left alone for the same reason: the gap it closes is bounded by `tailHoldMaxGapPx`, 60% of one viewport. `followOutput.jumpBehavior` records the decision and the distance in viewports, because an animation that was never issued and one the loop cut short look identical afterwards and call for opposite fixes. It is also how the constant gets checked: a `backstop` reason in `followOutput.animatedScrollEnded` means an animation ran out its yield without arriving, and the number is too high.
…olish # Conflicts: # src/web-ui/src/flow_chat/components/modern/VirtualMessageList.tsx # src/web-ui/src/flow_chat/components/modern/useFlowChatFollowOutput.ts
…on-fixes fix(harmonyos): the conversation screen during a run, and switching sessions
feat(web-ui): polish interaction motion and view transitions
…round feat(mac): brand DMG installer layout
…the blank Scrolling up by a pixel gave the follow away for good, and that is right only for a reader who left the live region. The reserved blank is up to `tailHoldMaxGapPx` under `hold-tail` and the whole gap under a pinned Turn, so a small scroll up can leave the reader a few hundred pixels off the tail with nothing hidden from them at all — until output grows past the bottom edge and they silently stop seeing it, with no affordance saying so. The snap back does not cover this. Its predicate is relative to the follow target, and under a pin the target sits inside the blank, so a reader above it is reported as having nothing to snap back from. `scrollTop > contentEnd` is the other edge of the same region, and between them they now cover it. So a watch runs for as long as the reader holds the viewport, keeping one bit between samples: whether the blank was on screen at the previous one. A crossing is that bit going from set to clear, and `resolveTailDepartureCrossing` decides who keeps the viewport by which side moved further — the content end rising to meet a stationary reader hands it back, a reader out-moving the content end keeps it. Streaming does not stop because the reader scrolled, so both sides usually move and the tie-break is doing real work. Not scoped to one crossing. That was written first and is wrong: a reader who climbs out of the blank, reads for a while and scrolls back down to sit in it again is in exactly the position the rule exists for, and had already spent the single crossing they were given. The watch also opens on any exit that finds none open rather than only the one that took the follow — every wheel notch exits, and only the first of them finds anything to give up. Three guards, each measured: - **A live gesture defers the crossing.** Over one session's twenty departures, nine left blank on screen and two of those ended `content-caught-up`: one a reader still for 1.6s and overtaken, one 320ms into a live gesture with the reader climbing 200px while content grew 237. The tie-break called the second for the content, correctly, and acting on it would have taken the viewport back mid-scroll. The latch is kept rather than spent, so the next sample judges the same transition again — a reader who carries on climbing is let go by a verdict that never needed the veto, one who has stopped is followed once the claim lapses. - **A travelling snap back is not the reader.** It crosses the same line from the wrong side, downwards through the blank, so samples taken while one is in flight update the offsets and take no verdict. - **Geometry read from two moments is not geometry.** `scrollTop` is clamped to `scrollHeight - clientHeight`, so no settled viewport is more than the spacer past the content end (`isTailBlankMeasurable`). A history prepend shifts the viewport by the height it inserted before that height is in the scroll range: measured, 14252px of shift against a content end still reading 989, a blank of 6239px in a transcript reserving a few hundred. Read at face value that is a reader deep in the blank, and the next sample would have pulled them out of the history they had just asked for. Resuming does not scroll. The blank closing is the two offsets meeting, so what remains is one sample of growth and the follow loop's ease covers it; a one-shot scroll would be a snap the reader can see for a correction they cannot. It also retires the pin, because the pin's reservation is the blank the reader just scrolled out of and restoring it would pull them back down to the offset they left. The unmount cleanup no longer depends on `closeTailWatch`. It is rebuilt whenever `getTailSpacerPx` changes identity, and a watch spans many renders by construction, so an effect tearing down with the callback closed every one of them a frame after it opened. `followOutput.tailWatch` and `tailWatchEnded` bracket the watch, the latter carrying `crossings` so one that resolved nothing can be told from one that resolved against the reader; `followOutput.tailCrossing` records each transition with its verdict, both raw deltas, and whether a gesture was live. Known gap added for the reader who scrolls out of the blank and stops at the content end — measured at four of nine watched departures, `readerMovedPx` tracking the blank height almost exactly — which neither this nor the snap back covers. Acting on it means reading intent from a resting position that has content in it, which the snap back's licence explicitly does not extend to.
Completed thinking rows were reserved at their full content height even when the renderer mounted them collapsed. Late measurement then removed thousands of pixels and could move an upward-scrolling reader toward the tail. Project the thinking expansion state into both rendering and height estimation. Apply late measurement deltas only when the entire row is above the viewport, and let those shifts invalidate stale snap-back targets. Keep coalesced item-resize diagnostics behind the FlowChat diagnostics switch. The probe records row identity, measurement geometry, and whether compensation was applied without logging flow-item content. Partly visible rows are intentionally left uncompensated. No persisted or remote protocol shapes change.
Expand explore-group classification to include selected non-critical dedicated tools and tools using the default card renderer. Keep command, task, session-message, and other conditionally important tools visible as whole rounds. Exclude MCP tools from default-card classification and add registry and projection tests for these boundaries.
Add an optional budget parameter (small/normal/large) to analyze_image and view_image so the agent can trade a cheap preview against a high-resolution detail pass. Omitting the parameter keeps the existing size-limit path, so default behavior is unchanged.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
10 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds an optional \�udget\ parameter (\small\ |
ormal\ | \large) to \�nalyze_image\ and \�iew_image, letting the agent trade a cheap preview against a high-resolution detail pass.
Background
Borrowed from Qwen-MM-Plugins after an A/B evaluation on DocVQA + ChartQA (300 calls across 6 switch combinations). Budget-only was the only group that improved both strict and loose accuracy while the agent actually adopted the parameter (large chosen 51/55 times).
What changed
Deliberately out of scope
From the same evaluation, \crop_image/\draw_bbox\ had zero adoption (0/300 calls), upscaling never triggered (max 1.03x), and the decision skill showed no measurable effect — those were dropped from this PR.
Fixes GCWing#2248