Skip to content

fix(Bob): carry the reference into generation, teach the full component system - #754

Merged
surajshetty3416 merged 36 commits into
frappe:developfrom
surajshetty3416:ai-shell-geometry
Aug 16, 2026
Merged

fix(Bob): carry the reference into generation, teach the full component system#754
surajshetty3416 merged 36 commits into
frappe:developfrom
surajshetty3416:ai-shell-geometry

Conversation

@surajshetty3416

@surajshetty3416 surajshetty3416 commented Aug 15, 2026

Copy link
Copy Markdown
Member

Asking the AI to build or edit a page that should fit the rest of the site kept failing in repeatable ways: sidebar layouts came out stacked, a font the reference uses once took over every heading, fonts that only exist on another site rendered as the browser's Times fallback, embedded components showed placeholder content and were not functional or reusable, icons the AI placed rendered as empty squares, script edits never reached published pages, and a selected block was quietly ignored.

What changed

  • Reference reads feed generation directly. The page-generation step receives the reference's measured facts (layout scaffold, font roles, component details) alongside the brief, instead of relying on prose to carry them. Sidebar-beside-content layouts are now buildable, and measured reference values win over the prompt's own style defaults.
  • The component system is fully usable, both halves. The agent sees and edits default content, per-instance overrides, declared props, client scripts, and data scripts. Behaviour it writes for a component ships with every embed.
  • Missing fonts are caught, not shipped. A referenced font that cannot load on this site is flagged during the read and in self-review, with system fonts and font stacks handled correctly and probes bounded to one parallel three-second sweep.
  • Rendering correctness. Icons the AI places are baked server-side into the authoritative tree (they used to vanish on reload and publish); script edits regenerate the minified public file that published pages serve; canvas updates wait for the round's commit so a just-created component can never fetch as missing.
  • Chat and composer. Context blocks are attached explicitly and treated as the request's subject; a wand button sharpens the typed prompt in place (replacing the style-preset picker); timeline labels follow each step's live status; code blocks are readable in both themes.
  • Cleanup. A final pass collapsed the accretion from review rounds: one shared guard seam for block updates, schema resolution once per prop write, dead code and a redundant retry removed. Net effect across the branch, excluding the generated icon asset: substantially more deleted than added outside the new capabilities.
Why generation missed the reference's layout and fonts

The generation step receives only a short text brief distilled by the conversational model, and prose loses load-bearing facts: "sidebar beside a scrollable main area" came back as a sidebar stacked above the content, and "uses Newsreader" came back as Newsreader on every heading when the reference uses it once at 48px. read_page now stashes a compact scaffold of the reference's page-level layout plus a design digest where each font states its role (which elements, which sizes), and generate_page passes that to the generation model as ground truth. The generation prompt gained an app-shell recipe (root row of sidebar plus scrolling main column), a rule that measured reference values beat its craft defaults, and nesting discipline (a styled container's content goes inside it). Self-review must re-preview after a layout fix, and its rubric names the visual failure signatures: fallback serif, stale component defaults, content stacked outside an empty container.

The component system: instances, props, client scripts

An embedded component renders its own blocks with default content until the page overrides them. read_page describes each embedded component's default content, declared props, data script, and the adjustments the reference page's own instance applies, including the pattern of hiding shared chrome and building a page-specific variant; the same description is included for the open page's own embeds. The codec exposes which component-internal block each instance child mirrors (of) so a default can be traced to the ref that overrides it.

Props: update_block gains a props argument. A first override borrows the declaration's full config; a prop the definition never declared is declared there too (typed from its value, flagged so the editor's Block Options panel shows it) — but only on a bare schema: a definition that already declares props rejects unknown names against the live schema, so a typo on one instance can never pollute a shared component, and an instance pinned to an old snapshot cannot open the live schema either. Child-targeted prop writes route to the instance root, matching where the canvas and the renderer read them.

