fix(pl-middle-layer): re-render production when a block's render failed outright - #1784
Open
vgpopov wants to merge 1 commit into
Open
fix(pl-middle-layer): re-render production when a block's render failed outright#1784vgpopov wants to merge 1 commit into
vgpopov wants to merge 1 commit into
Conversation
…outright The Run button is enabled whenever a block's production carries an error, but the mutator recognised only one of the two shapes such a failure takes. `productionHasErrors` read the field's `status`, and that status is derived from the resource the field points at — so it saw a value resource that exists and carries an error, and missed a field whose own error slot is filled, which has no value resource at all. A block in that second shape reports `outputErrors` to the desktop, so its Run button is live, yet `renderProduction` finds nothing to re-render for it and commits an empty transaction: the click is accepted and the block never re-runs. Nothing tells the user their retry was dropped. `f.error` is now carried through `ProjectMutator.load` alongside the value reference, and both shapes are folded into one `hasError` predicate that `productionHasErrors` applies to all three production fields. The condition the desktop enables the button on and the condition the mutator renders on are the same condition again. Carrying the flag in both branches of the hydration is the load-bearing part: an errored field has a null value, so it takes the branch that has no ref, and setting the flag only where a ref exists would have changed nothing.
🦋 Changeset detectedLatest commit: cbe94c7 The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
❌ 1 Tests Failed:
View the full list of 1 ❄️ flaky test(s)
To view more test analytics, go to the Test Analytics Dashboard |
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
2 participants
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.
Problem
Press Run on a block whose production failed and nothing happens. The click is accepted — the desktop logs
Task: run-block | Success: truein ~300ms — and the block never re-runs. No error, no feedback, no state change.Reproduced against a real project: a block sitting at
calculationStatus: "Done",stale: false,outputErrors: true,canRun: true, with 4 of 8 outputs failed. Seven consecutive Run dispatches, seven successes, zero render activity.Cause
Two places decide whether a block's production "has errors", and they disagree.
The button-enable rule —
middle_layer/project_overview.ts:117-121— is a 4-way disjunction:The actually-render rule —
mutator/project.tsproductionHasErrors— read only the fieldstatus, and that status is derived at hydration fromgetResourceData(f.value).error. So it covered the last two disjuncts and missed the first two.The gap is in hydration.
FieldData(pl-client/src/core/types.ts:126-135) carriesvalueanderror, butProjectMutator.loadread onlyvalue. A field whose own error slot is filled has a null value, so it was stored as a bare{ modCount: 0 }— no ref, no status, no trace the failure ever happened.Downstream:
productionRenderedis true (the field state object exists),productionStaleis false (args unchanged),productionHasErrorsis false — sorequireProductionRenderingis false,renderProductionselects nothing, and the transaction commits empty.Fix
BlockFieldStategainsfieldError, set fromf.errorduring hydration — in both branches, since an errored field takes the branch that has no ref.hasError()folds the two failure shapes into one predicate.productionHasErrorsapplies it toprodCtx,prodOutput,prodUiCtx.requireProductionRenderingis the only consumer, and it is reached only from the explicit user actionProject.runBlock, so the blast radius is one code path.Verification
tsc --noEmit— exit 0formatter:check— cleanlinter:check— 0 warnings, 0 errorsrequireProductionRenderingis the sole readerNot runtime-tested.
pl-middle-layer's suite is integration-only (TestHelpers.withTempRootneeds a livepl), and there is no fixture block pack that fails production, so a test needs both a backend and a new fixture. Happy to add it if wanted.That the observed block takes the newly-covered shape follows by elimination rather than by reading its resource: the two disjuncts the mutator already saw map onto
status === "Error", which must be false or Run would have re-rendered — so the true disjunct is one of the two that were dropped.Follow-ups (not in this PR)
forcepath forrunBlock, so an explicit Run never depends on a staleness predicate being right.get_project_overviewinpl-mcp-serverdropsoutputsError/exportsError, so output error text is unreadable headlessly.Greptile Summary
This PR preserves field-level production errors while hydrating project mutation state, allowing an explicit Run action to rerender production that failed before creating a value resource.
BlockFieldState.fieldError, which records whether the project field’s own error slot is populated.hasError(), which unifies field-level failures with errors carried by an existing value resource.productionHasErrorsto use the unified predicate for production context, output, and UI context fields.@milaboratories/pl-middle-layer.BlockFieldState— the mutator’s hydrated state for a block project field; it now retains afieldErrorflag alongside its reference, resource status, and value.fieldError— indicates that a project field’s own error slot is populated, including outright render failures that leave no value resource; newly captured fromFieldData.error.FieldStatus— describes the state of an existing value resource, including"Error"; unchanged, but now complemented byfieldError.hasError()— the new common predicate for the two production failure representations: a field-level error or an errored value resource.productionHasErrors— determines whether failed production requires rendering again; changed to applyhasError()toprodCtx,prodOutput, andprodUiCtx.Confidence Score: 5/5
The PR appears safe to merge, with the newly retained field-error state consistently feeding the existing explicit production-rerender path.
The change preserves an error signal previously discarded during hydration and combines it with the existing value-resource error state without changing public interfaces or introducing a demonstrated incorrect execution path.
Important Files Changed
Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart TD A[ProjectMutator loads production fields] --> B{Field has its own error?} B -- Yes --> C[fieldError = true] B -- No --> D[fieldError = false] C --> E[hasError] D --> E F{Value resource status is Error?} --> E E --> G{productionHasErrors} G -- Yes --> H[requireProductionRendering] H --> I[Explicit Run rerenders production]Reviews (1): Last reviewed commit: "[MILAB-XXXX]: fix: re-render production ..." | Re-trigger Greptile
Context used: