fix: persist resolved default k/q/i-grid on unedited execution units (SOF-7990) - #161
Closed
VsevolodX wants to merge 3 commits into
Closed
fix: persist resolved default k/q/i-grid on unedited execution units (SOF-7990)#161VsevolodX wants to merge 3 commits into
VsevolodX wants to merge 3 commits into
Conversation
…(SOF-7990)
ExecutionUnit.savePersistentContext only kept explicitly edited context items, so
the default (unedited) path never wrote a kgrid entry into an execution unit's
persisted context. assign-precision-for-material and similar consumers reading
context["kgrid"] unconditionally then hit a KeyError on any workflow that never
had its k-grid touched by the user.
PointsGridFormDataProvider (kgrid/qgrid/igrid) now opts into persisting its
resolved default via a new isPersistedWhenNotEdited flag, checked alongside
isEdited in savePersistentContext's filter.
A persisted-but-unedited item is read back into the provider on the next
construction (getContextProvidersInstances seeds providers from unit.context
regardless of isEdited), so without guarding against it the resolved default
would pin to whichever material was rendered first instead of recomputing per
material. PointsGridFormDataProvider's constructor now discards inherited data
when the incoming item isn't edited, so every render recomputes fresh while
still persisting the result.
Also fixes convergence_mixin.py's bare get_context_item("kgrid").get(...),
which raised AttributeError instead of the intended ValueError when no kgrid
context item exists yet.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Doc comments on the new isPersistedWhenNotEdited flag and its call sites had grown into multi-paragraph essays, well past this codebase's actual convention of dense one-liners. Cut each to a single line. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Comments should explain the why, not point back at the ticket that motivated the change — that belongs in the PR description and commit history, not code that outlives it. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Member
Author
|
Closing: the premise was never verified. The KeyError at |
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.
Summary
ExecutionUnit.savePersistentContextonly kept explicitly edited context items, so the default (unedited) path never wrote akgrid/qgrid/igridentry into an execution unit's persistedcontext.assign-precision-for-materialand similar consumers readingcontext["kgrid"]unconditionally hit aKeyErroron any workflow that never had its k-grid touched by the user.PointsGridFormDataProvider(kgrid/qgrid/igrid) now opts into persisting its resolved default via a newisPersistedWhenNotEditedflag, checked alongsideisEditedinsavePersistentContext's filter.getContextProvidersInstancesseeds providers fromunit.contextregardless ofisEdited), so without a guard the resolved default would pin to whichever material was rendered first instead of recomputing per material.PointsGridFormDataProvider's constructor now discards inheriteddatawhen the incoming item isn't edited, so every render recomputes fresh while still persisting the result.convergence_mixin.py's bareget_context_item("kgrid").get(...), which raisedAttributeErrorinstead of the intendedValueErrorwhen nokgridcontext item exists yet.Test plan
npm test— 49 passing (baseline 47), no failing linepython -m pytest— 84 passed (baseline 81)npx tsc --noEmit— cleannpx eslint src/js tests/js— 0 errors, 5 pre-existing warningsWorkflow.test.tsinvariant fails whensavePersistentContextalone is reverted (flag left true) — both confirmed by deliberately reverting each fix in isolation and re-running.mat3ra/web-app(dependency bumped to this commit): a job created viaPUT /api/2018-10-01/jobs/createon the default k-grid path now persistskgridwith a sensible resolved grid, where before it wascontext: [].Notes for reviewers
reviews/mat3ra/wode/working-tree-2026-08-07/comments-c0f12bc.mdin the planning repo (not part of this diff).web-app, unrelated to this fix:globalSettings.defaultKPPRA(injected as10viaimports/workflows/context/registry.ts) doesn't reach the module instance of wode used during job rendering, which falls back to wode's internal default of5. Confirmed independent of this PR by reproducing the same behavior with the unmodified, currently-published@mat3ra/wode. Flagged separately, not addressed here.Co-Authored-By: Claude Sonnet 5 noreply@anthropic.com