Skip to content

Show in Found only the facts about the values Expected constrains - #485

Merged
CatarinaGamboa merged 1 commit into
mainfrom
fix/480-found-target-vars
Nov 5, 2026
Merged

CatarinaGamboa merged 1 commit into
mainfrom
fix/480-found-target-vars

Conversation

@CatarinaGamboa

@CatarinaGamboa CatarinaGamboa commented Nov 4, 2026 •

Copy link
Copy Markdown
Collaborator

Closes #480.

Problem

The message's Found listed every path condition of the method, so a guard on another variable read as the reason for the error, and nothing was said about the variable Expected constrains.

Change

The message keeps only the facts of Found linked to the terms of Expected: the same factsOf/linkedTerms the counterexample already uses, exposed as one package-private RefinementError.linkedToExpected that StateRefinementError reuses.

  • When facts remain, the frames are unchanged: X is not a subtype of Y and found X but expected Y. When every fact is linked, Found is shown as written (e.g. !(x < 0 || x > 5), not split).
  • When none remains (for a state error: none applies a ghost or state; an alias such as box == getBox says nothing about the state), the message reads expected <refinement> but could not prove it.

Only the message text changes. The counterexample, hints, the Found the IDE shows and the verification query are unchanged. TestGeneratedNames now reads the whole Found of refinement errors, since the message no longer lists unrelated generated names.

Before / after

Reproducer 1 (refinement):

before: Refinement Error: !(port < 0 || port > 65535) is not a subtype of timeout >= 0
after:  Refinement Error: expected timeout >= 0 but could not prove it
        Counterexample: timeout == -1 (unchanged)

Reproducer 2 (state):