Client scripts: component blocks can carry their own {js, css} (component blocks only — page-wide code stays in page scripts, where the editor shows it). The js runs once per rendered instance with this bound to that instance's element and (component_data, props) as arguments; the css is scoped to the block. Writes mirror into the component definition so every embed gets the behaviour. The prompt teaches the access syntax, demands class hooks the model sets itself (the rendered HTML is not the block tree), defaults self-contained interactive units to components without being asked, and requires a built component to end self-contained. Bindings accept props.<name> and component.<key> sources, so text that mirrors a prop reflects prop edits live on the canvas.

Font availability

Builder pages default to Inter, so text in a Times-like serif means a block named a family that is not loading — typically one that existed on the source site only as an uploaded User Font. read_page checks each family against this site's User Fonts and a cached Google Fonts probe, and tells both the agent and the generation step to use a resolvable family and to inform the user about the missing asset. The check probes only the first family of a stack, skips CSS generics and web-safe system fonts, runs all uncached probes in one parallel sweep bounded to three seconds, counts negatives only from a complete sweep, and fails open on network trouble. A combined multi-family request cannot replace per-family probes: that endpoint answers 200 and silently drops unknown families.

Rendering correctness fixes

Icons: the agent emits a compact icon: <name> reference whose SVG was baked only during client-side parsing — under the server-authoritative tree that meant every icon vanished on reload and publish. The server now bakes from the same icon package the frontend uses (shipped as a small gzipped asset, transform kept in lockstep).

Scripts: update_script wrote the doc directly, bypassing the save hook that rewrites the minified public file published pages serve — edits looked applied in the editor while published pages ran the old code. It saves through the document lifecycle now.

Op mirroring: canvas updates are emitted after the round's commit, so an op referencing a doc the round created (a component extract) can no longer reach the editor before the doc exists and cache a missing-component placeholder.

surajshetty3416 and others added 8 commits August 15, 2026 15:08
A 'match the sidebar page' build came out with the sidebar stacked ABOVE
the content: the generation prompt hard-required a vertical root stack,
and the generation model never sees the reference read anyway (only the
loop model does) — the brief's prose is the only carrier, and prose loses
page-level geometry.

- read_page stashes each reference's layout scaffold (root + two levels,
  layout styles, component embeds); generate_page hands it to the
  generation model as ground truth next to the brief.
- GENERATION_YAML gains the APP SHELL exception: a root row of rail
  component + scrolling main column, with the section rules applying
  inside the main column.
- LAYOUT_STYLE_KEYS learns flexDirection/alignItems/overflow/grid — the
  component contract could not state the one property this hinged on.
- resolve_page_reference prefers the published page when several share a
  route (the site had four drafts of the reference route).
- Self-review must re-preview after a layout fix: the failed session
  'corrected' the layout blind and told the user it was fixed.

Co-Authored-By: Claude <noreply@anthropic.com>
The rebuilt page used the reference's exact fonts and token handles but
promoted its one-off 48px serif hero moment into the display face for
every heading, at 1.5-3x the reference scale: 'Newsreader x1' in the
digest reads like half of a heading/body pairing, and the generation
prompt's contrast floor pushed the sizes up.

- design_digest fonts now carry elements and sizes per family, so the
  working face and a one-spot accent are tellable apart.
- The reference stash hands the digest to the generation model, whose
  preamble and GENERATION_YAML now say: measured reference values beat
  the craft defaults — a calm reference stays calm, an x1 font is an
  accent to place exactly where the reference does.

Co-Authored-By: Claude <noreply@anthropic.com>
A page copied from another site can name a font that only existed there
as an uploaded User Font. Google Fonts serves nothing for the name, the
browser falls back to its default serif, and a faithful copy of the
reference still renders broken. read_page now checks each family against
this site's User Fonts and a cached Google Fonts probe, warns the loop
AND the generation step, and the preview rubric names the Times-fallback
signature so self-review reacts to it.

