PG1204 commented on PR #8301:
URL: https://github.com/apache/texera/pull/8301#issuecomment-5520608392
> ๐ก **1 resolved ยท 1 open ยท 0 new**
>
> The rework lands the fix: the snapshot assertion now runs where a failure
can fail the test. Nothing new this round, and the only open item is the naming
note carried over on unchanged code.
>
> ### Design & architecture (1)
> * `workflow-editor.component.ts:586` โ `applyOperatorBorder` still
repaints the full execution-state rendering, not just the border (advisory,
carried over from the last two rounds)
>
> Verification trace
> Checked the fix rather than the reply. The assertion is now outside the
subscriber (`workflow-status.service.spec.ts:120`), with only the capture left
inside at `:112`, so a failure surfaces synchronously instead of escaping as an
unhandled rxjs error. Traced both ways the guarantee can break: if the two
`next()` calls swap, or if the state snapshot is never written, `currentState`
is still the constructor's initial `{}` at capture time, so the captured value
is `{}` and the assertion fails. The capture-by-reference is sound because the
service reassigns `this.currentState = state` rather than mutating it, so the
captured object is the one emitted for this event.
>
> Nothing else moved: the delta is one file, +8/โ3, and the other 17 changed
files are byte-identical to the last reviewed head. Re-diffed the two smallest
consumers against the merge base this round โ `udf-debug.service.ts:159` and
`result-table-frame.component.ts:157` both replace `map[id]?.operatorState ===
X` with `map[id] === X`, equivalent including for an absent operator โ and
confirmed the operator-add path's `if (statistics)` guard already existed in
the base, so the new early return in `changeOperatorStatistics` replaces a
branch that was unreachable there. No reference to `getStatusUpdateStream` /
`getCurrentStatus` / `currentOperatorStatus` survives in `frontend/src` or
`agent-service/src`.
The full-state repaint is deliberate and is what fixes the state label
vanishing after navigating away and back (an invalid operator previously kept
only its red stroke on restore, pinned by the "restores the execution-state
label" spec). But you're right that the name stopped matching the job two
rounds ago, which has been renamed to applyOperatorStateAndBorder.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
