Repository navigation
Feature/embed f3 orchestration - #155
shri4anand wants to merge 10 commits into
Conversation
Two CI gaps from the embed work: - theme-variables.css holds the design tokens extracted from App.vue, so it is the one file where raw hex/rgb literals belong. The stylelint override granting that exemption still listed only App.vue and style.css, leaving 204 errors on the extracted file. Add it to the same override. - The E2E workflow selects Playwright projects through an explicit matrix, so the new 'embed' project never ran. Append it to the existing ui/privacy/permissions job rather than adding another job, since each one spins up Postgres, Django and Celery. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
/api/problems/<slug>/ returned only model fields, so a client fetching one
problem outside a problem set had no display_config and could not render the
problem's stimulus — a prompt problem's function-call table, terminal transcript
or image — and no feedback_config to decide whether to show segmentation. The
problem-set payload has always carried these, built from each handler's
get_problem_config().
Extract that enrichment into StudentService.get_problem_handler_configs() and
merge it into the detail response. Configs fall back to {} for an unregistered
type or a handler error, so a broken handler degrades to "no stimulus" rather
than a 500.
The LMS embed is the only caller of this endpoint today; the SPA reads problems
through the problem-set payload, so nothing else changes.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The embed rendered title, description and input only, so an EiPL problem showed "read the function below" above an empty editor. Port the stimulus block from ProblemSet.vue: reference code, function-call table, terminal transcript or image. Which stimulus is safe is decided per type by the adapter (EmbedStimulus) rather than by the payload. ProblemDetailView serializes reference_solution for every type, so a config-driven fallback to "show the code" printed the hidden function on a probeable problem. eipl reads the code; prompt gets only its configured stimulus, since the code is what the learner is trying to have generated; both probeable types get nothing, because the probe panel is the stimulus. Also thread a problem_set launch param through to the submit call. Submission.problem_set is a non-null FK, so a bare single-problem launch fails at the database for every activity type. This is a stopgap — B1-B5 should resolve the problem set from the LTI context instead. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
demo/embed/ scripts and documents a click-through demo of the embed work. Tier 1 runs against the real Django stack. tier1_setup.py grants the AI consent an LTI-provisioned learner would not have — prompt and probeable_spec have no pre-gate, so without it they fail mid-pipeline and leave orphan Submission rows — then mints a mock-Firebase launch token and prints ready-made launch URLs. Tier 2 runs against stub-api.js, a zero-dependency stand-in for the four endpoints the embed calls, so the same walkthrough works with no Django, Celery, Redis, Docker or LLM key. The mock host gains a launch-token field and a picker covering both tiers, composing the launch URL from problem, problem set and token. The README records what running against the real stack surfaced and what is still open, including the non-null problem_set FK and reference_solution being served to students for probeable problems. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Probing a list[int] parameter with "1, 2, 3" sent the literal string to the
oracle. Python then iterated it character by character and raised "'>' not
supported between instances of 'str' and 'int'" from inside the sandbox, which
tells the learner nothing about what they typed wrong. Only the bracketed form
worked, because JSON.parse happened to succeed.
The cause was the fallback chain: after JSON.parse failed it recognised int,
float and bool, and everything else fell through to "return the string".
list[int] matched nothing.
Recognise container annotations (list, tuple, set, sequence, iterable, with or
without an element type), split on commas and coerce each element by its inner
type. A stray opening or closing bracket is tolerated, since "[1, 2, 3" is a
typo rather than a different intent.
Extract the parser to a pure exported parseProbeInputValue so it can be tested
without mounting the composable. The composable still returns it under the
original parseInputValue name, so consumers are unaffected.
Note: RefuteInput.vue carries an independent copy of this parser with the same
class of bug, and worse — it tests type.includes('int') first, so a list[int]
parameter takes the integer branch and "1, 2, 3" silently becomes 1. Left alone
here because refute is outside the embeddable set; it wants its own change.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The embed calls one API origin, fixed at build time, so the stub and Django were competing for the same port. Only one tier could be live, and picking from the other group in the mock host gave a 404 with no hint why. Add --upstream. The stub keeps answering for its own demo-* slugs and forwards everything else, streaming the reply back untouched so SSE still works and the upstream's own CORS and auth behaviour is what the browser sees. Routing is by ownership rather than by path: a problem slug it does not know, a task id it did not create, or a submit body naming someone else's problem all go upstream. Run Django on 8001 and the stub on 8000 with --upstream, and both groups in the picker work at once — real problems, submissions, probes and SSE from Django, demo slugs from the stub. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Maintainer pass on the embed PR ahead of merge. - Restore the learner's last answer from /api/last-submission/ after the problem loads. Host-supplied SPLICE state still takes precedence when a host answers getState, but an LTI platform such as Canvas never does, so restoration now works from Purplex's own records too. Programmatic restores no longer mark the editor dirty, so a later restore is not wrongly refused. - Post outbound SPLICE messages to the known host origin instead of "*", so score and state cannot be read by an arbitrary framing page. - Honour the ?token= launch parameter only in development builds; it was a placeholder for the LTI launch and must not ship as a production path. - Translate the three submission failure messages (en/es). - Restore the #!/bin/bash shebang that an earlier commit dropped from start.sh. - Tests: bridge target-origin unit tests, an E2E case for server-side restoration, and a default last-submission stub so the host-state cases stay deterministic. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
CoffeePoweredComputers
left a comment
CoffeePoweredComputers
left a comment
There was a problem hiding this comment.
Maintainer review. Thanks for this, the shell, adapters, orchestration and bridge are solid, the tests are real, and the CSS extraction is a pure move. I pushed a commit (aa0e83e) with the fixes that were safe to make directly; details in the PR description under "Maintainer changes".
Context that changed while this was open. The target host is Canvas via an LTI launch, not a SPLICE-speaking host. That does not invalidate this PR: everything here except the SPLICE score/state round-trip still applies, and the bridge stays useful for listing problems in the SPLICE catalog (splice.cs.vt.edu). It does change two things: answer restoration cannot rely on the host answering getState (fixed in my commit), and the ?token= launch parameter has no production replacement until the LTI launch lands (now dev-only).
What I changed
- Restore the last answer from
/api/last-submission/after load, with host state still taking precedence when it arrives. Programmatic restores no longer flip the editor to dirty, which previously blocked a later restore. - Outbound
postMessagenow targets the known host origin instead of*. ?token=is honoured only in development builds.- The three submission failure strings are translated (en/es).
start.shshebang restored.- Tests: two bridge unit tests, one E2E case, a default
last-submissionstub.
Still open, see inline comments
- The single-problem endpoint sends
reference_solutionfor every type, including the hidden oracle on probeable problems. Pre-existing and also true of the problem-set payload, so it should be its own backend PR rather than folded in here. - The probe input parser rewrite is a good fix but is unrelated to embedding and touches every probeable problem in the main SPA. Please split it into its own PR so it gets reviewed on its own terms.
- #146 and #147 asked for
probe.usedevents, probe state in the payload, and acomprehensionevent. None of those are here. Canvas ignoressendEventand the catalog only logs it, so I would drop those criteria rather than build them, but the PR should not claim to close those issues as written. I have edited the description accordingly.
Verified locally on aa0e83e: vitest 529/529, embed Playwright project 10/10 against the Vite dev server, vite build clean with no Firebase chunk in the embed graph, eslint and stylelint clean on the changed files, i18n 100%, backend tests/unit/test_problem_detail_handler_configs.py 7/7 and pytest -m architecture 51/51 against a throwaway Postgres. vue-tsc is broken in the lockfile (TypeScript 5.5.4 with vue-tsc 1.8.x) independently of this PR; CI has it on continue-on-error.
| # Handler-provided render configs, matching what the problem-set payload | ||
| # carries. Clients that fetch a single problem (the LMS embed) need these | ||
| # to render the problem's stimulus and feedback correctly. | ||
| data.update(StudentService.get_problem_handler_configs(problem)) |
There was a problem hiding this comment.
Not introduced here, but worth flagging because the embed becomes a new consumer of this endpoint: ProblemSerializer includes reference_solution for every type, so a probeable problem's hidden oracle is readable in the network tab. The adapters in this PR are careful never to render it, which is good, but the fix belongs on the server. The problem-set payload in student_service.py (line 208) has the same exposure for the main SPA. Suggest a separate backend PR that strips reference_solution for probeable_code and probeable_spec on both student-facing paths; I would fold that into B3 (#140).
| * Exported for testing — it is pure, and the parsing rules are where probe | ||
| * input actually goes wrong. | ||
| */ | ||
| export function parseProbeInputValue(rawValue: string, type: string): unknown { |
There was a problem hiding this comment.
This is a genuine improvement (1, 2, 3 for a list[int] parameter now reaches the oracle as a list instead of failing inside Python), and the tests are good. But it changes behaviour for every probeable problem in the main SPA and has nothing to do with embedding. Please move this commit into its own PR so it can be reviewed and, if needed, reverted independently.
| // can be exercised against a local backend (see demo/embed/). Never honoured | ||
| // in production builds — a credential in a query string ends up in referrers | ||
| // and server logs. The LTI launch (B2, #139) hands the embed its JWT instead. | ||
| const ltiToken = environment.isDevelopment ? launchParams.get('token') : null |
There was a problem hiding this comment.
I gated this to development builds. Under the LTI plan the launch endpoint will hand the embed its short-lived JWT after validating the Canvas id_token; that handoff should not be a long-lived token in the query string, since it ends up in referrers and access logs. A one-time code exchanged for the JWT, or a same-origin cookie set during the launch, are the two usual shapes.
|
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
Description
This is the frontend half of the embed epic. The backend track (#138–#142) is not included and is still required
before the embed can authenticate against a real deployment.
What's here:
embed.html→embed-main.ts→ chromelessEmbedApp.vue) rather than a chromeless retrofit of the SPA. No router, noVuex, no Firebase in the embed's import graph.
useSpliceBridge.tsowns all postMessage I/O: thegetStatehandshake(which doubles as the readiness signal),
reportScoreAndStatewithscore/100normalized to SPLICE's 0..1,lti.frameResizeviaResizeObserver, andsendEventtelemetry. Correlates replies bymessage_idand validates sender origin. The reference library doesneither. Render never blocks on
getState.response, per the spec's warningthat it "may not even return". State is capped at 32 KB and checked for
structured-cloneability.
useEmbedSubmission.tsre-implements the submit → SSE → completionorchestration that lives inline in
ProblemSet.vue(~1359–1404). It isre-implemented rather than imported because that version is entangled with
routing, Vuex progress, the submission cache, hint tracking and toasts. The
sync-vs-async branch, SSE connect/disconnect lifecycle, and error/timeout
paths are preserved.
embed/adapters.ts) supply only the state payload,telemetry, and rehydration.
eipl/prompt/probeable_specshare onemapping body — their handlers'
serialize_resultoutput is identical —while
probeable_codediffers because it returnsstudent_coderather thanLLM-generated variations.
sseServiceFirebase coupling severed.getSSEToken()was hard-wired tofirebaseAuth; it now takes a pluggable token provider, and the Firebasepath moved behind a dynamic
import()so it stays out of the embed's staticimport graph. The embed installs its own provider at boot. Main-SPA behavior
is unchanged.
manualChunksrewritten to function form. The old object form forcedAdminUsers.vueinto anadminbucket regardless of the real import graph,which pulled shared modules — and Firebase with them — into every entry that
touched them. Those components are already behind dynamic
import()inrouter.ts, so they still code-split on their own.Verified at the bundle level: the 150 kB
vendor-firebasechunk is absentfrom
dist/embed.html's chunk graph.Type of Change
Testing
41 unit tests covering the embed (10 bridge, 15 adapters, 16 orchestration);
full frontend suite passes at 505/505.
9 Playwright E2E tests (
e2e/embed/splice-protocol.spec.ts, newembedproject) drive the real embed bundle inside a real cross-document iframe and
assert the protocol contract end to end: handshake, answer restoration, frame
resize, score normalization, telemetry, refusal of non-embeddable types, and
rejection of a reply whose
message_idwas never issued. The API is stubbed atthe network boundary, so the suite needs only the Vite dev server — no Django,
Celery, or Docker — and stays free of LLM latency.
Also run clean:
vite build,eslint,stylelint, andi18n:check(100% en/es).
Manual verification:
embed-mockhost.htmlframes the embed and logspostMessage in both directions. Its "Adopt last reported state" button copies
the last
reportScoreAndStatepayload into the prefill box, reproducing thelaunch-to-launch round-trip a real LMS performs.
Checklist
Related Issues
Closes #143 (F1 — Embed Vite entry + chromeless shell)
Closes #144 (F2 — useSpliceBridge + mock SPLICE host harness)
Part of #145 (F3 — Embed submit→SSE→completion + answer restoration; restoration from Purplex's own records added by the maintainer commit, host-state path as before)
Part of #146 (F4 — EiPL + Prompt embed adapters; the
comprehensionevent criterion is not implemented and is likely to be dropped, see review)Part of #147 (F5 — Probeable adapters;
probe.usedevents and probe state in the payload are not implemented, see review)Partially addresses #148 (X1 — protocol-level E2E; the live-backend pass and
privilege-boundary security review remain, both blocked on the backend track)
Maintainer changes (aa0e83e)
Target host is now Canvas via an LTI launch (see the review thread and the epic #137). Changes pushed directly to this branch:
/api/last-submission/after the problem loads; host-supplied SPLICE state still wins when a host answersgetState. Programmatic restores no longer mark the editor dirty.postMessagetargets the known host origin, not*.?token=launch parameter honoured only in development builds.start.shshebang restored.last-submissionstub.N/A — the UI is the existing activity components rendered chromeless. Reviewers
can see it by running the Vite dev server and opening
embed-mockhost.html.