Conversation
81e0c8c to
83ba49f
Compare
A declared state interface replaced the component picker with a forced whole-area `#experiment` reference, removing the per-component selection and outline that `main` already ships. Sampling was also gated on the reference selector, so the only way to obtain state was to give up the selection. Reference identity and area state are now independent request-scoped evidence items: - `handleToggleElementPick` always arms the picker again, so a scene that declares the interface keeps main's per-component selection, outline, and send-time clearing. - `sampleInteractiveState` follows the current Scene instead of the draft reference, so an unreferenced follow-up still reports current facts and never re-creates or extends a reference. - The Host carries area state with or without a component reference. The evidence header names both identities and refuses to present area facts as properties of the referenced component. - `metadata` is absent when only area state travels, so no element identity and no Spotlight authorization can be derived from it, and the accepted-reference receipt stays driven by explicit references only. Review follow-ups in the same change: - Client sampling follows `NEXT_PUBLIC_COURSEWARE_REFERENCE_ENABLED`. An ungated packet turned an ordinary Pi question into a 400 while the reference feature was disabled. - A Scene that declares the interface always receives an availability boundary, including when the browser produced no packet at all. It is reported as `not-sampled` rather than the previous `no-interface`, which was a false statement about an activity that does declare one. Courseware without the interface keeps its unreferenced behaviour. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The interactive-observation snippet is referenced by all six widget content templates but was never added to the packaged-asset manifest, so the asset test and the golden scene prompt both failed. - `SNIPPET_IDS` now lists `interactive-observation`, restoring both the "exactly the generation-owned templates and referenced snippets" check and the "every referenced snippet is packaged" cross-check. - The interactive system-prompt snapshot is re-pinned. The change is purely additive: the snippet is appended to the simulation template. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The route rejects a request that carries a reference or a state packet while `NEXT_PUBLIC_COURSEWARE_REFERENCE_ENABLED` is off, but an ordinary question carries neither. It still reached the Host, and a Scene that declares the state interface then received the full page-state block — several kilobytes of constraints about evidence the deployment can never sample. The Host now returns before building that note when the feature is off. A route-level regression asserts that neither the Director prompt nor the Child prompt gains `PAGE-REPORTED STATE` in that configuration; disabling the guard makes it fail with exactly that symptom. Also reopens the composer before the unreferenced follow-up in the classroom browser spec. An accepted answer may close it, which made the assertion flaky without changing the behaviour under test. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…mbled evidence Cross-review found two defects in the request-scoped state evidence. The reference's Scene was folded into the sample's staleness test. The packet is already bound to the current Scene by the identity check above it, so a valid current-Scene sample was being discarded as `stale-sample` purely because the student's component reference came from an earlier Scene. Reference and area state are independent evidence items; freshness is a property of the sample alone. With the coupling gone the two can now disagree on Scene, so the note says so explicitly rather than letting the model attribute area facts to a component that may not be on the current Scene. The assembled evidence had no stated output budget. The static component packet is bounded to 24,000 code points upstream, but that bound covers the static packet alone; the note and the escaped observation JSON were appended without a recheck. Escaping `<` for the prompt expands one code point into six, and `<` is legal in a label or a fact value, so a packet the Host accepts could assemble to 149,385 code points. The budget is now declared as the static bound plus the room the note frame needs, which is what makes the degradation terminate. Over budget, the state body drops whole to an explicit `unavailable` statement: truncating the JSON would emit a broken packet, and thinning a `complete` relation set would turn an exhaustive set into a false one. The relationship summary degrades with it, so the prose never asserts COMPLETE over a body that is gone. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Thanks. This is the right direction: the courseware declares its own state and the platform reads it generically, with no per-widget adapters. Notes from a first pass. The first two are changes I'd like before a deeper review; the rest are smaller. 1. The contract is too heavy for v1. Please cut it down. The observation snippet is ~110 lines injected as REQUIRED into six generation templates. It asks every generated page to maintain an objects/facts/relations graph, declare relation completeness (
Suggested v1 shape:
Keep the 2. Fold the responder into the existing iframe patch instead of adding a second injection point.
Please add it as one more shim inside 3. Acceptance should be measured, not built. Current evidence is one generated lesson and one model. Before merge I'd like a small generation sweep: two or three lessons per interactive template with the default model, reporting how many publish a readable state and whether the "current value vs source default" question is answered correctly. That number is the merge gate. The simplified contract should push it up, not down. 4. Scope: agreed. Legacy adapters, whiteboard, multi-select and canvas/WebGL references stay out. Please don't add anything else to this PR. 5. Minor: the PR body still says "Draft status is unchanged" and carries a prepared-draft header comment. Please refresh it once the above lands. |
…ration Accept any JSON report within byte and depth budgets, without generated field requirements or relationship-completeness semantics. Keep publishState and an optional rendered result in the generation guidance. Prepare the observation responder through patchHtmlForIframe and let the pool own document identity, preserving state across placeholder remounts. Settle sampling failures locally and align browser/server nesting limits. Cover permissive JSON delivery, resource limits, lifecycle, legacy behavior, and real renderer remounts with focused regression tests.
|
@wyuc Thanks so much for your detailed review and guidance! I made the initial contract heavier than it needed to be and missed that in my own review; I'll pay more attention to the next design and implementation. The follow-up simplifies the contract, unifies iframe preparation, and records real generation measurements. Pushed commits: Contract and iframe preparationGeneration now asks for a summary, lesson-specific state, and optional rendered data only for an explicit apply/run step. The reader accepts arbitrary JSON without required-field or semantic validation. The 32,768-byte cap is supplemented by a disclosed 64-level depth bound: small, deeply nested JSON reproduced failures in downstream recursive processing. The responder now goes through Injection still requires the pool-owned identity. Unsupported crypto environments retain static references. This differs from your request for unconditional injection across all callers. I retained the identity condition because all existing classroom sampling paths are covered, while thumbnails and video exports have no sampling consumer. Would you be comfortable with this narrower boundary? If not, I'll make injection unconditional. The server still checks the declaration marker to preserve legacy-question behavior; this is separate from the removed client prefilters. Generation measurements and fixesThe real scene-content route used the configured default
Original HTML remains unmodified and matches raw responses and sampled source hashes. Some early harness retries lost first-attempt artifacts; the original numbers are final retained outcomes, not a first-pass rate. The results support the specific fixes and complete template coverage, not universal model reliability. VerificationFinal related suites: 365/365 unit/integration tests, 145/145 generation package tests, and 23/23 browser regressions. The two new procedural questions also passed against the real model. Prior real unavailable-state Q&A and the classroom Pro-mode round trip remain separate lifecycle evidence. An initial parallel run timed out in one Legacy-route test; isolated and subsequent serial runs passed, and the timeout remains unattributed. Root type checking still reports two errors in unchanged editor tests. Those tests are not claimed as passing; detailed logs are retained and can be shared. No full-repository green result is claimed. Updated demoThe GIF is converted from a real, continuous classroom screen recording at 2× speed. Without selecting a component, the first question correctly distinguishes current 1400 kg/m³ / 4.116 N from drawn 1000 kg/m³ / 2.94 N. After clicking Refresh drawing, a second question correctly reports agreement (the drawing rounds 4.116 to 4.12). These are separate Q&A sessions. This demonstrates automatic per-question activity sampling, not the component-reference path. It uses the existing buoyancy lesson and is a feature demo, not a new generation-sweep sample.
The PR description has been minimally corrected to remove obsolete graph/relationship and draft wording. Scope remains unchanged: no legacy-state adapters, whiteboard, multi-select, Canvas/WebGL component references, or full Workbench expansion. |
|
Thanks, this round lands where I wanted it: the contract is now Your question: the narrower injection boundary is fine. Injecting only when the pool supplies an identity mirrors how I ran the suites at P2 (both reviewers, blocking): unguarded P3, please fix while you're in there:
Noted, no change requested:
Once the P2 and the two P3s are in with tests green, I'll approve. |

