Skip to content

Drop the bare true row left at the end of Found - #476

Merged
CatarinaGamboa merged 3 commits into
mainfrom
study-simplify-trues
Nov 4, 2026
Merged

CatarinaGamboa merged 3 commits into
mainfrom
study-simplify-trues

Conversation

@CatarinaGamboa

@CatarinaGamboa CatarinaGamboa commented Nov 4, 2026 •

Copy link
Copy Markdown
Collaborator

Closes #375.

Problem

#459 dropped the true binders the error does not need. When the dropped binder was the last one of Found, VCBinderSimplification.removeBinder replaced it with a bare true row, so Found still ended in true after the path conditions. That is the case of the issue's screenshot: wParam comes from writer.getDefaultWriteParam(), nothing is known about its state, and its ∀wParam true row turned into a bare true that reads as if baseline || extended were true.

Change

In VCBinderSimplification, a suffix that simplifies to just true is left out of the row above it (withNext/isPlainTrue). This covers a dropped last binder and a collapsed false binder. It happens in the same Binder step, so the panel's history gets no extra step.

  • A Found that simplifies away entirely still shows a lone true (found true but expected …). Nothing is known, and an empty Found would hide that.
  • Rows with content, path conditions and binders the error needs are unchanged. The message's found … text doesn't change either, because P && true was already printed as P.

Before / after

Study exercise A1_01 (dcm4che SlideRecoder, line 131), the issue's screenshot:

before (main):                          after:
∀#fresh²⁹⁷   baseline²⁹³ || extended²⁹⁴  ∀#fresh²⁹⁷   baseline²⁹³ || extended²⁹⁴
true

Other cases (checked while developing; the unit tests below cover the rule):

Case Before (main) After
no path condition true true (unchanged)
if (baseline || extended) ∀#fresh baseline || extended, true ∀#fresh baseline || extended
nested if (lossless) if (!extended) ∀#fresh #fresh == lossless && #fresh, ∀#fresh !extended, true same rows, no true
known state in a branch ∀#fresh …, ∀wParam startCompression(wParam) unchanged

Study exercise A1_56 (JabRef EditAction, the issue comment) already had no true rows on main after #459. It is unchanged:

∀#fresh¹¹²        !#fresh¹¹¹
∀#fresh¹²⁶        !(#fresh¹²⁴ || #fresh¹²⁵)
∀undoManager¹³¹   undoManager¹³¹ == getUndoManager¹²⁹

Follow-up (not in this PR)

The guard rows above (∀#fresh¹¹² !#fresh¹¹¹, ∀#fresh¹²⁶ !(…)) say nothing about Expected, and they use fresh variables whose binders were dropped. In A1_01, the one row left (baseline || extended) also says nothing about wParam. Whether to drop path conditions unrelated to Expected is pending the PM's decision. The #fresh names are #373.

Tests

  • VCBinderSimplificationTest: 3 new tests: a last true binder after a path condition, after nested conditions, and a collapsed false binder after a row. trueBinderWithoutSuffixBecomesPlainTrue (lone true) still holds.
  • Without the fix, all three new unit tests end in true.
  • mvn test (verifier): 684 tests, 0 failures. No existing expectation changed.

🤖 Generated with Claude Code

When the binder simplification drops the last binder of a Found chain, it
left a bare `true` row after the path conditions, which reads as if the
value above were true. A suffix that becomes just `true` is now left out;
a Found that simplifies away entirely still shows `true`.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@CatarinaGamboa CatarinaGamboa added the study Needed before the error-message user study label Nov 4, 2026
@CatarinaGamboa

Copy link
Copy Markdown
Collaborator Author

Codex review, round 1

(Codex v0.164.0, codex exec review, on commit 7a656f2. Its shell sandbox cannot start on this machine (bwrap: loopback: Failed RTM_NEWADDR), so it was given the PR description, the diff against origin/main and the full changed and related files on stdin.)

The change removes the trailing true row when binder simplification produces it, while preserving a lone true result. I found no concrete correctness bug or blocking test gap in the supplied diff.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@CatarinaGamboa

Copy link
Copy Markdown
Collaborator Author

Codex review, round 2

(On commit 6c948cb, same setup as round 1. Between the rounds I changed one thing myself: the code comment P => true -> P read as an implication, where P => true is just true. It now says that Found reads its rows as a conjunction (P && true -> P). Full suite: 689 tests, 0 failures.)

The change removes trailing plain-true rows while preserving a lone true result, and the supplied tests cover the reported cases. I found no concrete correctness bug in the provided diff.

Neither round found anything, so there is nothing to fix or decline, and the review loop ends here.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@CatarinaGamboa
CatarinaGamboa merged commit c974821 into main Nov 4, 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

study Needed before the error-message user study

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Simplify trues when not used

1 participant