Co-Authored-By: Claude <noreply@anthropic.com>
Embedded chrome rendered its defaults (a breadcrumb reading 'Home' on a
product page): the agent knew to embed components and copy their scripts
but nothing taught the OTHER half — that an instance's children are
per-page overridable, and that reference pages adapt shared chrome that
way (including hiding it and building their own variant).

- read_page's component contract now lists each component's default
  content and the overrides the reference's own instance applies.
- The codec emits 'of' on instance children (which component-internal
  block a page-side ref mirrors) so a default can be traced to the ref
  that overrides it.
- System prompt gains the instance mechanics; the generation prompt
  defers page-specific chrome bits to the editing pass; the preview
  rubric checks for stale default chrome.

Co-Authored-By: Claude <noreply@anthropic.com>
The component system's second half was invisible: components declare
props (instances set their own values) and can carry a server data
script that receives those props — the agent knew neither existed.

- The codec emits 'props' (name -> effective value) on definitions and
  instances; read_page's contract lists declared props, data-script
  presence, and the reference instance's prop values.
- update_block gains a 'props' arg (server merge + canvas twin) so an
  instance can be parameterized the way the component intended, instead
  of overriding its children.
- The generation prompt gains nesting discipline (a styled container's
  children were twice emitted as its siblings) and the preview rubric
  names the orphaned-content signature.

Co-Authored-By: Claude <noreply@anthropic.com>
A replay embedded the reference's sidebar and styled it to LOOK right
statically instead of attaching its companion script (dead hover/toggle
behaviour), and spread the reference's one-off serif across every
section opener. Both instructions now say it outright: embed + script
attach are ONE action, and a once-used font caps at the hero moment.

Co-Authored-By: Claude <noreply@anthropic.com>
The props write-path existed but was unreachable: declared props only
surfaced in reference reads, so on the open page an instance advertised
nothing and the model reached for a child override where a prop was the
intended surface (confirmed in a live test). render_page_context now
appends the same component contract — declared props, default content,
data script, this page's own overrides — in an open-page voice.

Co-Authored-By: Claude <noreply@anthropic.com>
'Checked how it looks' showed while the screenshot was still being
taken, and 'Building the page' stayed progressive forever after the
build finished. Labels now carry both voices: begin_activity emits the
running form, finish_step swaps in the past-tense form on the same step
id, so the live view and the persisted timeline each read true.

Co-Authored-By: Claude <noreply@anthropic.com>
@surajshetty3416 surajshetty3416 changed the title AI: carry the reference into generation, teach the full component system fix(Bob): carry the reference into generation, teach the full component system Aug 15, 2026
@greptile-apps

greptile-apps Bot commented Aug 15, 2026

Copy link
Copy Markdown
Contributor

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Reviews (29): Last reviewed commit: "docs(ai): state the font-probe latency r..." | Re-trigger Greptile

Comment thread builder/ai/agent/tree.py Outdated
Comment thread builder/ai/agent/tools/query.py Outdated
@codecov

codecov Bot commented Aug 15, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 76.62602% with 115 lines in your changes missing coverage. Please review.
✅ Project coverage is 66.22%. Comparing base (335854a) to head (bc51f7a).

Files with missing lines Patch % Lines
builder/ai/agent/loop.py 10.86% 41 Missing ⚠️
builder/ai/agent/tools/query.py 79.86% 30 Missing ⚠️
builder/ai/agent/tree.py 75.63% 29 Missing ⚠️
builder/ai/block_codec.py 58.33% 5 Missing ⚠️
builder/ai/agent/tools/scripts.py 0.00% 4 Missing ⚠️
builder/ai/page_writer.py 75.00% 4 Missing ⚠️
builder/ai/agent/artifact.py 75.00% 2 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop     #754      +/-   ##
===========================================
+ Coverage    65.61%   66.22%   +0.60%     
===========================================
  Files           88       90       +2     
  Lines         9585    10015     +430     
