425eeb0751
Measured (scratch, reverted): with the graph-fed cross-TU candidate expansion disabled, finding fingerprints stay exact (fold 0fec78e5..., yams 247523c5..., status ok) and securityGraphCandidateFunctions returns an empty set (expanded=0) for these manifests. The 18.8s graph build is pure overhead for the expansion path here. Remaining graph consumer is summary priming; S0b tests whether its SCC-ordered eager prepass can be dropped in favor of the lazy on-demand summary path.
229 lines
10 KiB
Markdown
229 lines
10 KiB
Markdown
# Optimization Loop — Decision Record
|
||
|
||
> Date: 2026-07-30
|
||
> Status: accepted (workflow active)
|
||
> Grounding: `project-eval-scaffolding-2026-07-27.md` (manifests, gates),
|
||
> `PROMPT-eng-codex.md` (retrieval contract, execution loop, handoff),
|
||
> `PROMPT-yams-cpp-workflow.md` (design-first TDD, measurement-first profiling).
|
||
|
||
## Decision
|
||
|
||
Performance and bug-finding optimization on fold becomes a **phased, evidence-first
|
||
loop** driven by the existing benchmark infrastructure, not ad-hoc intuition
|
||
driven tuning:
|
||
|
||
```text
|
||
measure -> select -> implement -> validate -> checkpoint -> measure ...
|
||
```
|
||
|
||
Each phase is a persisted workflow state with a declared outcome, so an
|
||
optimization campaign survives session restarts and rework (validate failure
|
||
returns to implement, never to measure) without losing evidence.
|
||
|
||
## Phase contracts
|
||
|
||
### 1. measure
|
||
|
||
- Build release first: `ninja -C build/release`.
|
||
- Run the profiled harness over the self-hosting manifests:
|
||
|
||
```sh
|
||
python3 scripts/benchmarks/run_optimization_harness.py \
|
||
--manifest tests/benchmarks/projects/fold_source.json \
|
||
--out-dir tests/benchmarks/runs/optimization-<date>
|
||
python3 scripts/benchmarks/run_optimization_harness.py \
|
||
--manifest tests/benchmarks/projects/yams_source.json \
|
||
--out-dir tests/benchmarks/runs/optimization-<date>
|
||
```
|
||
|
||
- Record: per-detector wall time, RSS, finding count, stability
|
||
(max_churn=0, pairwise Jaccard=1.0), token/report bytes.
|
||
- Outcome `evidence-ready` when at least one run artifact exists with the
|
||
profile rows intact; `evidence-failed` if the harness cannot run (fix the
|
||
environment before optimizing anything).
|
||
|
||
### 2. select
|
||
|
||
- Pick exactly **one** target per iteration from measured evidence:
|
||
- a hotspot (top per-detector wall or RSS row), or
|
||
- a bug-finding quality gap (FP churn, missed class, unstable fingerprint).
|
||
- State the KPI and the expected direction *before* touching code
|
||
(PROMPT-yams-cpp-workflow rule 5: never optimize from intuition).
|
||
- Outcome `target-picked` with KPI written into the task/workflow state;
|
||
outcome `no-target` (terminal) when no measured target clears the
|
||
improvement bar this round.
|
||
|
||
### 3. implement
|
||
|
||
- Design-first TDD per `PROMPT-yams-cpp-workflow.md` rules 2–4: smallest
|
||
testable seam, Catch2 behavior test red before the production change,
|
||
assertions per the fold policy in `AGENTS.md`/`fold-assertion-policy`.
|
||
- One change per iteration. Hybrid/router/backend boundary rules from
|
||
`AGENTS.md` apply unchanged.
|
||
- Outcome `change-made`.
|
||
|
||
### 4. validate
|
||
|
||
Gates, in order, fastest first:
|
||
|
||
1. `git diff --check`
|
||
2. Focused tests for the touched area (`meson test -C build/release fold:<area>`).
|
||
3. Full suite for CLI/router/backend changes (`meson test -C build/release --print-errorlogs`).
|
||
4. Statistical A/B against baseline:
|
||
|
||
```sh
|
||
python3 scripts/benchmarks/run_ab_evaluation.py \
|
||
--baseline <prev-release-or-pre-change-fold> --current build/release/src/cli/fold
|
||
```
|
||
|
||
Comparison uses `fold_eval.metrics.paired_comparison` when paired samples
|
||
exist (single medians are explicitly labelled median-only, never treated
|
||
as a regression oracle).
|
||
5. Stability gate: finding fingerprints unchanged unless the change is a
|
||
deliberate detector-behavior change (then dispositions must be updated and
|
||
reviewed in the same iteration).
|
||
|
||
Hard failure rules: wall/RSS regression beyond the manifest's
|
||
`wall_regression_pct`/`rss_regression_pct`, churn > 0, or Jaccard < 1.0
|
||
without reviewed dispositions → outcome `gate-failed` → back to `implement`
|
||
(revert or fix; never weaken a gate to pass).
|
||
|
||
### 5. checkpoint
|
||
|
||
- Index changed files and a handoff note into YAMS with the
|
||
`PROMPT-eng-codex.md` metadata contract
|
||
(`task=optimization-loop, phase=complete, owner=opencode, source=note`).
|
||
- Commit with measured before/after numbers in the body; `git push` only with
|
||
operator confirmation.
|
||
- Update the baseline artifacts under `tests/benchmarks/projects/baselines/`
|
||
when a sustained win is accepted.
|
||
- Outcome `checkpointed` → loop to `measure`.
|
||
|
||
## Artifacts per iteration
|
||
|
||
| Artifact | Location |
|
||
|---|---|
|
||
| Profiled run evidence | `tests/benchmarks/runs/optimization-<date>/*.run.json` |
|
||
| A/B comparison | harness stdout + run dir summary |
|
||
| Decision note | YAMS (`source=note`) + commit message |
|
||
| Baseline update | `tests/benchmarks/projects/baselines/` (only on accepted wins) |
|
||
|
||
## Iteration ledger (2026-07-31 campaign)
|
||
|
||
| Iter | Target | Result |
|
||
|---|---|---|
|
||
| 1 | sql-injection hotspot profiling | Characterization test fixed (UAF); context-reuse attempt contaminated |
|
||
| 2 | Red evaluation gate | Dispositions re-baselined; both manifests pass |
|
||
| 3 | Call-graph fallback | Merge duplicate tolerance fix: 24.1s → 1.7s (93%) |
|
||
| 4 | Broken schema test | Full suite 137/137 |
|
||
| 5 | Summary flake | 3 rig attempts, not reproduced; rig committed |
|
||
| 6 | Z3 instrumentation | Solver-setup hypothesis falsified (154 solvers, 6ms) |
|
||
| 7 | Scratch context re-attempt | Accepted; all gates green |
|
||
| 8 | Summary-path sizing | 66,147 constructions/scan eliminated |
|
||
| 9 | Feasibility memo | Measured dead end (3.5% duplicates) |
|
||
| 10 | Dual-walk | Measured dead end (0% fallback rate) |
|
||
|
||
Cumulative: fold self-scan wall 251.7s → 86.2s (66% reduction),
|
||
finding fingerprints exact at every step.
|
||
|
||
## Deferred: in-pass graph contribution collection (design, 2026-07-31)
|
||
|
||
The no-index exhaustive call-graph build (~16.9s on the fold manifest)
|
||
is a full ClangTool re-parse of all TUs (buildProjectCallGraph). The
|
||
TU ASTs already exist in the main detector pass, and
|
||
graphToContribution(database, ctx, ...) (internal in
|
||
source_call_graph_contribution.cpp) builds a complete contribution
|
||
from a live ASTContext. Planned change (next campaign slice):
|
||
|
||
1. Expose `buildContributionFromContext(...)` (public wrapper around
|
||
graphToContribution) in `source_call_graph.hpp`.
|
||
2. In the main TU pass (source_tu_runner Consumer::HandleTranslationUnit),
|
||
build each TU's contribution post-detectors and accumulate it in
|
||
SourceTuRunState; the Consumer needs a `CompilationDatabase&` for
|
||
fingerprinting (makeTranslationUnitFingerprint uses compile commands).
|
||
3. Extend `assembleProjectCallGraph` with an optional prebuilt
|
||
contributions parameter: when they cover the shard's files AND no
|
||
project index is open, go straight to
|
||
mergeTranslationUnitGraphContributions (skipping load+collect+persistence);
|
||
all other paths unchanged.
|
||
4. Fanout topology: contributions are per-shard; each shard's assembly
|
||
covers its own files — consistent with today's per-shard exhaustive
|
||
fallback.
|
||
|
||
Equivalence argument: graphToContribution is the same function the
|
||
ClangTool collector calls with the same compilation database and
|
||
configuration profile; the main-pass ASTs come from the same compile
|
||
commands, so contributions should be byte-identical. The finding
|
||
fingerprint gate (exact) decides acceptance; the exhaustive fallback
|
||
is preserved as the worst case.
|
||
|
||
Estimated: ~80-150 lines across source_call_graph.{hpp,cpp},
|
||
source_tu_runner.{hpp,cpp}, project_graph_orchestrator.{hpp,cpp},
|
||
scan_runner.cpp.
|
||
|
||
**INVALIDATED (iteration 12, 2026-07-31)**: implementation began and
|
||
collapsed on an ordering dependency — `assembleProjectCallGraph` runs
|
||
BEFORE the main TU pass because the TU pass consumes the graph
|
||
(`graph_security_candidates` expands security candidates across TUs;
|
||
symbolic summaries are primed from the call graph). In-pass collection
|
||
cannot serve the same scan: the TU pass needs the graph the
|
||
contributions would feed. Reversing the order breaks candidate
|
||
expansion (fingerprint changes), and keeping every TU AST alive across
|
||
both passes is infeasible (~1.4 GiB RSS for the fold self-scan).
|
||
The only sound reductions for the no-index build are persistence
|
||
(cache-dir, already exists) or a lighter graph parse — neither
|
||
acceptable. The ~16.9s no-index build is the inherent cold-run price
|
||
of cross-TU facts; closed as a target.
|
||
|
||
## Non-goals
|
||
|
||
- No gate weakening, no detector-disabling to reduce runtime, no silent
|
||
coverage caps (router rules in `AGENTS.md`).
|
||
- Comparators (Clang SA, sanitizers) stay differential evidence, never ground
|
||
truth.
|
||
- Binary-lane heuristics stay heuristic; performance work on the binary path
|
||
never converts into source-coverage claims.
|
||
|
||
## Option 2 scoping (2026-08-31, README-documented)
|
||
|
||
Decision: replace the per-scan double parse (graph pass + detector pass,
|
||
`call_graph_ms` 18.8s on the fold self-scan) with single-pass graph building.
|
||
Chosen over PCH/module caching to preserve portability (README 'Performance
|
||
architecture'): no clang-version-locked artifacts, no SDK-locked module cache,
|
||
no flag-matching bug surface.
|
||
|
||
Dependency map (who consumes the graph inside the detector pass):
|
||
|
||
- `primeCrossTUProperties` (scan_runner.cpp) — third parse, only for
|
||
div-by-zero/null-deref/int-overflow/oob; no-op on the fold manifest.
|
||
- `assembleProjectCallGraph` -> `call_graph` -> `graph_security_candidates`
|
||
feeding the security walk's candidate expansion (prunes 2711 of 6480
|
||
functions — a real perf win that must be preserved).
|
||
- summary priming consumes the graph for SCC-ordered callee-first passes.
|
||
|
||
Tension to resolve before implementation: the candidate-expansion closure
|
||
needs the full cross-TU graph, which exists only after every TU is parsed, so
|
||
"run detectors in the same single pass" holds only for graph-INDEPENDENT
|
||
detectors. Graph-dependent detectors require either (A) keeping the ASTs alive
|
||
across the deferred closure (memory: ~210MB/AST, must stay under the existing
|
||
1.68GB peak profile), or (B) re-materializing the walk input from persisted
|
||
IR — the exact equivalence proof is the work, policed by the
|
||
finding-fingerprint + gold gates.
|
||
|
||
## Option 2 S0 result (2026-08-31)
|
||
|
||
Measured (scratch, reverted): disabling the graph-fed cross-TU candidate
|
||
expansion on fold + yams produced IDENTICAL finding fingerprints
|
||
(fold 0fec78e5..., yams 247523c5..., status ok, gold gates green) with
|
||
`expanded=0` in every run — `securityGraphCandidateFunctions` returns an
|
||
empty set for these manifests' detector mixes, so the graph's candidate-
|
||
expansion consumer is finding-neutral/empty here. The 18.8s graph build is
|
||
therefore pure overhead for the expansion path on these corpora.
|
||
|
||
Remaining graph consumer not yet tested: summary priming (SCC-ordered eager
|
||
prepass). S0b: disable the eager `primeSymbolicSummaries` graph-SCC layering
|
||
(rely on the lazy on-demand `ensureSymbolicSummary` path) and compare
|
||
fingerprints — if exact, the graph is fully dispensable on fold+yams and
|
||
option 2 collapses to "skip the graph build unless a corpus's detectors
|
||
actually consume it" (lazy, equilibrium fallback to the full graph when a
|
||
corpus needs expansion or SCC-ordered priming).
|