From b410c462ae50e97fa63a2cfa2547f7dc76d8b641 Mon Sep 17 00:00:00 2001 From: Paul Reinlein Date: Thu, 26 Feb 2026 10:07:31 -0500 Subject: [PATCH 1/3] Simplify review process and remove rescue --- .claude/skills/README.md | 2 - .claude/skills/lading-optimize-hunt/SKILL.md | 14 +- .../assets/index.template.yaml | 2 +- .../skills/lading-optimize-rescue/SKILL.md | 196 --------- .../skills/lading-optimize-review/SKILL.md | 50 +-- .../assets/bug-found.template.yaml | 8 - .../assets/duplicate.template.yaml | 8 - .../skills/lading-optimize-submit/SKILL.md | 7 +- .../skills/lading-optimize-validate/SKILL.md | 372 ------------------ 9 files changed, 14 insertions(+), 645 deletions(-) delete mode 100644 .claude/skills/lading-optimize-rescue/SKILL.md delete mode 100644 .claude/skills/lading-optimize-review/assets/bug-found.template.yaml delete mode 100644 .claude/skills/lading-optimize-review/assets/duplicate.template.yaml delete mode 100644 .claude/skills/lading-optimize-validate/SKILL.md diff --git a/.claude/skills/README.md b/.claude/skills/README.md index 4b19b9171..47937a658 100644 --- a/.claude/skills/README.md +++ b/.claude/skills/README.md @@ -18,9 +18,7 @@ Run preflight first. It validates your environment and tells you what to do next | `/lading-optimize-find-target` | Select and analyze an optimization target | | `/lading-optimize-hunt` | Baseline, implement, review, and record optimizations | | `/lading-optimize-review` | 5-persona peer review before merge | -| `/lading-optimize-validate` | Bug validation with Kani proofs or property tests | | `/lading-optimize-submit` | Git branch, commits, and optional PR for optimizations | -| `/lading-optimize-rescue` | Salvage optimization work lacking benchmarks | ## Workflow diff --git a/.claude/skills/lading-optimize-hunt/SKILL.md b/.claude/skills/lading-optimize-hunt/SKILL.md index 695eaea98..74be853b0 100644 --- a/.claude/skills/lading-optimize-hunt/SKILL.md +++ b/.claude/skills/lading-optimize-hunt/SKILL.md @@ -1,12 +1,12 @@ --- name: lading-optimize-hunt -description: Systematic optimization hunter for lading. Finds memory optimizations AND bugs - both are valuable. Run /lading-optimize-validate when bugs are discovered. -allowed-tools: Bash(cat:*) Bash(sample:*) Bash(samply:*) Bash(cargo:*) Bash(ci/*:*) Bash(hyperfine:*) Bash(*/payloadtool:*) Bash(tee:*) Read Write Edit Glob Grep Skill Task +description: Coordinates optimization attempts. Captures baselines, implements changes, invokes review, and records outcomes. +allowed-tools: Bash(cat:*) Bash(cargo:*) Bash(ci/*:*) Bash(hyperfine:*) Bash(*/payloadtool:*) Bash(tee:*) Read Write Edit Glob Grep Skill --- # Optimization Hunt -Systematically explores the lading codebase, implements optimizations, validates with benchmarks. **Finding bugs is equally valuable as finding optimizations.** +Coordinates optimization attempts: captures baselines, implements changes, invokes review, and records all outcomes. ## Role: Coordinator and Recorder @@ -98,13 +98,7 @@ ci/validate **No exceptions. If ci/validate fails, fix the issue before continuing.** -If `ci/validate` repeatedly fails on what appears to be a **pre-existing bug** (not caused by your change), invoke validation: - -``` -/lading-optimize-validate -``` - -After validation completes, return here and select the next target. +If `ci/validate` repeatedly fails on a **pre-existing bug** (not caused by your change), document it and stop. --- diff --git a/.claude/skills/lading-optimize-hunt/assets/index.template.yaml b/.claude/skills/lading-optimize-hunt/assets/index.template.yaml index e628ebb90..4bb82308c 100644 --- a/.claude/skills/lading-optimize-hunt/assets/index.template.yaml +++ b/.claude/skills/lading-optimize-hunt/assets/index.template.yaml @@ -3,5 +3,5 @@ entries: target: technique: status: - verdict: + verdict: file: assets/db/.yaml diff --git a/.claude/skills/lading-optimize-rescue/SKILL.md b/.claude/skills/lading-optimize-rescue/SKILL.md deleted file mode 100644 index 8a19f7bf5..000000000 --- a/.claude/skills/lading-optimize-rescue/SKILL.md +++ /dev/null @@ -1,196 +0,0 @@ ---- -name: lading-optimize-rescue -description: Salvages optimization work lacking benchmarks. Generates missing evidence, validates claims. Bugs discovered during rescue are valuable - invoke /lading-optimize-validate. -allowed-tools: Bash(cargo:*) Bash(hyperfine:*) Bash(*/payloadtool:*) Bash(ci/*:*) Bash(diff:*) Bash(tee:*) Read Write Edit Glob Grep Skill ---- - -# Optimization Rescue - -Salvage optimization work done without proper benchmarks. Generate evidence, validate claims, and discover bugs hiding in "optimizations." - -## Valuable Outcomes - -| Outcome | Value | Action | -|---------|-------|--------| -| **Change validated** | Real improvement proven | KEEP in working directory | -| **Change invalidated** | No improvement | DISCARD, record lesson | -| **Bug discovered** | Correctness issue found | Invoke `/lading-optimize-validate` | - -**Bugs found during rescue are SUCCESS, not failure.** - ---- - -## Phase 0: Pre-flight - -Run `/lading-preflight` first. - ---- - -## Phase 1: Audit - -Examine the changes in your working directory. Identify all modified files and categorize each change: -- Preallocation (`Vec::with_capacity`, `String::with_capacity`) -- Avoiding clones (borrowing instead of owned) -- Moving allocations out of loops -- Data structure changes -- **Potential bug** (suspicious patterns) - ---- - -## Phase 2: Triage - -| Hot Path? | Decision | -|-----------|----------| -| Yes (profiled, top 10%) | INVESTIGATE | -| Warm (suspected hot, 10-25%) | SKEPTICAL | -| Cold (no profile evidence) | LIKELY DISCARD | -| **Looks buggy** | INVESTIGATE for correctness | - -### Bug Warning Signs in Rust - -| Pattern | Risk | -|---------|------| -| `.unwrap()` or `.expect()` added | Panic path (lading MUST NOT panic) | -| `unsafe` block added | Memory safety risk | -| Changed return type | Semantic change | -| Removed bounds checks | Correctness risk | -| Clone removed without lifetime analysis | Use-after-move risk | -| `mem::transmute` or `mem::forget` | Undefined behavior risk | - ---- - -## Phase 3: Generate Evidence - -### Establish Baseline for Comparison - -**Baseline must be from unmodified code.** You'll need to capture baseline metrics before your changes, then measure again with your changes applied. - -### For payloadtool (end-to-end): - -```bash -# Choose a config file (e.g., ci/fingerprints/json/lading.yaml) -CONFIG=ci/fingerprints/json/lading.yaml - -# Baseline (without changes) -cargo build --release --bin payloadtool -hyperfine --warmup 3 --runs 30 --export-json /tmp/baseline.json \ - "./target/release/payloadtool $CONFIG" -./target/release/payloadtool "$CONFIG" --memory-stats 2>&1 | tee /tmp/baseline-mem.txt - -# With changes applied -cargo build --release --bin payloadtool -hyperfine --warmup 3 --runs 30 --export-json /tmp/optimized.json \ - "./target/release/payloadtool $CONFIG" -./target/release/payloadtool "$CONFIG" --memory-stats 2>&1 | tee /tmp/optimized-mem.txt -``` - -### For inner loops (criterion): - -Use `cargo criterion` for micro-benchmarks. Run before and after changes: - -```bash -# Baseline (without changes) -cargo criterion 2>&1 | tee /tmp/criterion-baseline.log - -# With changes applied -cargo criterion 2>&1 | tee /tmp/criterion-optimized.log - -# Compare results manually - look for "change:" lines showing improvement/regression -``` - -**Note:** Criterion automatically compares against the previous run and reports percentage changes. - -### Create Benchmarks If Missing - -If no benchmark exists for the changed code, create one: - -```rust -// In lading_payload/benches/.rs -use criterion::{criterion_group, criterion_main, Criterion, Throughput}; - -fn benchmark_function(c: &mut Criterion) { - let mut group = c.benchmark_group("function_name"); - group.throughput(Throughput::Bytes(1024)); - - group.bench_function("baseline", |b| { - b.iter(|| { - function_under_test() - }) - }); - - group.finish(); -} - -criterion_group!(benches, benchmark_function); -criterion_main!(benches); -``` - ---- - -## Phase 4: Validate - -### Decision Matrix - -| Result | Decision | -|--------|----------| -| Time improved >=5% | KEEP | -| Memory reduced >=10% | KEEP | -| Allocations reduced >=20% | KEEP | -| No significant change | DISCARD | -| Regression | DISCARD | -| **ci/validate fails** | Possible BUG -> `/lading-optimize-validate` | -| **Determinism broken** | Possible BUG -> `/lading-optimize-validate` | -| **Panic path added** | BUG -> `/lading-optimize-validate` | - -### Verify Determinism - -Determinism is verified via fingerprints. The same config (with fixed seed) must produce identical output: -```bash -CONFIG=ci/fingerprints/json/lading.yaml -./target/release/payloadtool "$CONFIG" --fingerprint > /tmp/run1.txt -./target/release/payloadtool "$CONFIG" --fingerprint > /tmp/run2.txt -diff /tmp/run1.txt /tmp/run2.txt # Must be identical -``` - -**Note:** Seed is specified in the config file, not as a CLI flag. - ---- - -## Phase 5: Handle Bug Discovery - -If rescue uncovers a bug instead of an optimization: - -``` -/lading-optimize-validate -``` - -After validation: -1. Bug recorded in `.claude/skills/lading-optimize-hunt/assets/db.yaml` (via /lading-optimize-validate) -2. Record rescue as BUG_FOUND in Phase 7 -3. The bug fix remains in working directory (with tests!) - ---- - -## Phase 6: Reconstruct - -Keep only: -- **KEEP** changes (validated optimizations with benchmark proof) -- **BUG_FOUND** changes (with tests from /lading-optimize-validate) - -Discard everything else from your working directory. - -### Mandatory Before Finishing - -```bash -ci/validate -``` - -**No exceptions. Rescued changes must pass ci/validate.** - ---- - -## Phase 7: Report - -Report if the rescue was successful. - ---- diff --git a/.claude/skills/lading-optimize-review/SKILL.md b/.claude/skills/lading-optimize-review/SKILL.md index fc4e90984..16df3d253 100644 --- a/.claude/skills/lading-optimize-review/SKILL.md +++ b/.claude/skills/lading-optimize-review/SKILL.md @@ -1,8 +1,8 @@ --- name: lading-optimize-review -description: Reviews optimization patches for lading using a 5-persona peer review system. Requires unanimous approval backed by benchmarks. Bugs discovered during review are valuable - invoke /lading-optimize-validate to validate them. +description: Reviews optimization patches using a 5-persona peer review system. Requires unanimous approval backed by benchmarks. argument-hint: "[bench] [fingerprint] [file] [target] [technique]" -allowed-tools: Bash(cat:*) Bash(sample:*) Bash(samply:*) Bash(cargo:*) Bash(ci/*:*) Bash(hyperfine:*) Bash(*/payloadtool:*) Bash(tee:*) Read Write Edit Glob Grep Skill +allowed-tools: Bash(cat:*) Bash(sample:*) Bash(samply:*) Bash(cargo:*) Bash(ci/*:*) Bash(hyperfine:*) Bash(*/payloadtool:*) Bash(tee:*) Read Glob Grep context: fork --- @@ -16,15 +16,12 @@ A rigorous 5-persona peer review system for optimization patches in lading. Requ Review judges using benchmarks and 5-persona review, then returns a structured report. -## Valuable Outcomes +## Outcomes -| Outcome | Value | Action | +| Outcome | Votes | Action | |---------|-------|--------| -| **Optimization APPROVED** | Improvement validated | Merge | -| **Optimization REJECTED** | Learned where NOT to optimize | Record lesson | -| **Bug DISCOVERED** | Found correctness issue | Invoke `/lading-optimize-validate` | - -**Finding bugs during optimization review is SUCCESS, not failure.** +| **APPROVED** | 5/5 APPROVE | Return APPROVED report | +| **REJECTED** | Any REJECT | Return REJECTED report | --- @@ -126,7 +123,7 @@ hyperfine --warmup 3 --runs 30 --export-json /tmp/optimized.json \ - [ ] No semantic changes to output - [ ] **Determinism preserved** (same seed -> same output) - [ ] No `.unwrap()` or `.expect()` added (lading MUST NOT panic) -- [ ] **No bugs introduced** (if bug found -> `/lading-optimize-validate`) +- [ ] **No bugs introduced** (if bug found -> REJECT with bug details) - [ ] Property tests exist for changed code ### 4. Rust Expert (Lading-Specific Patterns) @@ -178,37 +175,8 @@ ci/kani lading_payload |---------|-------|--------| | **APPROVED** | 5/5 APPROVE | Return APPROVED report | | **REJECTED** | Any REJECT | Return REJECTED report | -| **DUPLICATE** | Duplicate Hunter REJECT | Return DUPLICATE report | -| **BUG FOUND** | Correctness issue | Invoke `/lading-optimize-validate`, return BUG_FOUND report | - -### Bug Triage - -When issues are discovered, determine the correct action: - -| Signal | Is It a Bug? | Action | -| --------------------------------------- | ------------ | ---------------------------------- | -| ci/validate fails after optimization | Possible bug | Invoke `/lading-optimize-validate` | -| Determinism broken | Possible bug | Invoke `/lading-optimize-validate` | -| Conservative finds correctness issue | Possible bug | Invoke `/lading-optimize-validate` | -| Benchmark regression but correct output | NOT a bug | REJECT optimization | -| No improvement but correct output | NOT a bug | REJECT optimization | - -### When Bug Is Found - -If review discovers a bug instead of an optimization: - -``` -/lading-optimize-validate -``` - -The validate skill will: -1. Confirm this is actually a bug (not just a failed optimization) -2. Attempt Kani proof (if feasible) -3. Create property test reproducing the bug -4. Verify the fix works -5. Record in `.claude/skills/lading-optimize-hunt/assets/db.yaml` -Then return here to return a BUG_FOUND report in Phase 6. +Duplicates, bugs, correctness issues, and missing benchmarks are all rejections. Describe the specific reason in the report's `reason` field. --- @@ -222,8 +190,6 @@ Fill in the appropriate template and return the completed YAML: | --------- | ---------------------------------------------------------------------- | | approved | `.claude/skills/lading-optimize-review/assets/approved.template.yaml` | | rejected | `.claude/skills/lading-optimize-review/assets/rejected.template.yaml` | -| duplicate | `.claude/skills/lading-optimize-review/assets/duplicate.template.yaml` | -| bug_found | `.claude/skills/lading-optimize-review/assets/bug-found.template.yaml` | 1. Read the appropriate template from the `.claude/skills/lading-optimize-review/assets/` directory 2. Fill in placeholders using argument values: diff --git a/.claude/skills/lading-optimize-review/assets/bug-found.template.yaml b/.claude/skills/lading-optimize-review/assets/bug-found.template.yaml deleted file mode 100644 index 6212082e7..000000000 --- a/.claude/skills/lading-optimize-review/assets/bug-found.template.yaml +++ /dev/null @@ -1,8 +0,0 @@ -id: -target: -technique: -date: -status: failure -verdict: bug_found -reason: | - diff --git a/.claude/skills/lading-optimize-review/assets/duplicate.template.yaml b/.claude/skills/lading-optimize-review/assets/duplicate.template.yaml deleted file mode 100644 index 4da4dd981..000000000 --- a/.claude/skills/lading-optimize-review/assets/duplicate.template.yaml +++ /dev/null @@ -1,8 +0,0 @@ -id: -target: -technique: -date: -status: failure -verdict: duplicate -reason: | - diff --git a/.claude/skills/lading-optimize-submit/SKILL.md b/.claude/skills/lading-optimize-submit/SKILL.md index 21b167bee..295b35971 100644 --- a/.claude/skills/lading-optimize-submit/SKILL.md +++ b/.claude/skills/lading-optimize-submit/SKILL.md @@ -1,16 +1,13 @@ --- name: lading-optimize-submit description: Full optimization workflow with git branch creation, commits, and optional PR. Wraps /lading-optimize-hunt with git automation. -allowed-tools: Bash(git:*) Bash(gh:*) Bash(cat:*) Read Write Edit Skill +allowed-tools: Bash(git:*) Bash(gh:*) Bash(cat:*) Read Skill --- # Optimization Submit Workflow **Complete optimization workflow with git automation.** This skill wraps `/lading-optimize-hunt` and handles: - Git branch creation -- Baseline benchmarking -- Code changes -- Re-benchmarking with changes - Git commit with formatted results - Optional push and PR creation @@ -136,8 +133,6 @@ gh pr create \ - [x] Kani proofs pass (or N/A: ) - [x] Determinism verified -Ready for `/lading-optimize-review`. - 🤖 Generated with [Claude Code](https://claude.com/claude-code) EOF )" diff --git a/.claude/skills/lading-optimize-validate/SKILL.md b/.claude/skills/lading-optimize-validate/SKILL.md deleted file mode 100644 index e770afa93..000000000 --- a/.claude/skills/lading-optimize-validate/SKILL.md +++ /dev/null @@ -1,372 +0,0 @@ ---- -name: lading-optimize-validate -description: Validates discovered bugs with reproducing tests and validates fixes with regression tests. Called by review when bugs are found during optimization, or by hunt when bugs are found during analysis. Creates property tests (proptest) and Kani proofs when feasible. -allowed-tools: Bash(ci/*:*) Bash(cargo:*) Bash(*/payloadtool:*) Bash(diff:*) Read Write Edit Glob Grep ---- - -# Correctness Validation - -When optimization hunting discovers a bug instead of an optimization opportunity, this skill validates the finding through tests. **No bug fix should be merged without a test that would have caught it.** - -## When This Skill Is Called - -### Primary Caller: Review - -`/lading-optimize-review` invokes this skill when: -- The Skeptic persona finds benchmark anomalies suggesting correctness issues -- The Conservative persona finds correctness violations (broken determinism, semantic changes) -- ci/validate fails after an optimization that appeared correct - -### Secondary Caller: Hunt - -`/lading-optimize-hunt` invokes this skill when: -- A bug is found during code analysis (Phase 2), before any optimization is attempted -- ci/validate repeatedly fails on what appears to be a pre-existing bug (Phase 4) - -``` -Review or Hunt discovers potential bug - | - v - /lading-optimize-validate - | - +-- Confirm this is actually a bug (Phase 1.0) - +-- Attempt Kani proof (if feasible) - +-- Create property test (proptest) - +-- Validate fix works - +-- Record in .claude/skills/lading-optimize-hunt/assets/db.yaml - | - v - Return to calling skill with status -``` - -### Bug vs. Failed Optimization Triage - -Not every failure is a bug. Use this table before proceeding: - -| Signal | Bug? | Action | -|--------|------|--------| -| ci/validate fails on changed code | Possible | Investigate — may be bug or bad optimization | -| ci/validate fails on unchanged code | Likely pre-existing bug | Proceed with validation | -| Determinism broken after optimization | Possible | Investigate — optimization may have introduced it | -| Determinism broken on main branch | Pre-existing bug | Proceed with validation | -| Benchmark regression but correct output | NOT a bug | Return `NOT_A_BUG` — this is a failed optimization | -| No improvement but correct output | NOT a bug | Return `NOT_A_BUG` — this is a failed optimization | -| Wrong output regardless of optimization | Bug | Proceed with validation | -| Panic on valid input | Bug | Proceed with validation | - ---- - -## Philosophy - -> "A bug without a test is just an anecdote. A bug with a test is knowledge." - -Finding bugs during optimization work is **valuable**, not a failure. But a bug fix without a reproducing test: -1. Can't prove the bug existed -2. Can't prove the fix works -3. Can regress silently later - -**Every bug fix MUST include a test that fails before the fix and passes after.** - -### Property Tests vs Unit Tests - -Per lading's CLAUDE.md: **ALWAYS prefer property tests over unit tests.** - -- Property tests (proptest) verify invariants across generated inputs -- Unit tests only check specific examples you thought of -- Property tests find edge cases you didn't anticipate - -### Kani Proofs - -**Kani provides exhaustive verification when feasible.** - -Kani constraints: -- More labor-intensive to write -- May not compile complex code -- Runs EXTREMELY slowly for complex code -- When it works, it's more complete than property tests - -**Approach: Try Kani first. Fall back to proptest if Kani fails.** - ---- - -## Phase 1: Understand the Bug - -### 1.0 Confirm This Is a Bug - -Before investing in test infrastructure, verify this is actually a correctness bug: - -| Check | Expected | If Not Met | -|-------|----------|-----------| -| Output is incorrect for valid input | Yes | If output is correct, return `NOT_A_BUG` | -| Issue reproduces without the optimization | Check main branch | If only with optimization, it's a bad optimization — return `NOT_A_BUG` | -| Issue is a correctness problem, not perf | Yes | If purely performance, return `NOT_A_BUG` | -| Issue is in lading code, not test harness | Yes | If test harness, fix test instead | - -**If this is NOT a bug**, return immediately: - -```yaml -validation_result: - status: NOT_A_BUG - reason: "" -``` - -The calling skill (review or hunt) will handle this as a rejected optimization, not a bug. - -### 1.1 Document the Bug -```yaml -bug: - file: lading_payload/src/block.rs - function: Cache::fixed - discovered_by: hunt-optimization - description: "Off-by-one in capacity calculation causes short read" - impact: "Generated payloads truncated by 1 byte" - root_cause: "Used < instead of <= in boundary check" -``` - -### 1.2 Identify Bug Category - -| Category | Example | Test Strategy | -|----------|---------|---------------| -| **Off-by-one** | Wrong bounds | Property test: output length = expected | -| **Capacity error** | Wrong preallocation | Property test: no panic, correct size | -| **Determinism** | Non-reproducible output | Property test: same seed = same output | -| **Overflow** | Integer overflow | Kani proof (if feasible) or proptest | -| **Logic error** | Wrong condition | Property test with counterexample | -| **Panic path** | Unwrap on None | Property test: no panic for any input | - -### 1.3 Find the Minimal Reproducer - -Identify the smallest input that triggers the bug: -```rust -// What input demonstrates the bug? -let config = Config { size: 1024, seed: 42 }; -let result = buggy_function(&config); -// Expected: 1024 bytes -// Actual: 1023 bytes (BUG: off by one) -``` - ---- - -## Phase 2: Attempt Kani Proof - -**Try Kani first - it provides exhaustive verification.** - -### 2.1 Check if Kani is Feasible - -```bash -# For lading_throttle bugs -ci/kani lading_throttle - -# For lading_payload bugs -ci/kani lading_payload -``` - -### 2.2 If Kani Works - -Write a proof that demonstrates the invariant: - -```rust -#[cfg(kani)] -mod proofs { - use super::*; - - #[kani::proof] - fn capacity_never_underflows() { - let size: usize = kani::any(); - kani::assume(size <= MAX_REASONABLE_SIZE); - - let result = calculate_capacity(size); - - // The invariant that was violated - kani::assert(result >= size, "Capacity must be >= requested size"); - } -} -``` - -### 2.3 If Kani Fails - -Document why and proceed to property tests: - -```yaml -kani_attempted: true -kani_result: FAILED -kani_reason: "Compilation error: unsupported feature XYZ" -# OR -kani_reason: "Timeout after 30 minutes on complex function" -``` - ---- - -## Phase 3: Create Property Test - -### 3.1 Write Property Test That Fails on Buggy Code - -```rust -#[cfg(test)] -mod tests { - use super::*; - use proptest::prelude::*; - - proptest! { - #[test] - fn output_length_equals_requested(size in 1usize..10000) { - let config = Config { size, seed: 42 }; - let result = function_under_test(&config); - - // The property that was violated - prop_assert_eq!(result.len(), size, - "Output length {} != requested size {}", result.len(), size); - } - } -} -``` - -### 3.2 Test Naming Convention - -Don't prefix with `test_` - they're obviously tests: -``` -output_length_equals_requested -determinism_preserved_across_runs -no_panic_on_zero_size -capacity_at_least_requested -``` - -### 3.3 Verify Test Fails Before Fix - -**Important:** Ensure the test fails on buggy code before applying the fix. You may need to temporarily revert your fix to verify. - -```bash -# Run the new test - should FAIL on buggy code -ci/test -``` - -**If test passes on buggy code, the test doesn't reproduce the bug. Rewrite it.** - ---- - -## Phase 4: Validate the Fix - -### 4.1 Verify Test Passes After Fix - -```bash -ci/test -``` - -### 4.2 Run Full Validation - -```bash -# MANDATORY: Full validation suite -ci/validate -``` - -### 4.3 Run Kani (If Applicable) - -```bash -# If the bug was in throttle or payload -ci/kani lading_throttle -ci/kani lading_payload -``` - -### 4.4 Verify Determinism - -Determinism is verified via fingerprints. The same config (with fixed seed) must produce identical output: -```bash -CONFIG=ci/fingerprints/json/lading.yaml -./target/release/payloadtool "$CONFIG" --fingerprint > /tmp/run1.txt -./target/release/payloadtool "$CONFIG" --fingerprint > /tmp/run2.txt -diff /tmp/run1.txt /tmp/run2.txt # Must be identical -``` - -**Note:** Seed is specified in the config file, not as a CLI flag. - ---- - -## Phase 5: Document and Record - -### MANDATORY: Update db.yaml - -1. Add entry to `.claude/skills/lading-optimize-hunt/assets/db.yaml` index -2. Create detailed file in `.claude/skills/lading-optimize-hunt/assets/db/` directory - -**`.claude/skills/lading-optimize-hunt/assets/db.yaml` entry:** -```yaml -entries: - - file: - function: - category: - status: validated - detail_file: .claude/skills/lading-optimize-hunt/assets/db/.yaml -``` - -**`.claude/skills/lading-optimize-hunt/assets/db/.yaml`:** -```yaml -file: -function: -category: -discovered_by: -date: -bug_description: | - -kani: - attempted: - result: - reason: -tests_added: - - name: - type: -verification: - test_fails_before_fix: yes - test_passes_after_fix: yes - ci_validate_passes: yes - determinism_verified: yes -lessons: | - -``` - ---- - -## Phase 6: Return to Calling Skill - -After validation complete, return status to the calling skill: - -```yaml -validation_result: - status: VALIDATED # or INVALID, NEEDS_WORK, NOT_A_BUG - bug_confirmed: true - fix_confirmed: true - kani_proof_added: true # or false - property_tests_added: 1 - ci_validate_passes: true - determinism_preserved: true - ready_for_merge: true -``` - -### Status Values - -| Status | Meaning | -|--------|---------| -| `VALIDATED` | Bug confirmed, fix verified, tests added | -| `INVALID` | Reported issue could not be reproduced | -| `NEEDS_WORK` | Bug confirmed but fix is incomplete | -| `NOT_A_BUG` | Issue is a failed optimization, not a correctness bug | - -The calling skill should: -1. Record the bug discovery as a SUCCESS (not failure) -2. Include validation status in the review -3. Proceed with merge if VALIDATED - ---- - -## Checklist - -Before returning VALIDATED: - -- [ ] Bug documented with root cause -- [ ] Kani proof attempted (document if failed) -- [ ] Property test written (proptest, not unit test) -- [ ] Test FAILS on buggy code (verified) -- [ ] Test PASSES on fixed code (verified) -- [ ] `ci/validate` passes -- [ ] Determinism verified (same seed = same output) -- [ ] No `.unwrap()` or `.expect()` in fix (use Result) -- [ ] Recorded in `.claude/skills/lading-optimize-hunt/assets/db.yaml` with detail file From c009a6460ee7617a453340a7479fe7e8a4eaa37e Mon Sep 17 00:00:00 2001 From: Paul Reinlein Date: Fri, 27 Feb 2026 11:32:06 -0500 Subject: [PATCH 2/3] otel traces: reuse scratch buffer in to_bytes instead of encode_to_vec Replace the per-call Vec allocation from encode_to_vec() with a RefCell scratch buffer that persists across calls, matching the pattern already used by OpentelemetryLogs and OpentelemetryMetrics. Co-Authored-By: Claude Opus 4.6 (1M context) --- lading_payload/src/opentelemetry/trace.rs | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/lading_payload/src/opentelemetry/trace.rs b/lading_payload/src/opentelemetry/trace.rs index e588c6183..e4f07b88c 100644 --- a/lading_payload/src/opentelemetry/trace.rs +++ b/lading_payload/src/opentelemetry/trace.rs @@ -8,6 +8,7 @@ //! [Specification](https://opentelemetry.io/docs/reference/specification/protocol/otlp/) use crate::{Error, Generator, common::strings}; +use bytes::BytesMut; use opentelemetry_proto::tonic::{ collector::trace, common::v1::{AnyValue, InstrumentationScope, KeyValue, any_value::Value}, @@ -17,6 +18,7 @@ use opentelemetry_proto::tonic::{resource::v1::Resource, trace::v1}; use prost::Message; use rand::{Rng, seq::IndexedRandom}; use std::{ + cell::RefCell, collections::{BTreeMap, BTreeSet}, io::Write, rc::Rc, @@ -1008,6 +1010,7 @@ pub struct OpentelemetryTraces { topology: CompiledTopology, error_rate: f32, str_pool: Rc, + scratch: RefCell, } impl OpentelemetryTraces { @@ -1080,6 +1083,7 @@ impl OpentelemetryTraces { topology, error_rate: config.error_rate, str_pool, + scratch: RefCell::new(BytesMut::with_capacity(4096)), }) } } @@ -1550,9 +1554,18 @@ impl crate::Serialize for OpentelemetryTraces { { let request = self.generate(&mut rng)?; let trimmed = trim_to_fit(request, max_bytes); - let buf = trimmed.encode_to_vec(); - if buf.len() <= max_bytes { - writer.write_all(&buf)?; + let needed = trimmed.encoded_len(); + { + let mut buf = self.scratch.borrow_mut(); + buf.clear(); + let capacity = buf.capacity(); + if capacity < needed { + buf.reserve(needed - capacity); + } + trimmed.encode(&mut *buf)?; + if buf.len() <= max_bytes { + writer.write_all(&buf)?; + } } Ok(()) } From 96032ac37acbbf6a5677c13a11fd10982743ae38 Mon Sep 17 00:00:00 2001 From: Paul Reinlein Date: Fri, 27 Feb 2026 11:42:40 -0500 Subject: [PATCH 3/3] Revert "otel traces: reuse scratch buffer in to_bytes instead of encode_to_vec" This reverts commit c009a6460ee7617a453340a7479fe7e8a4eaa37e. --- lading_payload/src/opentelemetry/trace.rs | 19 +++---------------- 1 file changed, 3 insertions(+), 16 deletions(-) diff --git a/lading_payload/src/opentelemetry/trace.rs b/lading_payload/src/opentelemetry/trace.rs index e4f07b88c..e588c6183 100644 --- a/lading_payload/src/opentelemetry/trace.rs +++ b/lading_payload/src/opentelemetry/trace.rs @@ -8,7 +8,6 @@ //! [Specification](https://opentelemetry.io/docs/reference/specification/protocol/otlp/) use crate::{Error, Generator, common::strings}; -use bytes::BytesMut; use opentelemetry_proto::tonic::{ collector::trace, common::v1::{AnyValue, InstrumentationScope, KeyValue, any_value::Value}, @@ -18,7 +17,6 @@ use opentelemetry_proto::tonic::{resource::v1::Resource, trace::v1}; use prost::Message; use rand::{Rng, seq::IndexedRandom}; use std::{ - cell::RefCell, collections::{BTreeMap, BTreeSet}, io::Write, rc::Rc, @@ -1010,7 +1008,6 @@ pub struct OpentelemetryTraces { topology: CompiledTopology, error_rate: f32, str_pool: Rc, - scratch: RefCell, } impl OpentelemetryTraces { @@ -1083,7 +1080,6 @@ impl OpentelemetryTraces { topology, error_rate: config.error_rate, str_pool, - scratch: RefCell::new(BytesMut::with_capacity(4096)), }) } } @@ -1554,18 +1550,9 @@ impl crate::Serialize for OpentelemetryTraces { { let request = self.generate(&mut rng)?; let trimmed = trim_to_fit(request, max_bytes); - let needed = trimmed.encoded_len(); - { - let mut buf = self.scratch.borrow_mut(); - buf.clear(); - let capacity = buf.capacity(); - if capacity < needed { - buf.reserve(needed - capacity); - } - trimmed.encode(&mut *buf)?; - if buf.len() <= max_bytes { - writer.write_all(&buf)?; - } + let buf = trimmed.encode_to_vec(); + if buf.len() <= max_bytes { + writer.write_all(&buf)?; } Ok(()) }