Repository navigation
app: move the DFT Workbench executor and restore into a tested module - #45
Merged
Merged
Conversation
PyPI/autostart mode ran a bare `uvx --from goldilocks-core[http]`, so it
resolved whatever release is newest on PyPI. The vendored
goldilocks-workbench tarball only accepts a core whose /capabilities
vocabulary_version matches its own ("1"), and core's v2 line moved to
"2" in bde3628. The next core release on PyPI would therefore have
broken DFT Workbench on every fresh PyPI-mode install.
Pin the requirement to the release the Dockerfile already clones
(CORE_PYPI_REQUIREMENT), use it in the spawn command and the startup
hint, and note in the docs that a GOLDILOCKS_CORE_PATH checkout must
speak the same vocabulary as the embedded UI.
`vite build` only transpiles, and the eslint config is not type-aware, so nothing reported type errors: `tsc --noEmit -p .` failed on main while CI stayed green. - Give MODEL_GROUPS an explicit type so the optional `disabled` field type-checks in findModelById (the one existing error). - Add `npm run typecheck` and run it in `poe web-check` and in the CI test job, before the build. This is also the gate that will surface goldilocks-workbench API changes (e.g. removed snapshot fields) when the vendored tarball is next bumped. Closes #17
The core version lived in three unconnected places: the Dockerfile's `git clone --branch v0.1.1`, PyPI mode's requirement, and the vendored goldilocks-workbench tarball. The tarball only works against a core that speaks the same vocabulary_version, so a bump that misses one of them breaks DFT Workbench in the browser while CI stays green. - src/goldilocks_agent/core_pin.json records the release (repository, tag, commit, version, vocabulary_version) and the tarball (file name, sha256, source commit). core_server.CORE_PIN loads it as package data; PyPI mode's requirement is derived from it. - The Dockerfile fetches the pinned commit and asserts HEAD matches it, and does so before copying the agent's own source, so editing src/ no longer refetches core. - tests/test_core_pin.py checks the pin against the Dockerfile, PyPI mode, package.json, the lockfile, the tarball's sha256, the vocabulary constant compiled into the tarball, and that every route the tarball calls is proxied (prod and dev). It also asserts goldilocks-core is never a Python dependency, and ruff's TID251 now bans importing it.
goldilocks-core's v2 line now fails closed: a model that can't load fails
the request instead of falling back to a heuristic. Several gaps in
core_server would hide or cause such failures:
- A failed start never reached the UI: ensure_running() restarted the
bring-up on the very next call, so the frontend's once-a-second poll
only ever saw "starting" while a failing config was respawned each
time. Now every call gets the error for 10 s before one retries it.
- Core's stdout/stderr went to DEVNULL, so a 500's traceback was lost.
It now goes to this process's stderr, prefixed `[goldilocks-core]`
(the terminal, or `docker logs`), and a startup failure's detail ends
with core's last output line.
- Any response under 500 on :8000 counted as a healthy core, so an
unrelated dev server was reported "ready". Only core's own
`{"status": "ok"}` counts now; any other answer is reported as port
8000 being taken, without spawning onto it.
- After /health passes, /capabilities is checked once. A core that
speaks a different vocabulary than core_pin.json records is an error
naming both versions and the fix, instead of a Workbench banner that
can't be dismissed. A core this module spawned is stopped first, so
the next attempt starts from the fixed checkout.
- mMACE's checkpoint was accepted if any file existed, so an interrupted
download stayed broken (a truncated checkpoint loads, then predicts
garbage). It is now downloaded in Python to a .part file and accepted
only when its published size and md5 match.
- PyPI mode never installed core's assets, while the Workbench sends
fetch_missing=false, so a fresh machine got HTTP 424 on every /explain.
It now runs `goldilocks assets install workbench` first, as poe serve
does. shutdown() signals the spawned process group, since the
top-level PID is a shell during an `sh -c` chain's earlier steps.
- The proxy's 503 is returned in core's own error envelope, the only
shape the Workbench reads a message from.
Verified against real servers: with an unrelated server on :8000 the
polls return that error without spawning; the vocabulary check against a
real core and a simulated mismatch.
A reload replayed the saved structure and settings into the embedded Workbench without ever starting goldilocks-core -- only an open DFT Workbench panel or a chat DFT tool calls /api/core-server/ensure -- so on the first load after an agent restart the replay hit a backend that wasn't running and silently did nothing. Worse, the save listener wrote "" whenever no structure was loaded, which the empty workspace on every page load triggered, so the saved state was gone before the replay failed. - The restore first waits for /api/core-server/ensure to report ready, and stops on anything else. It only replays into an untouched workspace, so a structure a chat DFT tool opened meanwhile is kept. - The save listener only ever writes a loaded structure, now with the vocabulary it was written in. The saved state is forgotten only when core rejects that structure for good. A transient failure is dismissed rather than left as a Retry banner, since Retry reopens the structure with default settings that would overwrite the saved ones. - The replayed draft keeps code, task and HPC profile only if core still offers them, and the overrides only under the same vocabulary, since an override core rejects sticks to the draft. States saved before this change all came from vocabulary 1. - The chat bundle card says why when Download can't save anything (no structure open, settings changed since, or the bundle still being generated) instead of doing nothing; its note is kept per chat. - Docs and comments now list everything that starts core. Checked in a real browser (Playwright, core mocked from recorded responses): the replay waits for ensure, restores structure, task, HPC and overrides, and the card then saves the zip. The saved state survives an ensure error, a /capabilities 503 and a 502 from /inspect, and is dropped after a 422. The same checks fail on the previous build. Closes #18
The code that drives the embedded Workbench on the model's or the page's behalf -- the dft_open_structure, dft_review and dft_download_bundle executor, the chat card's Download, and the reload restore -- lived inside App(), typed `any` and untested. That is the code a goldilocks-workbench bump breaks first: its snapshot fields and action semantics change between core releases, and nothing failed until a chat tool misbehaved in the browser. - app/src/dftWorkbench.ts now holds dispatchDftTool, downloadDftBundle, restoreDftWorkspace (the restore's sequence) and restorableDraftPatch, with the workspace and the core-readiness check passed in rather than taken from App()'s closure. The card's Download now also tells "still being generated" apart from "no review to generate it from". - app/src/dftWorkbench.test.ts (vitest, run by `poe web-check` and CI) drives them through a real createWorkspace + HttpCoreClient, with core replaced by responses recorded from a real goldilocks-core (app/scripts/record-core-fixtures.mjs): the tools' results and errors, the card's notes, and every restore outcome -- restored, core not up, workspace already in use, structure rejected for good, transient failure. The vendored client refuses responses from any other vocabulary, so bumping the tarball without re-recording the fixtures fails these tests rather than the browser. The browser checks from the restore fix still pass against this build.
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
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.
Closes #39
Stack 6/6, on top of #44 -- base
main, so the Files tab also shows the earlier PRs' commits; this PR's own change is its last commit, 1513275. Merge the stack in order, with merge commits.