===========================================
+ Hits          6289     6632     +343     
- Misses        3296     3383      +87     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

surajshetty3416 and others added 2 commits August 15, 2026 23:47
Typography's pre colors are fixed light-theme values, so a code block in
the dark chat panel rendered dark-on-dark (a route tree was barely
visible). Pin --tw-prose-pre-code/--tw-prose-pre-bg to the adaptive ink
and surface tokens; verified computed colors flip correctly with
data-theme in the live editor.

Co-Authored-By: Claude <noreply@anthropic.com>
- A first per-instance prop override now borrows the component
  declaration's full config (label, options), matching the canvas twin;
  a bare value dropped that metadata from the authoritative draft.
- Font availability probes the stack's FIRST family and skips generic
  CSS keywords, so 'Inter, sans-serif' no longer reads as missing.
- Replace filter() with a comprehension in the reference stash
  (frappe-no-functional-code).

Co-Authored-By: Claude <noreply@anthropic.com>
Comment thread builder/ai/agent/tools/query.py Outdated
surajshetty3416 and others added 8 commits August 16, 2026 00:52
Selection rode along invisibly and weakly: whatever happened to be
selected at send time was appended as '(User has selected: <id>)', a
bare unlabelled ref with no instruction — in a live session the model
ignored it and edited a sibling at the wrong altitude. Canvas selection
alone now sends nothing; the composer offers the selection as a '+' chip
the user attaches deliberately (removable, cleared after send), and the
server writes an authoritative note naming each attached block as the
SUBJECT and SCOPE of the request.

Co-Authored-By: Claude <noreply@anthropic.com>
The lock-collision error rendered raw in the chat: FAILED prefix,
internal page id, and an em dash. Show a plain sentence instead; the
FAILED convention stays internal to the lock return value.

Co-Authored-By: Claude <noreply@anthropic.com>
Several uncached families probed sequentially could stall a read for
seconds each on a slow network. Cap probes per read, and treat the first
network timeout as the verdict for the rest (one dead network costs one
timeout, not one per family). A combined css2 request is documented as
unsound: multi-family mode answers 200 and silently drops unknowns.

Co-Authored-By: Claude <noreply@anthropic.com>
Builder pages default to Inter (reset.css bundles InterVar on html), so
the Times-like serif is specifically the browser's own font taking over
when a NAMED family fails to load, not 'the default'. Say that in the
rubric and the availability warning. Also cut narrative comments across
the branch down to the constraint they carry.

Co-Authored-By: Claude <noreply@anthropic.com>
Blocks (and so components) carry clientScript {js, css}: the js runs
once per rendered instance with this = the instance's element and
(component_data, props) as arguments; the css is @scope'd to the block.
The agent knew none of it, which is why component behaviour kept living
in page-level companion scripts that must be re-attached by hand.

- Codec round-trips client_script; the component contract counts them
  ('behaviour ships with every embed; nothing to attach page-side').
- update_block gains client_script (server merge + canvas twin).
- The prompt teaches the syntax and prefers it over page scripts for
  behaviour that belongs to one block or component.

Co-Authored-By: Claude <noreply@anthropic.com>
Behaviour scattered on arbitrary page blocks would be invisible; page
scripts stay the home for page-wide code, where the editor's panel shows
them. update_block rejects client_script on plain blocks with a pointer
to set_page_script; the prompt and tool description carry the same rule.

Co-Authored-By: Claude <noreply@anthropic.com>
The props argument is the instance's resolved values (props.active_tab);
component_data is ONLY what the server data script mirrors under
component.component_data — other component.* keys feed bindings, not the
client script. Without that stated, a script reading component_data.items
against component.items gets undefined.

Co-Authored-By: Claude <noreply@anthropic.com>
Live tests proved the mechanics but exposed three gaps: scripts and
props landed as instance-only overrides (a fresh embed arrived bare),
the model selected rendered DOM by tag against its block-tree mental
model (bound text renders as plain divs, so querySelector('p') missed),
and it claimed untested interactions work.

