Skip to content

feat(bug-zoo): BZ-DETERMINISM-001 green on real lifters + real solver#1563

Merged
TSavo merged 4 commits into
mainfrom
feat/determinism-species
May 27, 2026
Merged

feat(bug-zoo): BZ-DETERMINISM-001 green on real lifters + real solver#1563
TSavo merged 4 commits into
mainfrom
feat/determinism-species

Conversation

@TSavo
Copy link
Copy Markdown
Owner

@TSavo TSavo commented May 27, 2026

The determinism reference species, green end-to-end through the bug-zoo runner, no mock: lab unit test passes; the exhibit's lifted composition obligation post(serialize) ⟹ canonical(out) is unsatisfied (z3 counterexample — the missing edge); the fixed state satisfied. Contracts are lifted from idiomatic Go by the real lifter; the obligation is discharged by real z3.

Includes the enabling lifter/mint work (stacked on #1561 guard→precondition and #1562 composite-literals):

  • lift-go: LiftPaths recurses a directory source-path (mint --project hands the lifter the project dir).
  • cmd_mint: contract-name extraction also reads fnName (go's camelCase field). CID-neutral for existing kits.
  • smoke: specimen species count 4→5.

Honestly disclosed (see README): the canonical predicate is modeled arithmetically (canonical == non-negative) so the obligation is SMT-refutable rather than undecidable. Same bug class, real lifters, real solver. The literal byte-order instance needs predicate-inlining + slice/byte SMT-modeling (tracked follow-up).

Depends on #1561 + #1562.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • Go lifter now automatically derives function preconditions from defensive guard patterns.
    • Improved handling of unkeyed slice and array composite literals in Go.
    • Deterministic file expansion when lifting multiple directory paths.
    • Added canonical byte-order determinism test case demonstrating proof lifting seams.
  • Bug Fixes

    • Rust CLI mint command now gracefully handles additional function name field formats.
  • Tests

    • Updated specimen count expectations in smoke tests.

Review Change Stack

TSavo and others added 4 commits May 27, 2026 05:35
The Go production lifter emitted `pre = true` for every function — a
consumer's defensive guard (`if !P(args){panic}`) never became a contract
precondition, so it was invisible at call sites. This brings Go to parity
with the Rust (lift_function_precondition) and C# (LiftMethodPrecondition)
walkers: a leading `if !P{panic}` lifts to `pre = P == true`, a bare
`if cond{panic}` to `cond == false`.

Scans only leading guards, stops at the first non-guard statement, and
refuses to invent a precondition from an unliftable condition. CID-neutral
for existing kits (the Go self-contracts contractSetCid is unchanged —
073e4010…; no go-kit function uses the guard pattern). Lifter tests pass.

Unblocks the BZ-DETERMINISM-001 reference species: a consumer's
canonical-byte-order precondition now lifts from idiomatic Go.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
`[]T{e0, e1, …}` lifts to a `go:slice-literal` ctor over positionally-lifted
elements; struct/map/keyed literals stay unmodeled (refusing rather than
inventing a lossy shape). CID-neutral for existing kits (Go contractSetCid
unchanged, 073e4010…). Lifter tests pass.

Lets a serializer that emits bytes in a given (non-canonical) order lift to a
real postcondition — the producer half of the BZ-DETERMINISM-001 seam, whose
consumer requires canonical byte order.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Real Go producer/consumer/seam on the two shipped lifter features (#1561
guard→pre, #1562 composite-literals): ContentAddress.pre lifts to
canonical_byte_order, Serialize.post to slice-literal, the lab round-trip
test passes while the lifted composition exposes the unmet precondition.

NOT yet runner-green: the bug-zoo `composition: Missing` check requires the
obligation to come back `unsatisfied` (a refutable SMT counterexample), but
the lifted obligation uses `go:slice-literal` + an uninterpreted
`go:call(CanonicalByteOrder,…)` that do not compile to valid SMT-LIB, so the
solver returns `undecidable`. Closing that needs SMT-modeling of slice/byte
ops + predicate inlining in the Go lifter — the next lifter feature.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The determinism reference species, end-to-end through the runner, no mock:
lab unit test passes; the exhibit's lifted composition obligation
post(serialize) ⟹ canonical(out) is `unsatisfied` (z3 counterexample) — the
missing edge; the fixed state `satisfied`. Contracts lifted from idiomatic Go
by the real lifter; obligation discharged by real z3.

Enabling lifter/mint changes (the "invest in the lifter features" work):
- lift-go: LiftPaths recurses a directory source-path (mint --project hands the
  lifter the project dir, not files) — emits non-test *.go, sorted.
- cmd_mint: contract-name extraction also reads `fnName` (the go production
  lifter's camelCase field), not only name/symbol/fn_name. CID-neutral for
  existing kits (only affects go-production-lifted contracts, which nothing
  else minted).
- smoke: specimen species count 4 -> 5.

Honestly disclosed (README): the canonical predicate is modeled arithmetically
(canonical == non-negative) so the obligation is SMT-refutable rather than
`undecidable`. Same bug class on real lifters + real solver. The literal
byte-order instance needs predicate-inlining + slice/byte SMT-modeling in the
lifter (tracked follow-up); the fixed state likewise canonicalizes to a constant
pending conditional-postcondition modeling.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 27, 2026

Walkthrough

This PR enhances the Go lifter with precondition derivation from defensive guard patterns and composite literal support, adds a minor field fallback to the Rust CLI, and introduces a comprehensive canonical byte-order determinism bug species demonstrating a composition-precondition seam in three phases: lab (latent bug), exhibit (bug revealed by lifter), and fixed (corrected).

Changes

Go Lifter Enhancements

Layer / File(s) Summary
Precondition from guards and composite literal support
implementations/go/provekit-lift-go/lift.go
Preconditions now derive from leading panic guards via liftLeadingGuardPreconditions, replacing the default true; composite literals support unkeyed slice/array forms lifted into go:slice-literal IR.
Directory expansion and workspace-relative paths
implementations/go/provekit-lift-go/lift.go
New goSourceFiles helper expands directories into sorted non-test .go files; LiftPathsWithOptions resolves workspace-relative paths and lifts each file deterministically.
Rust CLI name field fallback
implementations/rust/provekit-cli/src/cmd_mint.rs
Function name resolution gains a new fallback to the fnName JSON field before defaulting to unnamed.

BZ-DETERMINISM-001 Canonical Byte-Order Species

Layer / File(s) Summary
Lab phase: passing unit test with latent bug
menagerie/bug-zoo/species/BZ-DETERMINISM-001-canonical-byte-order/go/lab/harness/*
Lab implements Serialize as identity pass-through and ContentAddress with negative-encoding panic guard; unit test passes because non-canonical encodings are not exercised.
Exhibit phase: lifted proof reveals missing establishment
menagerie/bug-zoo/species/BZ-DETERMINISM-001-canonical-byte-order/go/exhibit/map-serializer/*
Exhibit lifts lab code to ProofIR, revealing that Serialize does not establish the canonical precondition required by ContentAddress; includes expected fixtures and diagnostics.
Fixed phase: corrected implementation discharges obligation
menagerie/bug-zoo/species/BZ-DETERMINISM-001-canonical-byte-order/go/fixed/map-serializer/*
Fixed version corrects Serialize to return canonical encoding 0, establishing the precondition; lifted ProofIR and diagnostics confirm the determinism obligation discharges.
Species specification and documentation
menagerie/bug-zoo/species/BZ-DETERMINISM-001-canonical-byte-order/README.md, menagerie/bug-zoo/species/BZ-DETERMINISM-001-canonical-byte-order/specimen.yaml, menagerie/bug-zoo/tests/smoke.rs
README documents the canonical byte-order seam and arithmetic predicate modeling for SMT refutability; specimen YAML defines predicates, harnesses, and composition checks for both exhibit and fixed phases; smoke test updated to reflect the new species count.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

🐰 A lifter now sniffs panic guards with care,
Derives preconditions floating in the air,
Slices bloom from literals, deterministic and true,
Lab breaks, exhibit shows, fixed makes it right—
A species grows from seam to light! ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main PR objective: adding a working BZ-DETERMINISM-001 bug-zoo species that passes using real lifters and the Z3 solver.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/determinism-species

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 golangci-lint (2.12.2)

level=error msg="[linters_context] typechecking error: pattern ./...: directory prefix . does not contain main module or its selected dependencies"


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 5e2cc4a145

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +1311 to +1312
if e.IsDir() || !strings.HasSuffix(name, ".go") || strings.HasSuffix(name, "_test.go") {
continue
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Walk subdirectories when expanding project roots

When the Go lifter is invoked through the project workflow, kit_dispatch sends source_paths: ["."]; for a normal Go module with packages under subdirectories, this branch skips every directory entry and only returns top-level .go files. That means provekit mint --project <module-root> silently omits nested package contracts, or returns an empty IR if the root only contains go.mod. Please recurse into subdirectories here, while still excluding tests and ignored directories such as vendor, .git, and .provekit.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

🧹 Nitpick comments (1)
menagerie/bug-zoo/tests/smoke.rs (1)

116-120: ⚡ Quick win

Assert the new species ID, not just the total count.

Line 118 updates the count to 5, but this can still pass if BZ-DETERMINISM-001 is absent and another species fills the slot.

✅ Suggested test hardening
     assert_eq!(
         reports.len(),
         5,
         "bug zoo reports the current shape species"
     );
+    assert!(
+        reports
+            .iter()
+            .any(|entry| entry["id"] == "BZ-DETERMINISM-001"),
+        "new determinism species should be present in --all JSON reports"
+    );
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@menagerie/bug-zoo/tests/smoke.rs` around lines 116 - 120, The test currently
only asserts reports.len() == 5 which can pass even if the specific species is
missing; instead assert that the new species ID "BZ-DETERMINISM-001" appears in
the returned reports. Locate the assertion around reports.len() in tests::smoke
(variable reports) and add or replace it with an assertion that checks reports
contains an entry whose identifier field (e.g., report.id or report.species_id)
equals "BZ-DETERMINISM-001" (use the actual field name on the report struct) to
guarantee the specific species is present.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@implementations/go/provekit-lift-go/lift.go`:
- Around line 1296-1317: goSourceFiles currently only reads one directory level
causing LiftPathsWithOptions to miss nested Go files; replace the os.ReadDir
logic with a recursive directory walk (e.g., filepath.WalkDir or os.WalkDir)
inside goSourceFiles so it traverses subdirectories, collects files ending with
".go" but not "_test.go", appends full paths to the files slice, and then sorts
and returns them; ensure the new walk skips directories you want excluded (e.g.,
vendor/.git if desired) and preserves the existing error handling and return
shape used by LiftPathsWithOptions.
- Around line 321-324: The code currently falls back to pre=true when
liftLeadingGuardPreconditions encounters an unsupported leading-guard shape;
instead change liftLeadingGuardPreconditions so it returns an explicit error (or
refusal) for unsupported forms and propagate that error back to the caller
(liftFunc) rather than letting formulaValue produce a vacuous true. Update the
call site where you do pre, err :=
formulaValue(l.liftLeadingGuardPreconditions(fn.Body.List)) to check for and
return a refuse("unsupported-leading-guard", fn.Pos(), ...) (or pass through the
error) when liftLeadingGuardPreconditions signals unsupported syntax; apply the
same change to the other occurrence referenced (the block around lines 366-379)
so all unsupported leading-guard scans refuse the lift instead of emitting
pre=true. Ensure error/refusal uses fn.Pos() and a clear refusal tag so callers
can diagnose the unsupported guard.
- Around line 401-415: isPanicOnlyBlock currently only checks ident.Name ==
"panic" which misclassifies shadowed identifiers; modify isPanicOnlyBlock to
accept a *types.Info (or the resolved callee object) and use info.Uses[ident] to
ensure the identifier resolves to the builtin panic. Specifically, change the
signature of isPanicOnlyBlock to accept (b *ast.BlockStmt, info *types.Info) (or
accept the resolved ast.Ident object), ensure you still validate the AST shape
(single ExprStmt → CallExpr → Ident), then look up obj := info.Uses[ident];
return true only if obj is a *types.Builtin and obj.Name() == "panic", handling
nil lookups safely. Ensure callers of isPanicOnlyBlock are updated to pass the
types.Info.

In `@menagerie/bug-zoo/species/BZ-DETERMINISM-001-canonical-byte-order/README.md`:
- Around line 26-27: The bullet under the "fixed" state omits the verb; update
the sentence that mentions `Serialize` so it reads: "**fixed** — `Serialize`
returns a canonical encoding; the obligation is satisfied." (i.e., insert "is"
before "satisfied") in the README.md entry that references `Serialize`.

---

Nitpick comments:
In `@menagerie/bug-zoo/tests/smoke.rs`:
- Around line 116-120: The test currently only asserts reports.len() == 5 which
can pass even if the specific species is missing; instead assert that the new
species ID "BZ-DETERMINISM-001" appears in the returned reports. Locate the
assertion around reports.len() in tests::smoke (variable reports) and add or
replace it with an assertion that checks reports contains an entry whose
identifier field (e.g., report.id or report.species_id) equals
"BZ-DETERMINISM-001" (use the actual field name on the report struct) to
guarantee the specific species is present.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 6bf5fc20-1d32-4d5e-bf2f-a97fe183c7c6

📥 Commits

Reviewing files that changed from the base of the PR and between e610dac and 5e2cc4a.

📒 Files selected for processing (24)
  • implementations/go/provekit-lift-go/lift.go
  • implementations/rust/provekit-cli/src/cmd_mint.rs
  • menagerie/bug-zoo/species/BZ-DETERMINISM-001-canonical-byte-order/README.md
  • menagerie/bug-zoo/species/BZ-DETERMINISM-001-canonical-byte-order/go/exhibit/map-serializer/expected-diagnostic.txt
  • menagerie/bug-zoo/species/BZ-DETERMINISM-001-canonical-byte-order/go/exhibit/map-serializer/expected.proofir.json
  • menagerie/bug-zoo/species/BZ-DETERMINISM-001-canonical-byte-order/go/exhibit/map-serializer/harness/.provekit/config.toml
  • menagerie/bug-zoo/species/BZ-DETERMINISM-001-canonical-byte-order/go/exhibit/map-serializer/harness/.provekit/lift/go-canonical/manifest.toml
  • menagerie/bug-zoo/species/BZ-DETERMINISM-001-canonical-byte-order/go/exhibit/map-serializer/harness/codec.go
  • menagerie/bug-zoo/species/BZ-DETERMINISM-001-canonical-byte-order/go/exhibit/map-serializer/harness/go.mod
  • menagerie/bug-zoo/species/BZ-DETERMINISM-001-canonical-byte-order/go/exhibit/map-serializer/kit-rpc/run-go-lifter.sh
  • menagerie/bug-zoo/species/BZ-DETERMINISM-001-canonical-byte-order/go/exhibit/sat-witness.json
  • menagerie/bug-zoo/species/BZ-DETERMINISM-001-canonical-byte-order/go/fixed/map-serializer/expected-diagnostic.txt
  • menagerie/bug-zoo/species/BZ-DETERMINISM-001-canonical-byte-order/go/fixed/map-serializer/expected.proofir.json
  • menagerie/bug-zoo/species/BZ-DETERMINISM-001-canonical-byte-order/go/fixed/map-serializer/harness/.provekit/config.toml
  • menagerie/bug-zoo/species/BZ-DETERMINISM-001-canonical-byte-order/go/fixed/map-serializer/harness/.provekit/lift/go-canonical/manifest.toml
  • menagerie/bug-zoo/species/BZ-DETERMINISM-001-canonical-byte-order/go/fixed/map-serializer/harness/codec.go
  • menagerie/bug-zoo/species/BZ-DETERMINISM-001-canonical-byte-order/go/fixed/map-serializer/harness/go.mod
  • menagerie/bug-zoo/species/BZ-DETERMINISM-001-canonical-byte-order/go/fixed/map-serializer/kit-rpc/run-go-lifter.sh
  • menagerie/bug-zoo/species/BZ-DETERMINISM-001-canonical-byte-order/go/lab/harness/codec.go
  • menagerie/bug-zoo/species/BZ-DETERMINISM-001-canonical-byte-order/go/lab/harness/codec_test.go
  • menagerie/bug-zoo/species/BZ-DETERMINISM-001-canonical-byte-order/go/lab/harness/go.mod
  • menagerie/bug-zoo/species/BZ-DETERMINISM-001-canonical-byte-order/go/lab/harness/run.sh
  • menagerie/bug-zoo/species/BZ-DETERMINISM-001-canonical-byte-order/specimen.yaml
  • menagerie/bug-zoo/tests/smoke.rs

Comment on lines +321 to 324
pre, err := formulaValue(l.liftLeadingGuardPreconditions(fn.Body.List))
if err != nil {
return FunctionContract{}, nil, refuse("internal-error", fn.Pos(), err.Error())
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

Don't silently weaken unsupported leading guards to pre=true.

If the function starts with an if that is clearly in the leading-guard prefix but isn't this exact supported shape, the scan just breaks and liftFunc emits pre = true. That drops a real fail-fast contract on the floor and can make caller-side obligations pass when they should fail. Please refuse the lift for unsupported leading guard forms instead of falling back to an empty conjunction.

Also applies to: 366-379

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@implementations/go/provekit-lift-go/lift.go` around lines 321 - 324, The code
currently falls back to pre=true when liftLeadingGuardPreconditions encounters
an unsupported leading-guard shape; instead change liftLeadingGuardPreconditions
so it returns an explicit error (or refusal) for unsupported forms and propagate
that error back to the caller (liftFunc) rather than letting formulaValue
produce a vacuous true. Update the call site where you do pre, err :=
formulaValue(l.liftLeadingGuardPreconditions(fn.Body.List)) to check for and
return a refuse("unsupported-leading-guard", fn.Pos(), ...) (or pass through the
error) when liftLeadingGuardPreconditions signals unsupported syntax; apply the
same change to the other occurrence referenced (the block around lines 366-379)
so all unsupported leading-guard scans refuse the lift instead of emitting
pre=true. Ensure error/refusal uses fn.Pos() and a clear refusal tag so callers
can diagnose the unsupported guard.

Comment on lines +401 to +415
// isPanicOnlyBlock reports whether a block is exactly `{ panic(...) }`.
func isPanicOnlyBlock(b *ast.BlockStmt) bool {
if b == nil || len(b.List) != 1 {
return false
}
exprStmt, ok := b.List[0].(*ast.ExprStmt)
if !ok {
return false
}
call, ok := exprStmt.X.(*ast.CallExpr)
if !ok {
return false
}
ident, ok := call.Fun.(*ast.Ident)
return ok && ident.Name == "panic"
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🌐 Web query:

Can Go's predeclared identifier panicbe shadowed by a local declaration, and if so, is checkingident.Name == "panic" sufficient to detect the builtin panic call?

💡 Result:

Yes, the predeclared identifier panic can be shadowed by a local declaration in Go. 1) Can panic be shadowed? Go’s specification says the scope of a predeclared identifier is the universe block, but that an identifier declared inside a function begins its scope within the innermost containing block. This means a local declaration of a constant/type/variable/function named panic will take precedence in its scope, effectively shadowing the predeclared panic name in that region [1]. 2) Is checking ident.Name == "panic" sufficient to detect calls to the builtin panic? No. Checking only ident.Name == "panic" is insufficient because ident.Name will be “panic” even when the identifier has been shadowed (i.e., it denotes a local variable/function named panic rather than the builtin). The builtin/non-builtin distinction requires name resolution during type checking. Authoritative way to detect the builtin call: during type checking, Go maps each identifier (ast.Ident) to the symbol (Object) it denotes. Built-in functions (like the predeclared panic) are represented as *types.Builtin objects [2][3]. Therefore, an AST-only string check can produce false positives; you should instead verify that the resolved object for the called identifier is a builtin whose Name is panic (i.e., object is a *types.Builtin with .Name() == "panic") [4][3]. Summary - Shadowing: Yes, local declarations can shadow the predeclared panic name within their scope [1]. - Detection: No, ident.Name == "panic" alone can’t reliably distinguish builtin panic(...) calls from calls to a shadowing local panic [2][3]. Use Go’s type info/name resolution to confirm the identifier resolves to the builtin panic object [2][4].

Citations:


🏁 Script executed:

#!/bin/bash
set -euo pipefail

FILE="implementations/go/provekit-lift-go/lift.go"
# Show the relevant region around lines 360-460 (includes 401-415)
nl -ba "$FILE" | sed -n '350,460p'
# Also quickly locate calls/usages of isPanicOnlyBlock
rg -n "isPanicOnlyBlock\\(" "$FILE"

Repository: TSavo/provekit

Length of output: 101


🏁 Script executed:

#!/bin/bash
set -euo pipefail
FILE="implementations/go/provekit-lift-go/lift.go"

# Print lines 380-440 with line numbers (no nl available)
python3 - <<'PY'
import itertools
path="implementations/go/provekit-lift-go/lift.go"
start, end = 380, 440
with open(path, "r", encoding="utf-8") as f:
    for i, line in enumerate(f, start=1):
        if start <= i <= end:
            print(f"{i}\t{line.rstrip()}")
        if i > end:
            break
PY

# Find where isPanicOnlyBlock is defined and used
rg -n "func isPanicOnlyBlock\\b" "$FILE"
rg -n "isPanicOnlyBlock\\b" "$FILE"

Repository: TSavo/provekit

Length of output: 2489


🏁 Script executed:

#!/bin/bash
set -euo pipefail
FILE="implementations/go/provekit-lift-go/lift.go"

python3 - <<'PY'
path="implementations/go/provekit-lift-go/lift.go"
start, end = 330, 390
with open(path, "r", encoding="utf-8") as f:
    for i, line in enumerate(f, start=1):
        if start <= i <= end:
            print(f"{i}\t{line.rstrip()}")
        if i > end:
            break
PY

# Locate lifter definition and its fields
rg -n "type lifter\\b" "$FILE"
rg -n "struct\\s*\\{" "$FILE" | head -n 20

# See where types.Info is used/passed around
rg -n "\\*types\\.Info" "$FILE"

Repository: TSavo/provekit

Length of output: 3607


🏁 Script executed:

#!/bin/bash
set -euo pipefail
FILE="implementations/go/provekit-lift-go/lift.go"

# Show lifter struct section (around where rg indicated info *types.Info at line ~27)
python3 - <<'PY'
path="implementations/go/provekit-lift-go/lift.go"
start, end = 1, 80
with open(path, "r", encoding="utf-8") as f:
    for i, line in enumerate(f, start=1):
        if start <= i <= end:
            print(f"{i}\t{line.rstrip()}")
        if i > end:
            break
PY

# Show objectForCallee helper
python3 - <<'PY'
path="implementations/go/provekit-lift-go/lift.go"
start, end = 960, 1045
with open(path, "r", encoding="utf-8") as f:
    for i, line in enumerate(f, start=1):
        if start <= i <= end:
            print(f"{i}\t{line.rstrip()}")
        if i > end:
            break
PY

Repository: TSavo/provekit

Length of output: 6195


Only treat calls that resolve to the builtin panic as fail-fast guards

  • The current isPanicOnlyBlock check (ident.Name == "panic") can misclassify when panic is shadowed by a local/package identifier, causing incorrect precondition lifting.
  • Thread *types.Info into isPanicOnlyBlock (or pass the resolved callee object) and require that the called identifier resolves to the builtin panic (e.g., info.Uses[ident] is *types.Builtin with Name() == "panic").
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@implementations/go/provekit-lift-go/lift.go` around lines 401 - 415,
isPanicOnlyBlock currently only checks ident.Name == "panic" which misclassifies
shadowed identifiers; modify isPanicOnlyBlock to accept a *types.Info (or the
resolved callee object) and use info.Uses[ident] to ensure the identifier
resolves to the builtin panic. Specifically, change the signature of
isPanicOnlyBlock to accept (b *ast.BlockStmt, info *types.Info) (or accept the
resolved ast.Ident object), ensure you still validate the AST shape (single
ExprStmt → CallExpr → Ident), then look up obj := info.Uses[ident]; return true
only if obj is a *types.Builtin and obj.Name() == "panic", handling nil lookups
safely. Ensure callers of isPanicOnlyBlock are updated to pass the types.Info.

Comment on lines +1296 to +1317
func goSourceFiles(path string) ([]string, error) {
info, err := os.Stat(path)
if err != nil {
return nil, err
}
if !info.IsDir() {
return []string{path}, nil
}
entries, err := os.ReadDir(path)
if err != nil {
return nil, err
}
var files []string
for _, e := range entries {
name := e.Name()
if e.IsDir() || !strings.HasSuffix(name, ".go") || strings.HasSuffix(name, "_test.go") {
continue
}
files = append(files, filepath.Join(path, name))
}
sort.Strings(files)
return files, nil
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

goSourceFiles still only scans one directory level.

This collects immediate children with os.ReadDir and skips every nested directory, so giving LiftPathsWithOptions a project root will still miss Go sources below the top level. That means project-directory lifting is incomplete for any layout with subpackages or nested harness code. A recursive walk here is needed.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@implementations/go/provekit-lift-go/lift.go` around lines 1296 - 1317,
goSourceFiles currently only reads one directory level causing
LiftPathsWithOptions to miss nested Go files; replace the os.ReadDir logic with
a recursive directory walk (e.g., filepath.WalkDir or os.WalkDir) inside
goSourceFiles so it traverses subdirectories, collects files ending with ".go"
but not "_test.go", appends full paths to the files slice, and then sorts and
returns them; ensure the new walk skips directories you want excluded (e.g.,
vendor/.git if desired) and preserves the existing error handling and return
shape used by LiftPathsWithOptions.

Comment on lines +26 to +27
- **fixed** — `Serialize` returns a canonical encoding; the obligation
`satisfied`.
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Fix fixed-state bullet grammar.

Line 27 currently omits the verb; use “is satisfied” for clarity and consistency.

✏️ Suggested edit
-- **fixed** — `Serialize` returns a canonical encoding; the obligation
-  `satisfied`.
+- **fixed** — `Serialize` returns a canonical encoding; the obligation
+  is `satisfied`.
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
- **fixed**`Serialize` returns a canonical encoding; the obligation
`satisfied`.
- **fixed**`Serialize` returns a canonical encoding; the obligation
is `satisfied`.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@menagerie/bug-zoo/species/BZ-DETERMINISM-001-canonical-byte-order/README.md`
around lines 26 - 27, The bullet under the "fixed" state omits the verb; update
the sentence that mentions `Serialize` so it reads: "**fixed** — `Serialize`
returns a canonical encoding; the obligation is satisfied." (i.e., insert "is"
before "satisfied") in the README.md entry that references `Serialize`.

@TSavo TSavo merged commit 9b7fed8 into main May 27, 2026
5 of 8 checks passed
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.

1 participant