# Differential Review Report — Fold symbolic/assertion changes

Date: 2026-06-04
Scope: unstaged Fold diff covering symbolic path modeling, P-Code path pruning, assertion layer/DBUILD checks, reporting evidence, router/build options, and tests.

## Verdict

Major review completed with 3 agents plus local verification. Several concrete issues were fixed during review. Four larger design findings remain as follow-up tasks because they require broader modeling changes.

## Fixed during review

1. **Fold assertions were masked as recoverable TU crashes**
   - Risk: `FOLD_ASSERT`/`FOLD_PRECONDITION` inside source analysis raised `SIGABRT`, and `runClangShardTranslationUnit` could quarantine the TU instead of failing the scan.
   - Fix: `include/fold/core/assert.hpp` now tracks assertion-failure-in-flight; `src/cli/clang_shard_tu.cpp` resets/checks this flag and aborts on Fold assertion failures instead of treating them as ordinary TU signals.

2. **`FOLD_DCHECK` test did not match macro semantics**
   - Risk: test assumed only `FOLD_ENABLE_DCHECK` controls `FOLD_DCHECK`, but macro also enables under `!NDEBUG`.
   - Fix: `tests/unit/test_signal_isolation.cpp` now requires `FOLD_ENABLE_DCHECK` for the test build and always expects `SIGABRT`.

3. **Debug-only abort conflicted with guarded-fact invalid-subject skip policy**
   - Risk: `GuardedFactStore::add` aborted in test/canary builds for invalid `AccessPath` while release silently skipped.
   - Fix: `src/symbolic/guarded_facts.cpp` now consistently treats invalid subjects as skipped facts, then asserts only scalar/boolean payload invariants for valid facts.

4. **P-Code path invariant was debug-only before release dereference**
   - Risk: `path_` was checked with `FOLD_DCHECK` before release dereference.
   - Fix: `src/symbolic/pcode_encoder.cpp` now uses always-on `FOLD_ASSERT` for initialized/boolean P-Code path before feasibility/observer paths.

5. **Disabled Rust/Swift frontend options broke compilation**
   - Risk: `-Drust-frontend=false` / `-Dswift-frontend=false` omitted backend sources but `scan_runner.cpp` still instantiated them.
   - Fix: `src/cli/scan_runner.cpp` now gates backend construction with `FOLD_HAVE_RUST` / `FOLD_HAVE_SWIFT` and throws clear disabled-frontend errors.

6. **DCHECK canary docs reused another build dir's native file**
   - Risk: `BUILD.md` example pointed `build/dcheck` at `build/release/conan_meson_native.ini`.
   - Fix: docs now use independent `build/dcheck` Conan/Meson files and show `meson configure ... -Denable-dcheck=true` for existing builds.

## Remaining findings / follow-up tasks

1. **High — P-Code CBRANCH assumes all later ops are fallthrough**
   - Ref: `src/symbolic/pcode_encoder.cpp:437-444`.
   - Scenario: if lifted P-Code is ordered taken-block-first, global `path && !cond` can suppress real hazards in the taken path. Unconditional branch/return boundaries are also not modeled.
   - Follow-up: task #12.

2. **Medium — unknown-call havoc does not clear AccessPath-backed facts**
   - Ref: `src/symbolic/encoder.cpp:1038-1069`.
   - Scenario: mutable calls clear member-value cache but not AccessPath ranges/non-null/guarded facts, so facts like `obj.v.size() > 0` may survive `mutate(&obj)`.
   - Follow-up: task #13.

3. **Low/Medium — latent/precondition evidence is serialized but not produced by detectors**
   - Refs: `src/report/finding_json.cpp:104-107`, `src/report/sarif.cpp:283-290`.
   - Scenario: real raw-pointer/config-derived precondition findings still appear as ordinary findings until detector emission is wired.
   - Follow-up: task #14.

4. **Low — hybrid profile merge drops timing/counter fields**
   - Ref: `src/cli/scan_router.cpp:454-476`.
   - Scenario: hybrid profiles merge detector rows and counts but omit lane timing/prefilter counters, reducing benchmark/profile fidelity.
   - Follow-up: task #15.

## Validation run after fixes

Passed:

```sh
ninja -C build/release \
  tests/unit/test_signal_isolation tests/unit/test_guarded_facts \
  tests/unit/test_pcode_encoder tests/integration/test_scan_e2e \
  tests/integration/test_subprocess_fanout

meson test -C build/release \
  fold:signal_isolation fold:guarded_facts fold:pcode_encoder \
  fold:scan_e2e fold:subprocess_fanout --print-errorlogs

git diff --check
```

Result: `5/5 OK`, 0 failures.

Also passed no-frontends build smoke:

```sh
meson setup build/review-no-frontends ... \
  -Dbuild-tests=false -Drust-frontend=false -Dswift-frontend=false \
  -Dghidra=disabled -Dcapstone=disabled -Dradare2=disabled \
  -Dbuild-mcp-server=false -Dbuild-lsp-server=false
ninja -C build/review-no-frontends libfold-core.a
```

## Coverage limits

This was a differential review over the current unstaged diff, not a full architectural audit. The largest remaining risks are path-model semantics and fact invalidation, which need dedicated implementation/testing passes.