Summary
Let classroom agents read what an interactive activity is doing now, including state that cannot be recovered from its source or visible DOM.
For example, a buoyancy activity can have a source default of
1000, a slider currently set to1400, and a paused drawing still showing an earlier result. This PR gives agents separate evidence for current parameters and the last rendered result.Newly generated courseware publishes a shared JSON state node inside
#experiment. The platform reads it once before each student question, freezes the sample, and attaches it to the request. Follow-up questions sample again, even without a new component reference. No per-widget adapters, polling, or recording.Behavior and boundaries
summary, free-formstate, and optionalrenderedfor activities with an explicit apply/run step. Reading accepts any JSON value without required fields or semantic validation, subject to a 32,768-byte cap and a 64-level depth bound protecting recursive processing. Sampling does not execute a business action.NEXT_PUBLIC_COURSEWARE_REFERENCE_ENABLED. Disabled ordinary questions receive no state evidence in model prompts.Interactive reference evidence plus state, and standalone state notes, have a 32,000-code-point output limit. Oversized state degrades as a whole to
unavailable / too-large; the JSON report is never truncated. PPT combinations also count existing static evidence when deciding whether state fits. Existing PPT evidence retains its own limits; if it already consumes too much room, only a bounded unavailable note is added. This is not a limit on the entire model prompt.Demo
The recorded session demonstrates component selection, current-versus-rendered answers, and freshly sampled follow-ups without a new reference.
In a second session, blocking the reader's reply produced a timeout while the page and declared interface remained present. The answer treated the previous density as historical and said the current density could not be determined.
These are two sessions with one generated buoyancy activity and one model (DeepSeek via MaaS), not acceptance across widget types or models.
Earlier cross-review fixes
Earlier verification
The results below describe commit
1edaef50and earlier runs. They are retained here as historical verification, not as results for the current candidate.Follow-up commit
1edaef50:pnpm exec vitest run tests/lib/chat/pi tests/lib/interactive --maxWorkers=2— 24 files / 333 tests passed. The new mixed-reference route tests first reproduced the original 400 in both Legacy and Native paths, then passed after the fix. Model execution is mocked.git diff --checkpassed for the follow-up.tsc --noEmit --incremental falsereported only the two known editor-test matcher errors (toHaveClass,toHaveStyle).Earlier verification, not rerun for this server-only follow-up:
f7446b2d, covering component picking/outlines, iframe keep-alive, observation lifecycle, and classroom sends/follow-ups. These are not claimed as a browser run of the final patch.No new real-model calls were made during that earlier follow-up. Updated generation measurements and verification details will follow in a separate comment.
Out of scope
Legacy state adapters, arbitrary/dynamic DOM selection, Canvas/WebGL component references, Whiteboard, multi-select, visual inference, editing authorization, new stores/registries/Context, Native tools, and generic Runtime policy.
Builds on #1224 and #1281 for existing reference identity and static evidence; those PRs do not validate the runtime-state behavior added here.