Repository navigation
Show a subclass's call on the state set it inherits from a superclass's spec - #174
Conversation
Codex review, round 1Run over both diffs (this PR and liquid-java/liquidjava-fsm#16). Codex's findings, verbatim: I found two issues in the server’s file selection. I could not run tests, as requested.
|
Round 1 replies
Server |
Codex review, round 2Run over both diffs (this PR and liquid-java/liquidjava-fsm#16). Codex's findings, verbatim: I found two correctness gaps and one language server performance risk. I did not run commands or tests, as requested.
The new test at |
Round 2 replies
Server |
Codex review, round 3Run over both diffs (this PR and liquid-java/liquidjava-fsm#16). Codex's findings, verbatim: I found three correctness gaps in the supplied changes. I reviewed the provided code only; I did not run commands.
|
Round 3 replies
No code change in this round. |
Codex review, round 4Run over both diffs (this PR and liquid-java/liquidjava-fsm#16). Codex's findings, verbatim: I found two correctness gaps and one test gap across the PRs. I could not run tests, as requested.
|
Round 4 replies
Codex's remaining findings repeat ones already declined, so the review loop stops here. No code change in this round. |
…'s spec When a state error was on a method declared in a subclass's spec (Frame's setUndecorated) and the state set in a superclass's spec (Window), the State Machine view showed no diagram for the error, or Window's diagram without the call and without any marked state. The diagrams of a state error are now parsed for the receiver's class with liquidjava-fsm 0.2.3, which merges the specs of its supertypes as the verifier does. Besides the declaration's file, the parser reads the files of the state sets the verifier knows, where those specs may be. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A supertype's spec without a state set, such as Frame's between JFrame and Window, was not read, so its methods were missing from the diagram. The server now also passes the files of the verifier's state refinements. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
a0b32aa to
974043b
Compare
Build: vscode-liquidjava main 7292a64 (#179) + #178 (04246ac) + #174 (974043b), merged locally; verifier 0.0.37, fsm 0.2.3 from Central. Files named *-ways-to-fix-pr172.png add #172 (6d41838) on top. Exercises: building-examples-liquidjava proposal-d-state-values, api 0.0.8. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…rong (#182) Closes #180. Based on main (#178 and #174 are merged there); not stacked on #172 (see below). ## What changes, per proposal When a call needs the states of several state sets (Door: `d.lock(); d.open();`, `open` needs `doorState(this) == closed && lockState(this) == unlocked`): - **Failure marked where it is.** Which set is wrong comes from the server's `errorContext` per diagram, not from message text: a set whose `invalidStates` is not empty is in a wrong state; a set whose possible states are all accepted, with another set in a wrong state, is "right". In a right set, a transition of the call from the actual state whose condition names the wrong set is drawn as usual (white, normal arrow) and only the clause the actual state does not meet is red, also when conditions are collapsed. In the wrong set the call is red as before. Every other transition of the call stays red as now: when it has no condition on the wrong set, when the condition is not only states, when no set is wrong on its own (the verifier can only reject the combination), or when the state is unknown. - **Wrong set first.** `orderStateMachines` puts the sets in a wrong state first, then the other sets of the error, then the rest. Focus mode fits the first diagram as before, so the ✕ state is now on screen (A1_38: `mark` first, the case from the IDE review). - **Self-loops.** A transition that keeps its state is drawn as a round loop (same ends, same side, as far out as before so it stays inside the laid-out SVG, slightly thicker) instead of ELK's flat curl the size of its arrowhead. It is redrawn after Mermaid renders, from a `%% lj-self-loops:` comment like the existing dimmed-transitions one. - **Conditions as in the spec, before and after the method.** A label reads `[condition before] method [condition after]` on one line, e.g. `[lockState(this) == unlocked] open`. Each condition is the spec text, so it reads like the Expected box and the spec, and a condition with other clauses stays readable: in `[lockState(this) == unlocked && size(this) > 0] open` only the clause the actual state does not meet is red. Edge labels no longer wrap at Mermaid's 200px width, so a condition stays on the line of its method. - **Set names in the summary.** "expected doorState `closed`, lockState `unlocked` · actual lockState `locked`": expected in declaration order; actual lists only the sets in a wrong state (all known ones if none is). A set in which every state allows the call is left out of both sides, for named and unnamed sets (the A1_56 "(undoable or notUndoable) and redoable" case). Unnamed sets keep the current wording. Single-set classes are unchanged except for the round self-loops and the summary naming a named set (`expected phase open · actual phase closed`). ## Ways to fix and #172 Not in this PR. Highlighting `unlock` on the diagram needs to know which calls a hint suggests, and `HintDTO` only has `kind` and `message`; getting the calls from the message text, or from paths in the diagram (judged unsound in #171), are both out. #172 lists the verifier's hints in a "Ways to fix" section and does not draw them; drawing a hint is #156, which needs a structured hint from the verifier first. So this PR does not depend on #172 and does not touch the hints; the two only meet in `fsm.ts`/`script.ts` call sites and should rebase cleanly in either order. ## Screenshots Headless VS Code, this branch's server (verifier 0.0.37, fsm 0.2.3). Focus mode is what opens from the error. **Door** (the spec from #178, `d.lock(); d.open();`): | | Before (main) | After | |---|---|---| | Focus on error | <img src="https://raw.githubusercontent.com/liquid-java/vscode-liquidjava/pr-screenshots/180/door-focus-before.png" width="380"> | <img src="https://raw.githubusercontent.com/liquid-java/vscode-liquidjava/pr-screenshots/180/door-focus-after.png" width="380"> | | Show all | <img src="https://raw.githubusercontent.com/liquid-java/vscode-liquidjava/pr-screenshots/180/door-all-before.png" width="380"> | <img src="https://raw.githubusercontent.com/liquid-java/vscode-liquidjava/pr-screenshots/180/door-all-after.png" width="380"> | **Study exercise A1_38** (BufferedInputStream, sets `stream` and `mark`; `buffered.reset()` without `mark()`; proposal-D-task-types with liquidjava-api 0.0.8): | | Before (main) | After | |---|---|---| | Focus on error | <img src="https://raw.githubusercontent.com/liquid-java/vscode-liquidjava/pr-screenshots/180/a138-focus-before.png" width="380"> | <img src="https://raw.githubusercontent.com/liquid-java/vscode-liquidjava/pr-screenshots/180/a138-focus-after.png" width="380"> | | Show all | <img src="https://raw.githubusercontent.com/liquid-java/vscode-liquidjava/pr-screenshots/180/a138-all-before.png" width="380"> | <img src="https://raw.githubusercontent.com/liquid-java/vscode-liquidjava/pr-screenshots/180/a138-all-after.png" width="380"> | The right set, scrolled down in focus mode (call drawn as usual, unmet condition red). These two and the A1_38 shots are from before the change to conditions as in the spec, which they still show as `[lockState: unlocked]`; the Door shots above are current: | Door `doorState` | A1_38 `stream` | |---|---| | <img src="https://raw.githubusercontent.com/liquid-java/vscode-liquidjava/pr-screenshots/180/door-focus-second-after.png" width="380"> | <img src="https://raw.githubusercontent.com/liquid-java/vscode-liquidjava/pr-screenshots/180/a138-focus-second-after.png" width="380"> | ## Not in this PR - Marking states that are current and fine (`closed` in `doorState`, `open` in `stream`), as agreed. - Highlighting the ways to fix on the diagram (above). ## Reviews - Codex: 5 rounds (comments below); rounds 1-4 found one or two issues each, all fixed; round 5 found nothing. - Clean-code audit: comment below; one fix, the rest declined with reasons. ## Tests - client: `npm run test:unit` 128 passed (5 new, each fails without the change: order, call/condition marks with `!=` and collapsed conditions, round loop, named summary without the any-state set; one existing expectation gains the self-loop comment), `npm run test:typecheck` and `npm run lint` clean. - server: no change; `mvn -B test` 39 passed. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Closes #173.
Uses liquidjava-fsm 0.2.3 (which includes liquidjava-fsm#16) and verifier 0.0.37, both on main since #179. Rebased onto main on 2026-11-10: the
server/pom.xmlbump is gone; server tests pass (39 with #178 merged in too).Why
A state error can be on a method declared in a subclass's spec (
Frame.setUndecorated) while the state set is declared in a superclass's spec (Window). In that case the State Machine view could not explain the error:Window's diagram withoutsetUndecoratedand marked no states (the issue's screenshot).java.awt.Frame. The parser then foundFrame's spec, which has no@StateSet, so the error had no diagram at all.What
StateRefinementErrorDTO. It parses the diagrams for the context'sreceiverClass. It reads the declaration's file plus the files of the state sets and state refinements the verifier registered (ContextHistory), so specs split across files are found, including a supertype spec with methods but no state set. Without a context it behaves as before (first class of the declaration's file).No verifier change is needed. liquidjava#464 already sends the context with the receiver's states for this case. The view needs a verifier release with #464 (after 0.0.36). With 0.0.36 nothing changes for this case: that verifier sends no context, so the view still shows the first class of the declaration's file (
Windowin the issue's file).Screenshots
Both columns use verifier main @ 9e4b5e64.
host.pack(); host.setUndecorated(true);)GameWindow)In A1_09 the actual state shows as "unknown" because the verifier finds both
displayableandnotDisplayablepossible (it depends on the path).notDisplayableis marked as the expected state, andsetUndecoratedis the failing call.I did not take a split-files screenshot. With
FrameRefinements.javaread beforeWindowRefinements.java, the verifier stops with liquid-java/liquidjava#461 before it reaches the state error.StateRefinementErrorDTOTestcovers that layout.Tests
mvn test: 39 passed, 0 failures (liquidjava-fsm 0.2.3 built locally from Update README #16, verifier 0.0.36). NewStateRefinementErrorDTOTest:java.awt.Frame, withsetUndecorated,disposeandpackonWindow's states;displayableis marked invalid andnotDisplayableaccepted.JFramespec over a method-onlyFramespec andWindow's state set: all their methods are drawn.npm run test:unit: 123 passed.npm run test:typecheck: clean.npm test(VS Code): 9 + 1 passing.Overlap with #172
None. #172 changes only the client webview. This PR changes only the server and its fsm dependency.
🤖 Generated with Claude Code