Repository navigation
Label a named state set's diagram with its name - #178
Merged
Merged
Conversation
CatarinaGamboa
added a commit
that referenced
this pull request
Nov 9, 2026
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Collaborator
Author
Codex review, round 1
ResponseKnown and intended: the PR body says it depends on a liquidjava-fsm 0.2.3 release of liquid-java/liquidjava-fsm#17, as #169 did with 0.2.2. The server was tested locally against #17 installed as 0.2.3 in an isolated Maven repository. Nothing else was found; no changes in this round. |
Collaborator
Author
Codex review, round 2
|
Collaborator
Author
Clean-code reviewAudit of this PR's diff against the clean-code-principles checklist (
No findings to fix; no changes. |
This was referenced Nov 9, 2026
Merged
CatarinaGamboa
force-pushed
the
feat/state-values
branch
from
November 10, 2026 00:36
c69aaf5 to
63a6473
Compare
CatarinaGamboa
changed the base branch from
main
to
chore/bump-verifier-0.0.37
November 10, 2026 00:36
CatarinaGamboa
added a commit
that referenced
this pull request
Nov 10, 2026
Picks up verifier 0.0.37 (named state sets, states written as values of their sets, `x@line` display names, hints and found trimming since 0.0.36) and fsm 0.2.3 (reads named sets and the value form; built on verifier 0.0.37). Server changes are only in `DiagnosticConverterTest`: - `RefinementError` takes the refinement's aliases and `StateRefinementError` how it shows its states (`UnaryOperator<Expression>`); the tests pass `List.of()` and `UnaryOperator.identity()`. - `VariableFormatter.format` now writes `#value_12` as `value` (no superscript counter); variables that would read alike are told apart by the verifier as `x@line`. The translation-table test expects `value`. The client still knows superscript counters (`highlighting.ts`, `vc-changes.ts`, a fixture in `state-errors.spec.ts`); rendering the `x@line` names is #175. Server: 35 tests pass locally. #178 (named set titles) goes on top of this. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
CatarinaGamboa
changed the base branch from
chore/bump-verifier-0.0.37
to
main
November 10, 2026 00:53
The verifier's messages now show a state of a named set as a value of its set, doorState(d) == closed. liquidjava-fsm 0.2.3 reads these specs and reports each diagram's stateSetName, which the server passes on and the State Machine view uses as the diagram's title, also when the class has a single state set, so the diagram can be matched with the message. A state set without a name keeps its states as the title. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
CatarinaGamboa
force-pushed
the
feat/state-values
branch
from
November 10, 2026 00:53
63a6473 to
04246ac
Compare
CatarinaGamboa
added a commit
that referenced
this pull request
Nov 10, 2026
…'s spec (#174) Closes #173. Uses liquidjava-fsm 0.2.3 (which includes [liquidjava-fsm#16](liquid-java/liquidjava-fsm#16)) and verifier 0.0.37, both on main since #179. Rebased onto main on 2026-11-10: the `server/pom.xml` bump 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: - With verifier 0.0.36 (no state context), it showed `Window`'s diagram without `setUndecorated` and marked no states (the issue's screenshot). - With verifier main (liquid-java/liquidjava#464), the context names `java.awt.Frame`. The parser then found `Frame`'s spec, which has no `@StateSet`, so the error had no diagram at all. ## What - **liquidjava-fsm#16.** The parser merges the specs of a class's supertypes, following the rule from liquidjava#464: state sets of the class and then of its supertypes, nearest first; for each method, the class's own refinement, otherwise that of the nearest type that declares it. The details are in that PR. - **`StateRefinementErrorDTO`.** It parses the diagrams for the context's `receiverClass`. 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). - **Known limit.** A project class that extends an unannotated project class in a file the verifier registered nothing for stops the walk there, so that class's diagram leaves out the states further up. The parser only sees the spec files. 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 (`Window` in the issue's file). ## Screenshots Both columns use verifier main @ 9e4b5e64. | | Before (main) | After | |---|---|---| | Issue reproducer (`host.pack(); host.setUndecorated(true);`) | <img src="https://raw.githubusercontent.com/liquid-java/vscode-liquidjava/pr-screenshots/174/before-issue.png" width="380"> | <img src="https://raw.githubusercontent.com/liquid-java/vscode-liquidjava/pr-screenshots/174/after-issue.png" width="380"> | | Study exercise A1_09 (LITIENGINE `GameWindow`) | <img src="https://raw.githubusercontent.com/liquid-java/vscode-liquidjava/pr-screenshots/174/before-a1-09.png" width="380"> | <img src="https://raw.githubusercontent.com/liquid-java/vscode-liquidjava/pr-screenshots/174/after-a1-09.png" width="380"> | In A1_09 the actual state shows as "unknown" because the verifier finds both `displayable` and `notDisplayable` possible (it depends on the path). `notDisplayable` is marked as the expected state, and `setUndecorated` is the failing call. I did not take a split-files screenshot. With `FrameRefinements.java` read before `WindowRefinements.java`, the verifier stops with liquid-java/liquidjava#461 before it reaches the state error. `StateRefinementErrorDTOTest` covers that layout. ## Tests - server `mvn test`: 39 passed, 0 failures (liquidjava-fsm 0.2.3 built locally from #16, verifier 0.0.36). New `StateRefinementErrorDTOTest`: - Specs in two files: the diagram is `java.awt.Frame`, with `setUndecorated`, `dispose` and `pack` on `Window`'s states; `displayable` is marked invalid and `notDisplayable` accepted. - Without a context: the first class of the declaration's file, as before. - A `JFrame` spec over a method-only `Frame` spec and `Window`'s state set: all their methods are drawn. - The verifier run on that fixture: the files of all three specs are selected. - client `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](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
CatarinaGamboa
added a commit
that referenced
this pull request
Nov 10, 2026
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>
CatarinaGamboa
added a commit
that referenced
this pull request
Nov 11, 2026
…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>
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.
Brings liquidjava-fsm#17 to the extension, for the verifier's named state sets and states written as values of their sets (liquid-java/liquidjava#498, #510, #511; design in liquid-java/liquidjava#502).
Depends on a liquidjava-fsm 0.2.3 release of liquidjava-fsm#17: until it is on Maven Central,
serverdoes not build in CI.What changes
server/pom.xml: liquidjava-fsm 0.2.2 → 0.2.3, which readsphase(this) == open,open == phase(this)andphase(this) != openinfrom/toas the states they name, and reports each diagram'sstateSetName.StateMachineDTO: passesstateSetNameto the client (theliquidjava/fsmrequest returns the fsm records, so it gets it as is).doorState(d) == closed). A state set without a name keeps its states as the title, as now.Checked, no change needed
The verifier sends the same shapes to the IDE as before (
git diff fbc5e9f6 origin/main):StateErrorContextstill names states by their names, and hints are unchanged. Only the text of a state error's message,expectedandfoundchanges, todoorState(d) == closedfor named sets. The client renders those strings as they come, with the generic highlighter, and nothing in the client parses state names out of message text: the diagram marks and the summary line useerrorContext. The verification view shows the new form as is (last screenshot).Screenshots
Headless VS Code with this branch, liquidjava-fsm#17 installed locally as 0.2.3 and the verifier built from liquid-java/liquidjava main, on this spec (
d.lock(); d.open();as the error):Before (main, fsm 0.2.2), the diagrams are wrong:
openandlockgo from every state with the wholefromas a condition,closeandunlockare missing, the summary only nameslockState, and the titles are the states. After, the diagrams match the spec, the conditions keep the form the spec wrote, and the titles are the set names:The verifier's message for the same error (no change here; shown for context):
Not in this PR
DiagnosticConverterTestneeds the newStateRefinementErrorconstructor argument (display, from #511) and theRefinementErrorone (aliases); the server code itself compiles against it.Tests
vitest run124 passed (one new test, fails without the change), typecheck and lint cleanmvn test35 passed, against liquidjava-fsm#17 installed as 0.2.3 in an isolated local Maven repository🤖 Generated with Claude Code