- skills/fold-sast: rename to skill-fold-sast and expand the scan/precision guidance for C++/Rust/Swift lanes - docs/architecture: record call-graph phase decomposition and the optimization-loop TP/FP/TN/FN evidence policy - docs/mcp-tools + README: update budget/per-query defaults (1800s/6000ms) and note the OpenGrep pre-push hook
339 lines
19 KiB
Markdown
339 lines
19 KiB
Markdown
# Code quality and Yams benchmark
|
|
|
|
This document maps Fold's source-side code-quality surface and defines how to
|
|
measure it on the sibling Yams repository. Security, binary, and DAST findings
|
|
remain separate lanes; this workflow is about maintainability, structural
|
|
redundancy, and reliability of the source analyzer.
|
|
|
|
## Capability map
|
|
|
|
| Goal | Detectors | Evidence | Cost / limitation |
|
|
| --- | --- | --- | --- |
|
|
| Find copy/paste implementations | `clone` | Syntactic Type-2/Type-3 hashing and similarity | Cheap after summaries. `--min-body-stmts` counts source-level executable statements; project headers can still dominate results. Tune `--min-body-stmts`, `--clone-type`, and `--clone-min-score`. |
|
|
| Find equivalent implementations | `equiv` | Bounded Z3 proof | Pairwise within compatible signatures; expensive and intentionally filters trivial constant-return pairs. |
|
|
| Find overlapping classes | `class-dup` | Heuristic class match backed by bounded method equivalence | Requires at least two eligible methods and matching method-signature fingerprints; use as design-review evidence, not an automatic deletion command. |
|
|
| Find large control-flow surfaces | `complexity` | Syntactic McCabe score from Clang CFG | Cheap and deterministic. The threshold is inclusive and configurable with `--complexity-threshold`. |
|
|
| Find unreachable or unused code | `dead-statement`, `unused-function`, `dead`, `unreachable`, `always-condition` | CFG or bounded proof, depending on detector | `unused-function` is intentionally limited to internal-linkage functions. Symbolic dead-code passes cost materially more than CFG checks. |
|
|
| Find redundant state changes | `redundant-store` | Conservative scalar data-flow, reported as heuristic | Unsupported control flow and non-scalar lifetime objects are suppressed. It is deliberately not labelled proved without an all-path proof. |
|
|
| Find C++ lifetime/API hygiene issues | `use-after-move`, `iterator-invalidation`, `static-ptr-asymmetry`, `deprecated-api` | SAT-confirmed, heuristic, or syntactic by detector | `use-after-move` checks source order and path feasibility; imprecisely modelled control flow is suppressed. Review confidence and evidence per result. |
|
|
|
|
`--scope quality` selects complexity, CFG/API, and C++ lifetime hygiene.
|
|
`--scope redundancy` adds clones, semantic equivalence, duplicate classes,
|
|
dead paths, constant conditions, and redundant stores. Use both scopes for a
|
|
complete structural review.
|
|
|
|
## Recommended Yams rollout
|
|
|
|
Start with two lanes so high-volume redundancy results do not block useful
|
|
correctness checks.
|
|
|
|
1. PR advisory lane:
|
|
|
|
```sh
|
|
fold scan --mode source \
|
|
--compile-db ../yams/build/release/compile_commands.json \
|
|
--detectors complexity,dead-statement,unused-function,use-after-move,iterator-invalidation \
|
|
--complexity-threshold 20 --processes 4 \
|
|
--format sarif --output yams-quality.sarif
|
|
```
|
|
|
|
2. Nightly design-review lane:
|
|
|
|
```sh
|
|
fold scan --mode source \
|
|
--compile-db ../yams/build/release/compile_commands.json \
|
|
--scope quality --scope redundancy \
|
|
--min-body-stmts 6 --clone-min-score 0.85 \
|
|
--complexity-threshold 20 --processes 4 \
|
|
--format sarif --output yams-structural.sarif
|
|
```
|
|
|
|
The two-TU metadata slice now has a reviewed baseline and gates only new stable
|
|
finding IDs. Keep broader Yams paths advisory until their findings receive the
|
|
same source review and per-finding rationale. The CI clang-tidy job is likewise
|
|
advisory while it uploads a per-commit baseline candidate; promote it only after
|
|
a reviewer has classified the captured diagnostics and accepted the baseline.
|
|
|
|
## Benchmark contract
|
|
|
|
The real-project runner is `scripts/benchmarks/run_code_quality.py`. It:
|
|
|
|
- selects a stable, sorted subset from an existing compile database;
|
|
- runs repeated serial scans with Fold's internal profile enabled;
|
|
- runs the same workload through subprocess fanout;
|
|
- fingerprints canonical finding content across every run and lane;
|
|
- checks physical locations, backend provenance, crashes, wall-clock timeout,
|
|
and a configurable Z3-timeout budget;
|
|
- reports per-detector median timings, peak RSS, and the shared
|
|
frontend/orchestration remainder;
|
|
- separates function fingerprinting, standalone call-graph construction,
|
|
cross-TU property collection, and symbolic-summary priming from the still
|
|
unattributed frontend/orchestration remainder;
|
|
- decomposes call-graph work into cache load, dirty-TU contribution collection,
|
|
contribution merge/SCC work, exhaustive fallback, index writes, and security
|
|
candidate expansion;
|
|
- writes hash-addressed `RunArtifact` records with exact commands, inputs,
|
|
outputs, environment/tool facts, measurements, and exit status;
|
|
- optionally compares wall time and finding identity with a prior baseline,
|
|
using paired repetition samples, effect, and a sign test when the baseline
|
|
carries matching raw samples;
|
|
- optionally compares stable finding IDs with a reviewed disposition baseline,
|
|
reporting new, persisting, and resolved findings, accepted/actionable trends,
|
|
changed-file scope, review coverage, and alert density per KLOC.
|
|
|
|
Run the current Yams metadata slice with:
|
|
|
|
```sh
|
|
scripts/benchmarks/run_code_quality.py \
|
|
--compile-db ../yams/build/release/compile_commands.json \
|
|
--project-root ../yams \
|
|
--path-regex '/src/metadata/' --max-tus 12 \
|
|
--repeats 3 --processes 4 \
|
|
--dispositions tests/benchmarks/yams/metadata_dispositions.json \
|
|
--out-dir /tmp/fold-yams-quality
|
|
```
|
|
|
|
For an A/B optimization, preserve the first `summary.json`, repeat the same TU
|
|
selection, and pass it back as `--baseline`. Finding drift fails by default;
|
|
`--allow-finding-drift` is reserved for intentional detector changes.
|
|
|
|
### Optimization harness
|
|
|
|
Use the wrapper when the goal is to collect one concise optimization artifact
|
|
rather than inspect each project-evaluation run separately:
|
|
|
|
```sh
|
|
scripts/benchmarks/run_optimization_harness.py \
|
|
--manifest tests/benchmarks/projects/yams_source.json \
|
|
--out-dir /tmp/fold-optimization
|
|
```
|
|
|
|
It preserves the project evaluator's profile/stability gates, turns launch and
|
|
timeout failures into recorded results, and reports a model-independent SARIF
|
|
size estimate (`ceil(UTF-8 bytes / 4)`) plus findings per estimated 1K tokens.
|
|
This is a comparison metric, not a claim of exact tokens for a particular model.
|
|
Use `--max-report-tokens` only after reviewing a workload-specific baseline.
|
|
|
|
The broader selection, ablation, and evidence policy is
|
|
[`docs/architecture/optimization-loop.md`](optimization-loop.md). In
|
|
particular, an unlabelled Yams or Chromium alert count is not a TP/FP/TN/FN
|
|
measurement; retain the project artifact and add an explicit oracle before
|
|
claiming precision or recall.
|
|
|
|
### Local quality artifact contract
|
|
|
|
`fold quality` keeps `summary.json` and terminal output compact. Each executed
|
|
step writes its complete stdout/stderr under `logs/`; the differential scan
|
|
keeps its complete SARIF report, and project-policy runs retain their summary,
|
|
markdown, and immutable run artifacts. The compact summary contains status,
|
|
bounded tail evidence, artifact paths, and only new/resolved finding details;
|
|
persisting reviewed findings remain in the full policy artifact. Finding detail
|
|
lists are capped for rendering and report the omitted count.
|
|
|
|
The summary records deterministic `ceil(UTF-8 bytes / 4)` estimates for both its
|
|
compact pre-metric payload and complete text artifacts. These measurements are
|
|
observational: no token threshold is enforced until a workload-specific
|
|
baseline is reviewed.
|
|
|
|
The synthetic precision guard is
|
|
`tests/benchmarks/source/code_quality_structural.json`. It pins positive and
|
|
clean cases for clones, semantic equivalence, duplicate classes, complexity,
|
|
dead statements, and unused functions. The standard quality gate now runs it.
|
|
|
|
Adjacent executable surfaces protect the analyzer that supplies the Yams
|
|
results. `run_source_metamorphic.py` requires invariant findings across source
|
|
renaming, harmless insertion, equivalent guard forms, TU order, and worker
|
|
count. `run_sanitizer_differential.py` compares vulnerable/fixed division and
|
|
signed-overflow programs against UBSan and OOB/UAF programs against ASan, with
|
|
deterministic replay. `run_clang_sa_differential.py` retains Fold/Clang SA
|
|
agreement on division and null-dereference pairs. `run_organic_pairs.py` keeps
|
|
three provenance- and hash-pinned upstream CVE adaptations separate from the
|
|
synthetic score. These do not turn comparator or runtime evidence into source
|
|
proof; they test classifications against independent controlled oracles.
|
|
|
|
The optimization contract is deliberately multi-dimensional:
|
|
|
|
| Property | Gate / metric | Reject when |
|
|
| --- | --- | --- |
|
|
| Precision | Vulnerable and clean-near-miss goldens; one-to-one matching; source metamorphic, UBSan/ASan/Clang SA differential cases, and origin-separated organic pairs | A clean case gains a finding, a required positive disappears, a semantics-preserving transform drifts, or a controlled comparator pair disagrees |
|
|
| Determinism | Canonical finding fingerprint, intersection/union, churn, pairwise Jaccard | Serial repetitions or worker counts change finding identity |
|
|
| Reliability | Exit status, crashes, subprocess timeout, Z3 timeout count, physical location, backend provenance | A scan crashes, hangs, exceeds its timeout budget, or emits malformed evidence |
|
|
| Speed | Median/IQR wall time, detector timers, source-phase timers | An A/B run exceeds the configured baseline regression budget |
|
|
| Resource use | Peak driver RSS | Memory exceeds the configured baseline regression budget |
|
|
| Structure | Per-production-file line budget | A new C++ monolith exceeds 1,000 lines or a tracked exception grows |
|
|
|
|
Every precision defect discovered during Yams review should enter the nearest
|
|
unit or golden corpus before its fix. Every performance change must preserve
|
|
the finding fingerprint unless the semantic change is explicit and reviewed.
|
|
|
|
## Initial Yams baseline (2026-07-13)
|
|
|
|
A four-TU metadata slice (`connection_pool.cpp`, `path_utils.cpp`,
|
|
`query_helpers.cpp`, and `tree_differ.cpp`) produced the following local
|
|
release-build baseline:
|
|
|
|
| Metric | Result |
|
|
| --- | ---: |
|
|
| Visible functions, including project headers | 949 |
|
|
| Stable post-filter findings | 171 |
|
|
| Profiled serial median (2 runs) | 14.011 s |
|
|
| Two-worker throughput median (2 runs) | 8.954 s |
|
|
| Peak driver RSS | 644.188 MiB |
|
|
| Z3 timeouts | 0 |
|
|
| Finding stability | Identical across all 4 runs |
|
|
|
|
Finding distribution was 72 clones, 81 redundant stores, 7 dead statements,
|
|
6 use-after-move results, 4 complexity hotspots, and 1 unused function. These
|
|
counts are a workload fingerprint, not a claim that all 171 results are true
|
|
design defects.
|
|
|
|
## Calibrated Yams result
|
|
|
|
Reviewing every non-clone result exposed systematic precision defects rather
|
|
than 95 independent Yams defects. After fixing the detectors and pinning each
|
|
failure mode with a focused regression, the exact same four-TU workload
|
|
produced:
|
|
|
|
| Metric | Before | Calibrated |
|
|
| --- | ---: | ---: |
|
|
| Stable post-filter findings | 171 | 32 |
|
|
| Clone | 72 | 28 |
|
|
| Redundant store | 81 | 0 |
|
|
| Dead statement | 7 | 0 |
|
|
| Use after move | 6 | 0 |
|
|
| High complexity | 4 | 4 |
|
|
| Unused function | 1 | 0 |
|
|
| Profiled serial median | 14.011 s | 12.391 s |
|
|
| Two-worker median | 8.954 s | 7.487 s |
|
|
| Peak driver RSS | 644.188 MiB | 649.555 MiB |
|
|
| Z3 timeouts | 0 | 0 |
|
|
|
|
The calibrated fingerprint was identical across two serial and two parallel
|
|
runs. Serial time improved 11.6%, two-worker time improved 16.4%, and peak RSS
|
|
increased 0.8%.
|
|
|
|
The 28 remaining clone reports are structurally valid candidates, not automatic
|
|
defects: 23 are configuration-family boilerplate in `TuneAdvisor.h`, two are
|
|
document-metadata enum mappers, one is repair-tuning boilerplate, one groups
|
|
connection-pool trace-environment helpers, and one pairs two closely related
|
|
query builders. The four complexity metrics are also arithmetically correct.
|
|
One (`errorToString`, complexity 37) is primarily a switch mapping table, so a
|
|
reviewer may reasonably baseline it while retaining the metric.
|
|
|
|
The precision fixes covered same-expression and reordered use-after-move
|
|
events, unsupported-control-flow redundant stores, RAII objects mistaken for
|
|
scalar stores, dependent-template calls mistaken for unused helpers,
|
|
configuration-pruned branches mistaken for actionable dead statements, and AST
|
|
node counts mistaken for source-level statement counts. Serial and fanout runs
|
|
now share the same clone/equivalence thresholds and cross-TU wire data.
|
|
|
|
Building the executable evaluation foundation found two further source-proof
|
|
defects. Shared evidence annotation was promoting local findings to
|
|
summary-backed/cross-TU corroborated and overwriting detector proof metadata;
|
|
detectors now own those flags at the actual callsite. Summary encoding also
|
|
used the wrong fallthrough condition after an early-return `if` without an
|
|
`else`, producing a guarded-division false positive. Both failures are retained
|
|
as unit, cross-TU, metamorphic, and sanitizer-differential regressions.
|
|
|
|
## Optimization surface
|
|
|
|
The calibrated run identifies the next measurable targets:
|
|
|
|
1. **Shared frontend/orchestration remainder — 6.623 s in the calibrated
|
|
baseline.** This was roughly half of profiled analysis time. Profiles now
|
|
expose `function_fingerprint_ms`, `call_graph_ms`, `cross_tu_property_ms`,
|
|
and `symbolic_summary_ms`, so the next run can distinguish reusable-fact
|
|
passes from the main traversal and genuinely unattributed setup. Remove
|
|
duplicate frontend work before micro-optimizing cheap detectors.
|
|
2. **Expensive passes with few solver queries.** `always-condition` takes
|
|
1.322 s for two queries, `equiv` 1.064 s for none, `dead` 0.994 s for one,
|
|
`unreachable` 0.974 s for none, and `use-after-move` 0.751 s for none on this
|
|
corpus. Candidate construction, serialization, and CFG/state setup are a
|
|
larger surface than Z3 itself. Shared facts are the likely optimization
|
|
boundary; detector semantics must remain independently tested.
|
|
3. **Header amplification — 4 TUs expose 949 functions.** Project headers are
|
|
legitimate analysis inputs but inflate clone candidates and memory. Add a
|
|
report-scope benchmark (main-file-only versus project-header-inclusive)
|
|
before changing coverage defaults.
|
|
4. **Memory — 650 MiB peak RSS.** Track this alongside time; an optimization
|
|
that retains every AST/summary to save recomputation may regress practical
|
|
Yams throughput.
|
|
5. **Reviewed change-time precision.** The two-TU Yams baseline now classifies
|
|
every result and blocks new unreviewed IDs. Extend the reviewed slice only
|
|
with the same per-finding rationale and stable configuration/TU contract.
|
|
6. **Organic recall gaps.** The pinned libssh2 slice is detected and all fixed
|
|
slices are clean; nullable-return propagation for the GStreamer `strchr`
|
|
case and OpenSSL public-key flow remains explicit backlog. DART-style
|
|
predicate flipping belongs only behind targeted confirmation tasks after a
|
|
concrete campaign plateaus.
|
|
|
|
The serial/parallel reliability benchmark already found and now guards one
|
|
class of drift: fanout workers previously ignored complexity and clone tuning
|
|
options, and same-file equivalence was incorrectly promoted as cross-TU
|
|
corroboration. `fold:subprocess_fanout` pins parity across those lanes.
|
|
|
|
### Current phase-attributed Yams slice
|
|
|
|
After adding source-phase timers, a two-TU metadata slice exposed 916 visible
|
|
functions through project headers. Across two profiled serial runs and two
|
|
two-worker runs, all four result sets contained the same 32 findings (pairwise
|
|
Jaccard 1.0) with zero Z3 timeouts.
|
|
|
|
| Surface | Median | Analyze share |
|
|
| --- | ---: | ---: |
|
|
| Function fingerprint pass | 1.188 s | 20.38% |
|
|
| Largest detector (`equiv`) | 0.620 s | 10.64% |
|
|
| All named detector timers | 2.468 s | 42.35% |
|
|
| Still-unattributed frontend/orchestration | 2.172 s | 37.26% |
|
|
| Total analyzed time | 5.827 s | 100% |
|
|
|
|
This makes the duplicate fingerprint parse the first shared-fact optimization
|
|
target. The extraction into `incremental_fingerprint.cpp` gives that work a
|
|
narrow contract; the deterministic transitive-hash test and the stable Yams
|
|
fingerprint are the reliability guard for any later pass fusion or cache reuse.
|
|
|
|
The post-decomposition rerun kept the same TU-set fingerprint and the same 32
|
|
findings across two serial and two two-worker runs: Jaccard 1.0, zero churn,
|
|
zero Z3 timeouts. Median serial wall time was 5.890 s, two-worker wall time was
|
|
3.091 s, and peak driver RSS was 504.797 MiB. Against the immediately preceding
|
|
baseline, those are reductions of about 8.8%, 9.5%, and 12.9%, respectively;
|
|
they are local measurements, not universal speedup claims.
|
|
|
|
The reviewed evaluation-foundation rerun selected
|
|
`connection_pool.cpp`/`content_index_writer.cpp`. Suppressing explicit
|
|
`while (true)` loop controls as non-actionable reduced the result to 31
|
|
findings: 27 accepted-risk and 4 actionable. Across two serial and two
|
|
two-worker runs the IDs were identical (Jaccard 1.0, zero churn, zero Z3
|
|
timeouts), with zero new and zero resolved relative to the reviewed baseline.
|
|
Local medians were 6.887 s serial and 3.429 s at two workers, with 523.109 MiB
|
|
median peak driver RSS. Alert density was 4.493 per KLOC over the 6.900 KLOC
|
|
union of selected TUs and finding-bearing project headers; new-alert density
|
|
was zero. These are workload measurements,
|
|
not universal performance claims.
|
|
|
|
The reviewed file is
|
|
`tests/benchmarks/yams/metadata_dispositions.json`. Twenty-seven accepted-risk
|
|
records cover typed tuning-accessor families, enum/string mappers, and the
|
|
exhaustive error-code mapping. Four actionable records cover the three repeated
|
|
trace-toggle helpers and the multi-responsibility query/acquire/configuration
|
|
functions. Each record has a rationale; the baseline pins Yams revision, TU-set
|
|
fingerprint, and analysis configuration fingerprint.
|
|
|
|
## Production structure guard
|
|
|
|
The source-size ratchet now has no exceptions. The former coordination and
|
|
analysis monoliths have narrow modules with every production C++ file below
|
|
1,000 lines:
|
|
|
|
| Former concentration | Facade now | Extracted ownership |
|
|
| --- | ---: | --- |
|
|
| Source scan runner | 850 lines | backend dispatch, fingerprints, call graph, summaries, TU execution, evidence, quarantine |
|
|
| Security walker | 355 lines | analysis, contracts, refinement, engine core/control, batch observer |
|
|
| Div-by-zero detector | 633 lines | guard facts and context |
|
|
| Null / overflow / OOB detectors | 962 / 934 / 926 lines | candidate and proof-analysis helpers |
|
|
| Symbolic encoder | 215 lines | foundations, operators, expression dispatch and arms |
|
|
| radare2 backend | 827 lines | session transport, serialization/classification, disassembly state, extraction |
|
|
|
|
`fold:source_structure` rejects a new production `.cpp` above the limit. This
|
|
is a maintainability ratchet, not a quality score: module cohesion, behavior
|
|
tests, and the benchmark contract remain the semantic guards.
|