Repository navigation
Fix/dft workbench chat reliability - #6
Merged
Merged
Conversation
#7) LangGraph restarts a node from the top on every resume -- when call_tool's own loop batched multiple interrupt-gated tool calls in one execution, each resume re-ran the loop from scratch and re-emitted tool_status/tool_result for every already-resolved call ahead of the one that just resumed. Beyond duplicate events, this risked real duplicate execution of side-effecting tools (e.g. a second real MACE run) if a batch of client-executed/confirmation-required calls landed in one LLM turn. Rewritten around _find_next_tool_call, which finds the most recent AIMessage's first tool call without a matching ToolMessage yet (scanning backward rather than assuming state["messages"][-1] is the tool-calling message, since it no longer is once ToolMessages append one at a time). call_tool now handles exactly one call per execution and returns immediately; a new conditional edge (route_after_tool) loops back to the tool node while calls remain unanswered, then hands control to the llm node once the whole batch is done. Considered batching all pending interrupts into a single resume instead (LangGraph's own documented "Multiple Interrupts" pattern) -- deferred to #8, since it trades away this fix's incremental-commit robustness for fewer round trips, and today's real batches are small.
…esting All found and verified against a real running instance, real Claude calls, and Playwright-driven browser sessions (not just code reading). - #6: the dft-bundle-ready download card could get silently overwritten by the next streamed text delta landing in the same reply, because the plain-text handler rebuilt `messages` from a stale closure-captured snapshot instead of the live session state. Now tracks the assistant message by object reference and finds-and-replaces it in the current array. - #9: the active conversation itself was never restored after a page reload in either local or shared-deployment mode -- reloading always landed on a blank "no chat selected" screen even though the conversation was still on the server. Persists `activeId` via the existing browserStorage()/sessionStorage-vs-localStorage split. While testing #5 below, found this restore was itself broken for local mode: the write-side effect fired with the initial `activeId === null` before the async restore could read the saved value back, permanently clobbering it to "" on every single reload. Fixed by capturing the saved id once, synchronously, on the very first render (`useRef`'s argument is only ever evaluated the first time) instead of re-reading storage later after it's already been stomped. - #4: the model selector reset to Claude on every reload instead of keeping whatever was last picked. Same storage pattern as #9. - #5: DFT Workbench's structure/settings/review state (and the real Download button on its chat card) didn't survive a reload -- `coreWorkspace` is a single instance created fresh on every mount, and the dft-bundle-ready card only ever lived in ephemeral React state, never the LangGraph checkpointer. Now persists structure input and calc settings and replays them into coreWorkspace on mount (a real re-run of inspect/review/generate against goldilocks-core, since goldilocks-workbench's Workspace has no cheaper "just restore state" action -- a deliberate, one-off exception to #7's no-silent-duplicate- real-execution rule, made explicitly for this reload case). The chat card itself is rebuilt from the checkpointer's raw tool-call history, cross-referencing each ToolMessage's tool_call_id back to the assistant message's tool_calls[] to recognize a past dft_download_bundle result. - #3: the "Starting goldilocks-core's Workbench backend..." message gave no indication that first-time setup downloads several hundred MB and can take minutes -- now names the slow case (~3 min normal, ~15 min with MLIP's mMACE model) instead of implying every start is that slow. - DftWorkbenchStructureTab's own Structure dropdown stayed on "No structures loaded"/"--" after uploading a file, even though the upload had already been loaded into DFT Workbench for real -- auto-selects the newly uploaded structure once it actually lands in chatStructures (matched by name, not a predicted index, since a same-name re-upload and a fresh upload land at different slots).
…acing fix) Advisors accordion rows (Functional, Vdw, Nbnd, ...) were visibly more spaced out than Analysis's own read-only records rows (Composition, Geometry, Symmetry, ...) -- 56.8px vs 24.8px, in both the full-page grid view and the inline Tools-panel view. Root cause (and the actual fix) lives in goldilocks-core's web/ source: CalculationForm's Advisors accordion opted into a custom class giving its rows Mantine's --mantine-spacing-md padding, but RecordAccordionItem (used by AnalysisSection for its own records) never got the same treatment -- a gap its own docstring already flagged as known and deferred. Filed and fixed as junwen94/goldilocks-core#123; this commit is just the resulting rebuild landing here, same file:vendor/*.tgz publishing path documented in the README (rebuilt via `npm run build:lib && npm pack` from core/web).
Resolves the selectedModel init conflict: shared deployment still locks to the local Qwen model (stfc/main's fix), non-shared restores whatever model was last picked instead of resetting to Claude (#4, this branch's own fix) -- both apply, gated on SHARED_DEPLOYMENT.
junwen94
added a commit
that referenced
this pull request
Oct 9, 2026
…mmits The "#3/#4/#5/#6/#7/#9" comments and README's "see issue #1" referred to junwen94/goldilocks-agent, which was deleted. On this repository the same numbers are unrelated PRs (#1 is the Ollama-backend PR). The comments now name the commits that fixed those bugs (cc5f0dc, c7bfc57), whose messages name the original issues. Refs #12
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.
No description provided.