refactor(optimize-sorting): restructure benchmark flow with catastrophic gate - #1430
Closed
colehurwitz wants to merge 5 commits into
Closed
colehurwitz wants to merge 5 commits into
colehurwitz wants to merge 5 commits into
Conversation
Implement the three-tier optimize-sorting workflow for iterative speed optimization of spike sorting pipelines with hard accuracy constraints. New files: - factory/workflow/contributed/optimize_sorting/__init__.py - factory/workflow/contributed/optimize_sorting/workflow.py (25 nodes, 33 edges, 10 unique prompt templates, 7 helper functions) - factory/workflow/contributed/optimize_sorting/test_workflow.py (82 tests) Modified files: - factory/cli/_helpers.py: add optimize-sorting to CEO_MODES - factory/cli/_task_builder.py: add mode suffix, tier-keyword focus suppression - factory/cli/_ceo_helpers.py: add to focus allowlist, write focus.txt, skip add_backlog_item for tier-prefixed focus - factory/workflow/skill_export.py: add WORKFLOW_META entry - factory/workflow/definitions.py: register in _get_builtin_registry File-Size Gate note: workflow.py is 835 lines, exceeding the 500-line limit. This is a self-contained workflow definition with inline python3 -c commands and 10+ prompt templates that must be co-located per the contributed workflow pattern. Splitting would break the single-file pattern used by all other contributed workflows.
Sentrux Quality ReportAbsoluteDiff (vs base branch) |
Add 'git checkout main' at the start of the select_tier FnNode command so each experiment starts from a clean main branch state. This ensures experiments are measured independently against the pinned baseline. The baseline.json lock_baseline step remains idempotent (skips if baseline already exists), preserving main branch performance as the fixed reference point. Experiment branches are created downstream by the builder agents.
…cy gate passes After the accuracy gate passes (verdict=keep), run 2 additional benchmark runs (total 3) and compute mean/std for accuracy and speed_seconds before archiving. This gives statistical confidence that the improvement is real. Changes: - Add _benchmark_3x_command() helper: reads existing benchmark-result.json (run 1), runs 2 more benchmarks, averages all 3, and overwrites benchmark-result.json with mean/std fields (same format as baseline.json) - Add 3 new FnNodes: confirm_benchmark_t1, confirm_benchmark_t2, confirm_benchmark_t3 - Wire: accuracy_gate --PROCEED--> confirm_benchmark --> archive_result (HALT path unchanged — goes directly to archive for revert recording) - For T3: gate_per_unit_accuracy --PROCEED--> confirm_benchmark_t3 - Update _archive_prompt() to handle averaged vs single-run result formats - Node count: 25 → 28, Edge count: 33 → 36 File-size note: workflow.py (957 lines) exceeds 500-line gate but was already 857 lines. Splitting a single workflow definition would fragment the node/edge graph and harm readability. Closes #1431
…hic gate
Restructure the three-tier benchmark flow from:
run_benchmark → accuracy_gate (1 run) → confirm_benchmark → archive
to:
run_benchmark → catastrophic_gate → confirm_benchmark → accuracy_gate → archive
Changes:
- Add gate_catastrophic_t{1,2,3} GateNodes that only catch >10% accuracy
drops (acc < baseline_mean * 0.9). HALT → archive, PROCEED → confirm.
- Move confirm_benchmark_t{1,2,3} BEFORE accuracy gate so 3 runs always
execute when not catastrophic.
- Add new gate_accuracy_t{1,2,3} GateNodes AFTER confirm_benchmark that
check the averaged 3-run result. Output pass/fail only (no reloop since
3-run average is definitive).
- Update _accuracy_gate_command() to handle averaged format: check if
br['accuracy'] is a dict and use br['accuracy']['mean'].
- Update _per_unit_gate_command() to handle averaged per_unit_accuracy
format: check br['per_unit_accuracy'][uid]['mean'] vs baseline.
- Update _archive_prompt() to describe new flow semantics (catastrophic
gate halt = single-run format, accuracy gate = averaged format).
- Rewire edges: 31 nodes, 39 edges (was 28 nodes, 36 edges).
… topology - Update test node counts: 28→31 (add gate_catastrophic_t1/t2/t3) - Update test edge count: 36→39 - Update test reloop count: 5→2 (accuracy gate reloops removed) - Fix edge assertions: run_benchmark→gate_catastrophic→confirm→gate_accuracy→archive - Add test for catastrophic gate node types - Remove stale gate_accuracy_t1 reloop reference from builder T1 prompt - Remove stale gate_accuracy_t2 reloop reference from builder T2 prompt Closes #1432
Collaborator
Author
|
Superseded by unified PR — see #1443 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Changes
Added
gate_catastrophic_t{1,2,3}GateNodes: New first-line gates afterrun_benchmarkthat only catch catastrophic accuracy drops (>10% below baseline mean). On HALT, skip confirmation runs and go straight to archive. On PROCEED, continue toconfirm_benchmark.Moved
confirm_benchmark_t{1,2,3}before accuracy gate: Confirmation runs (2 additional benchmarks for 3-run average) now always execute when not catastrophic, rather than only on accuracy pass.Added new
gate_accuracy_t{1,2,3}GateNodes afterconfirm_benchmark: These check the averaged 3-run result. Output pass (PROCEED) or fail (HALT) only — no reloop since 3-run average is definitive.Updated
_accuracy_gate_command(): Handles averaged format by checking ifbr['accuracy']is a dict and usingbr['accuracy']['mean']. Removed catastrophic check (now handled by dedicated gate). Removed reloop output.Updated
_per_unit_gate_command(): Handles averagedper_unit_accuracyformat — checksbr['per_unit_accuracy'][uid]['mean']when value is a dict.Updated
_archive_prompt(): Describes new flow semantics — catastrophic gate halt produces single-run format, accuracy gate produces averaged format.Edge rewiring: New flow per tier:
run_benchmark → gate_catastrophic --PROCEED→ confirm_benchmark → gate_accuracy --PROCEED→ archivegate_catastrophic --HALT→ archive(catastrophic failure, skip extra runs)gate_accuracy --HALT→ archive(revert based on averaged result)gate_accuracy --PROCEED→ gate_per_unit_accuracy → archiveGraph size: 31 nodes, 39 edges (was 28 nodes, 36 edges)
Test & prompt fixes (Closes #1432)
gate_catastrophic_t1/t2/t3to tier node sets; total 28→31, per-tier counts +1 eachrun_benchmark → gate_catastrophic → confirm → gate_accuracy → archive)test_catastrophic_gates_are_gate_nodesgate_accuracy_t1reloop referencegate_accuracy_t2reloop handling section