- client_script writes and first-time prop declarations on an instance
  now mirror into the component definition.
- The prompt demands class hooks the model sets itself, never structural
  selectors, and forbids claiming unexecuted behaviour works.

Co-Authored-By: Claude <noreply@anthropic.com>
Comment thread builder/ai/agent/tree.py Outdated
A typo'd prop name on one instance would have declared itself on the
live component for every unpinned embed. A definition that already
declares props now rejects unknown names, echoing the declared list;
only a bare definition (the just-extracted authoring flow) accepts and
declares new props.

Co-Authored-By: Claude <noreply@anthropic.com>
Comment thread builder/ai/agent/tree.py Outdated
The re-run migrated a component's css into client_script but left the
behaviour js and repeater data page-bound: half-portable, double-fire
risk, and repeaters bound to page-data keys preview empty in the canvas
and render nothing on other embeds. The prompt now states the finishing
checklist: move js AND css into client_script, neutralize the build-time
page scripts, and write component list items as real blocks.

Co-Authored-By: Claude <noreply@anthropic.com>
Comment thread builder/ai/agent/tree.py Outdated
A prop Bob declared had no propOptions, so the editor's props panel
rendered it untyped (generic icon, string-coerced editing). Fresh
declarations now infer type from the first value on both appliers.

Co-Authored-By: Claude <noreply@anthropic.com>
Comment thread builder/ai/agent/tree.py Outdated
The editor's Block Options section (the instance props UI in the right
panel) filters to isStandard props; Bob's declarations lacked the flag,
so its props existed but never rendered. Set it on fresh declarations in
both appliers.

Co-Authored-By: Claude <noreply@anthropic.com>
Comment thread builder/ai/agent/tree.py Outdated
surajshetty3416 and others added 8 commits August 16, 2026 11:45
A prop edit changed nothing on the canvas: the widget's display text was
hardcoded and only the client script (which the canvas never runs) read
the prop. Bindings now accept 'props.<name>' and 'component.<key>'
prefixes selecting comesFrom, with codec round-trip, so text that
mirrors a prop is bound and the canvas reflects prop edits live. The
prompt says to bind such text instead of hardcoding it.

Co-Authored-By: Claude <noreply@anthropic.com>
…udge

- Op mirroring now emits after the round's checkpoint commit: an op
  referencing a doc the round created (a component extract) reached the
  canvas before the insert landed, and the failed fetch cached a
  'Component Missing' placeholder for the session. The store also
  retries once before falling back.
- Every server tool has a timeline icon; the generic wrench covered
  eleven of them.
- The prompt defaults self-contained interactive units to components
  without being asked.

Co-Authored-By: Claude <noreply@anthropic.com>
Icons the agent placed rendered as empty squares: baking was designed
client-side (lucideIcon.ts) when the client parse was the final word,
but the server-authoritative tree expands icon blocks with empty
innerHTML and nothing ever bakes them — so the canvas reload and the
published page both lose every icon. The server now bakes from the same
lucide-static set (shipped as a 93KB gzipped asset, transform kept in
lockstep with the frontend) in both expansion paths.

Co-Authored-By: Claude <noreply@anthropic.com>
apply_update_script wrote the doc via db.set_value, bypassing on_update:
the minified public file the published page serves was never rewritten,
so AI script edits looked applied in the editor while every published
page kept running the old code (a manual dirty-save fixed it, which was
the tell). Save through the ORM instead; verified the file content and
the cache-busting URL both update.

Co-Authored-By: Claude <noreply@anthropic.com>
'Updated 1 block.' sat beside 'Checked how it looks' — timeline rows
carry no terminal punctuation.

Co-Authored-By: Claude <noreply@anthropic.com>
The preset popover (categories + style cards) folded a canned style
line into the prompt; it is gone, picker component deleted. In its place
a wand button sharpens the typed draft in place via one cheap
completion: scope preserved (a small edit stays small), vague requests
gain concrete purpose/sections/tone, and the user reviews the rewrite
before sending. Live-verified end to end.