before: State Refinement Error: found !(#isEmpty_result¹⁶ || #isEmpty_result¹⁷) && box¹⁵ == getBox¹³ but expected full(box¹⁵)
after:  State Refinement Error: expected full(box¹⁵) but could not prove it

Study exercise (JabRef EditCommand, fixture state_hint_study_undo_manager_error):

after:  expected redoable(undoManager) but could not prove it

Facts that relate to Expected stay in the usual frame:

  • !(port < 0 || retries > t) with Expected t >= 0 shows !(retries > t) is not a subtype of t >= 0;
  • a state reaching Expected through an alias: found empty(b1) && box == b1 but expected full(box).

Tests

Each fails without the fix:

  • RefinementErrorCounterexampleTest: refinement reproducer, partial guard, Found kept as written when every fact is linked, state kept through an alias.
  • TestStateHints: the study's undo manager (state reproducer).

Full suite passes locally (697 tests). Size: +99 / −18 over 5 files.

🤖 Generated with Claude Code

@CatarinaGamboa

Copy link
Copy Markdown
Collaborator Author

Codex review, round 1

  • [P2] Format unconstrained variable names together — FoundFacts.java:52-53
    When the expected refinement contains two qualified variables with the same simple name and neither is constrained by the found facts, formatting each Var separately loses the ambiguity context. Both names can appear as x, producing a misleading message such as x, x (no constraint). Format the variables in one expression so the name resolver can retain the necessary qualifiers.

@CatarinaGamboa

Copy link
Copy Markdown
Collaborator Author

Round 1 response:

  • Fixed: format unconstrained variable names together. New ExpressionFormatter.formatVariables names each variable with the resolver of the whole expected refinement, so qualifiers stay when simple names collide and the names match how Expected is written; one test in ExpressionFormatterTest.
  • Declined: none.

@CatarinaGamboa

Copy link
Copy Markdown
Collaborator Author

Codex review, round 2

  • [P2] Distinguish ghost values from their receiver variables — FoundFacts.java:38-40
    When the expected refinement is size(x) > 0 and found only establishes x != null, this term match retains x != null as a relevant constraint. The resulting message omits the “no constraint” wording and hint even though nothing constrains size(x). Treat variables used only as ghost arguments separately from values the expected refinement constrains.

  • [P2] Require state information relevant to the expected application — StateRefinementError.java:59-61
    If found establishes size(x) == 0 but the method requires open(x), any function invocation makes isKnown return true. The error then presents the size fact as known state information and suppresses the unknown-state hint, although it says nothing about open(x). Check whether the found application constrains the state required by the expected refinement.

@CatarinaGamboa

Copy link
Copy Markdown
Collaborator Author

Round 2 response:

  • Declined: ghost arguments vs values. x != null is a fact about a variable of Expected, so showing it is accurate; the linking is the same one the counterexample already uses, and the "(no constraint)" wording is reserved for when Found says nothing at all about Expected's variables. Splitting ghost arguments from values would make the message and the counterexample disagree.
  • Declined: require the same ghost application. Requiring the found application to match the expected one would hide the most useful case, found closed(x) but expected opened(x) (another state of the same set). Any ghost application over the object is information about it; on main the general hint was likewise suppressed whenever Found was not true, so this is no regression.

Nothing relevant left, so the review loop stops here.

@CatarinaGamboa

Copy link
Copy Markdown
Collaborator Author

Codex review, round 3

  • [P2] Limit unknown-state descriptions to state receivers — StateRefinementError.java:51-52
    When a state precondition includes an ordinary guard, such as closed(this) && attempts > 0, and the found facts contain no state application, describeVariables labels every expected variable as being “in an unknown state.” The resulting message says that the integer attempts has an unknown state. Select the receivers of the expected state applications for this wording, rather than all variables in the predicate.

@CatarinaGamboa

Copy link
Copy Markdown
Collaborator Author

Round 3 response:

  • Fixed: limit unknown-state descriptions to state receivers. FoundFacts.describeStateReceivers names only the arguments of Expected's ghost/state applications (all variables if it has none), so closed(box) && attempts > 0 reads found box in an unknown state; one test in RefinementErrorCounterexampleTest.
  • Declined: none.

@CatarinaGamboa

Copy link
Copy Markdown
Collaborator Author

Codex review, round 4

  • [P2] Check state information for the expected object — StateRefinementError.java:63-64
    When the expected refinement combines a receiver state with a ghost predicate on another value—for example, full(box) && capacity(other) > 3—and found contains only capacity(other) > 3, this check treats the state as known because some ghost application appears in shown. The error then reports the guard as the found state and omits the “No state information is known” hint, even though nothing is known about box. Check whether the shown application concerns the object whose state is required.

@CatarinaGamboa

Copy link
Copy Markdown
Collaborator Author

Round 4 response:

  • Declined: check state information for the expected object. other is itself an object Expected applies a ghost to, so found capacity(other) > 3 but expected full(box) && capacity(other) > 3 shows a true state fact about Expected's objects, which is the agreed rule for keeping the found X but expected Y frame. The specific state hints (NEEDED_STATE etc.) are computed separately and still name box; the general hint was already suppressed on main whenever Found was not true.

Nothing relevant left.

@CatarinaGamboa
CatarinaGamboa force-pushed the fix/480-found-target-vars branch from c664547 to 6747d1e Compare November 5, 2026 11:58
@CatarinaGamboa

Copy link
Copy Markdown
Collaborator Author

Codex review, round 5

  • [P2] Distinguish state predicates from other ghost calls — StateRefinementError.java:55-57
    When a state check expects full(box) but Found contains only a linked value ghost such as size(box) > 0, appliesGhost returns true and the message says found size(box) > 0 but expected full(box). A FunctionInvocation can represent a value ghost, not a state, so this bypasses the intended “could not prove it” message despite no known state for box. Check whether the linked fact supplies state information rather than treating every ghost call as one.

@CatarinaGamboa

Copy link
Copy Markdown
Collaborator Author

Round 5 response:

  • Declined: distinguish state predicates from value ghosts. A ghost value of the object is part of what its typestate refines (states may be defined over ghosts, e.g. from = "size(this) > 0"), so found size(box) > 0 but expected full(box) shows a true, relevant fact about box. Telling the two apart would need the context's state sets in the error, for a case no test or study exercise reaches (same point as round 2).

Nothing relevant left.

The message's Found listed every path condition of the method, so a guard
on another variable read as the reason for the error. It now keeps only
the facts linked to the terms of Expected, the ones the counterexample
already keeps, and reads 'expected <refinement> but could not prove it'
when none is left (for a state error, when none applies a state). The
Found the IDE shows and the verification query are unchanged.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@CatarinaGamboa
CatarinaGamboa force-pushed the fix/480-found-target-vars branch from 6747d1e to b8f2b56 Compare November 5, 2026 12:08
@CatarinaGamboa

Copy link
Copy Markdown
Collaborator Author

Codex review, round 6

I found no actionable regressions in the diff. The targeted tests pass.

@CatarinaGamboa

Copy link
Copy Markdown
Collaborator Author

Round 6 response: no findings, nothing to fix or decline. Review loop done.

@CatarinaGamboa
CatarinaGamboa merged commit 2a9b40d into main Nov 5, 2026
1 check passed
CatarinaGamboa added a commit that referenced this pull request Nov 5, 2026
Refs #375.

Follow-up to #476 and #485, approved by the PM. The simplified Found
(the last step the VS Code panel shows) no longer lists path conditions
that Expected does not depend on.

## Problem

#485 trims the message to the facts linked to Expected. The panel's last
step still listed every `if`/loop guard, so the message and the panel
disagreed. In the study exercises:

- A1_56: the message is `expected redoable(undoManager) but could not
prove it`, but the panel still shows two `switch` guards.
- A1_01: the panel shows `baseline || extended`, which says nothing
about `wParam`.

## Rule

A binder of a generated boolean (a path condition such as
`#if_condition`) is dropped when both hold:
- **Not linked to Expected:** neither its variable nor any term of its
refinement shares a term with Expected, directly or through other facts
of Found.
- **Not mentioned by another row**, so no remaining row refers to a
variable whose binder was dropped.

Rows that are not generated booleans are never dropped by this rule; the
message still trims those (#485), so the message is always a subset of
the panel's last step. Without expected terms (`NeededVariables.NONE`),
nothing is dropped.

Each removal is a step of the existing `Binder` pass. **Stepping back in
the panel still shows the original facts:** the root of the
`VCSimplificationResult` origin chain keeps every dropped guard. I
checked this for the A1_56 shape (each guard is its own `Binder` step
and the root has both), and `TestGeneratedNames` now reads the original
Found, so it checks it on every run.

**One source for the linking.** `factsOf`, `linkedTerms`, `termsOf`,
`termName` and `linkedToExpected` moved from `RefinementError` to
`VCSimplificationUtils`, so simplification no longer needs anything from
diagnostics. `RefinementError` (message and counterexample),
`StateRefinementError` (message) and `NeededVariables` (panel) now all
use them. `VCSimplificationUtils` had its own `termsOf` that named
applications by `toString()`. Now applications over variables are named
as in the SMT model (`size(x)`), which the counterexample needs, and the
other applications are named as written (`f(1)`). All tests pass with
that change.

The "could not prove it" frame, the shortcut that keeps Found as written
when every fact is linked, and the counterexample are unchanged.

## Before / after (the panel's last step)

The rows below are the simplified chain exactly as
`VCImplication.toString()` prints it, from `origin/main` and this
branch.

A1_56 (JabRef `EditAction`, line 69). The message is unchanged on both:
`expected redoable(undoManager¹⁴¹) but could not prove it`.
```
before: ∀##if_condition_122:boolean, !##operation_result_121  =>
        ∀##if_condition_136:boolean, !##operation_result_134 || ##operation_result_135  =>
        ∀#undoManager_141:CountingUndoManager, #undoManager_141 == ##getUndoManager_result_139
after:  ∀#undoManager_141:CountingUndoManager, #undoManager_141 == ##getUndoManager_result_139
```

A1_01 (dcm4che `SlideRecoder`, line 131). The message is unchanged:
`found startTiling(wParam²⁹⁷) && startCompression(wParam²⁹⁷) but
expected compressionExplicit(wParam²⁹⁷)`.
```
before: ∀##if_condition_302:boolean, #baseline_298 || #extended_299  =>
        ∀#wParam_297:ImageWriteParam, startTiling(#wParam_297) && startCompression(#wParam_297)
after:  ∀#wParam_297:ImageWriteParam, startTiling(#wParam_297) && startCompression(#wParam_297)
```

A guard Expected depends on stays
(`ErrorCounterexampleLinkedPathCondition`). The message is unchanged:
`!(port < 0) && timeout¹⁰ == port - 1 is not a subtype of timeout¹⁰ >=
0`.
```
before: ∀##if_condition_0:boolean, !retries > 3  =>
        ∀##if_condition_4:boolean, !port < 0  =>
        ∀#timeout_10:int,    #timeout_10 == port - 1
after:  ∀##if_condition_4:boolean, !port < 0  =>
        ∀#timeout_10:int,    #timeout_10 == port - 1
```
`retries > 3` is dropped. `port < 0` is linked through `timeout == port
- 1` and stays.

In all three cases the original step still shows every guard.

No screenshots: the `liquidjava-dev` harness has no panel-capture script
yet, and the extension server would need this verifier built into it, so
I skipped them.

## Tests

`VCBinderSimplificationTest` has 4 new tests. Each fails without the
change.
- `removesPathConditionsExpectedDoesNotDependOn`: the A1_56 shape,
including a negated disjunction.
- `keepsPathConditionExpectedDependsOnThroughAnotherRow`: a guard linked
through `y == x - 10` stays while an unlinked one goes.
- `keepsPathConditionAnotherRowMentions`: a guard whose variable a
ternary row uses stays.
- `keepsPathConditionsExpectedNames`: a guard whose own variable
Expected names stays, and so does a guard on a call Expected names
(`f(1)`, Codex round 1).

**Changed:** `TestGeneratedNames` now reads the original Found instead
of the simplified one, because the simplified one no longer has the
guards whose names it checks. Only line 54 changes: it is now `isNull ==
#operation_result && #if_condition == isNull && #if_condition && x ==
0`, the alias as written before simplification.

`mvn test`: **702 tests, 0 failures**.

**Size:** +192 / −108 over 7 files. About 90 of the removed lines are
the helpers that moved out of `RefinementError`.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Found lists facts about other variables and nothing about the one the expected refinement constrains

1 participant