Co-Authored-By: Claude <noreply@anthropic.com>
The authoring-vs-strict decision ran against the pinned schema while the
declaration mirror wrote the live one: an instance pinned to a prop-less
version could declare accidental props onto a live schema other embeds
use. Judge against the LIVE schema (the mutation target); pinned-
declared names stay settable for what the instance renders. Adds the
edge-case test plus lucide baker tests.

Co-Authored-By: Claude <noreply@anthropic.com>
The canvas routes a prop write on a component child up to the instance
root (getPropsRoot), but the server stored it on the child, where
resolution never reads it — a reload silently restored the old value.
The working tree now routes to the same owner, validates against it,
and rejects props on blocks outside any instance.

Co-Authored-By: Claude <noreply@anthropic.com>
Comment thread builder/ai/agent/tools/query.py Outdated
surajshetty3416 and others added 2 commits August 16, 2026 15:00
Sequential probes could stall a reference read up to one timeout per
family on a slow network. All uncached families now probe concurrently
with a single 3s budget for the whole sweep; families that don't answer
in time are not judged (fail open), definitive answers cache for a day.
Five families live-probe in 0.17s.

Co-Authored-By: Claude <noreply@anthropic.com>
A partial sweep (some probes timed out) no longer reports its negatives:
on a flaky network, or behind a proxy that answers 4xx for everything, a
partial result must not drive font replacement. Definitive answers still
cache, so the next read warns from a complete picture.

Co-Authored-By: Claude <noreply@anthropic.com>
Comment thread builder/ai/agent/tools/query.py
Caching them deferred the same flaw one read: a negative observed while
other probes timed out became definitive evidence on the next read.
Negatives cache and report only from a complete sweep; positives are
safe evidence in any sweep.

Co-Authored-By: Claude <noreply@anthropic.com>
Comment thread builder/ai/agent/tools/query.py
surajshetty3416 and others added 2 commits August 16, 2026 15:18
Arial and friends are neither CSS generics nor Google families; a
completed probe branded a fine-rendering system font as missing and
pushed generation to replace it. They join the skip list.

Co-Authored-By: Claude <noreply@anthropic.com>
Cleanup pass over the branch, no behavior change:
- One screen_patch seam runs every per-field guard for both update
  paths; batch rejections keep their own reasons instead of all reading
  as bad binds.
- route_props resolves the pinned and live schemas once and feeds both
  validation and merge_props (was up to four resolve_component calls per
  prop write); undeclared_prop_names/undeclared_props_hint inlined.
- props_owner climbs via the existing parent_of.
- Codec prop compression reuses get_component_prop_values; its icon
  branch delegates to convert_icon_block (the live expansion path).
- bindingEntry shared by yaml.ts and toolDispatch, closing the canvas/
  server divergence on props./component. bind prefixes for add_block.
- componentStore retry removed: the after_commit op emit eliminated the
  race it patched, leaving only a 2s tax on genuinely missing docs.
- Dead non-server_applied script paths deleted from toolDispatch (~70
  lines, incl. a frappe.client.set_value call reintroducing the exact
  stale-published-script bug this branch fixed server-side).
- Smaller: instances dict derives pairs, digest computed once per read,
  shared strip_tags/layout_styles/block_label, cached get_all_user_fonts.

Co-Authored-By: Claude <noreply@anthropic.com>
Comment thread builder/ai/agent/tools/query.py
The bounded synchronous wait is deliberate; the reasoning now lives at
the constant it governs.

Co-Authored-By: Claude <noreply@anthropic.com>
@surajshetty3416
surajshetty3416 merged commit ce776cc into frappe:develop Aug 16, 2026
11 checks passed
@github-actions

Copy link
Copy Markdown

🎉 This PR is included in version 1.33.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant