Skip to content

feat: add diagnostic logging and AgentNode loop test for DataNode executor - #1494

Open
colehurwitz wants to merge 27 commits into
mainfrom
factory/run-e3ddbac6
Open

feat: add diagnostic logging and AgentNode loop test for DataNode executor#1494
colehurwitz wants to merge 27 commits into
mainfrom
factory/run-e3ddbac6

Conversation

@colehurwitz

@colehurwitz colehurwitz commented Sep 10, 2026

Copy link
Copy Markdown
Collaborator

Changes

  • Diagnostic logging in _execute_data (factory/workflow/executor.py):

    • data_item_setup_complete info log after setup() — lists files created in workspace (up to 20)
    • setup_read_path_mismatch warning during setup_reads rescan — detects when a declared read path doesn't match the actual file location by searching for the basename recursively
    • wait_for_reads_timeout_diagnostic warning in _wait_for_reads — logs completed_files snapshot when timeout occurs, aiding debugging of stuck workflows
  • Integration test: DataNode + Loop(AgentNode body, fn GateNode) (tests/test_data_node.py):

    • test_data_node_loop_with_agent_body — verifies 3-iteration RELOOP→PROCEED cycle with a mock agent_fn that appends to counter.txt
    • test_setup_read_path_mismatch_logs_warning — verifies that a mismatched read path causes inner executor halt (with fast timeout patch for CI)

All 747 workflow tests pass (69 in test_data_node.py).

🤖 Generated with Claude Code

colehurwitz and others added 10 commits September 9, 2026 13:03
…rkflows

Add DataItem and DataNode as workflow graph primitives, enabling
data-driven fan-out where a subgraph is executed per data item with
Semaphore-throttled concurrency and per-item fault isolation.

Touches 8 source files across 3 architectural layers:
- Models: DataItem + DataNode with exactly-one-source validation
- Executor: _execute_data with task_ref/inline/source_path resolution
- Validation: structural + reachability checks for both functions
- Skill export: DataNode rendering + subgraph skip-set + trailing else
- Inner loop: cached DataNode detection, executor delegation
- Compose: CAN_ITERATE capability inference
- MAP-Elites: appended has_data_node feature axis (9-tuple)
- Diversity: generalized to len(key) instead of hardcoded range(4)

Closes #1482

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add assert for self.workflow before WorkflowExecutor construction in
_step_with_data_node() to satisfy mypy's type narrowing. Add tests for
directory, jsonl, csv source_format paths and non-existent source_path
in the executor, plus a test for the inner_loop delegation path.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…r loop pipeline

Adds TestDataNodeIntegration class with 5 tests covering:
- DataNode workflow routing through _step_with_data_node()
- DataNode delegates to executor once (not per-instance)
- Non-DataNode workflows still use manual per-instance loop
- compute_features detects DataNode at feature index 8
- Full pipeline: DataNode → InnerLoop.step() → CycleRecord + features

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
DataNodes are infrastructure that drives per-item subgraph execution —
the optimizer should never accidentally mutate or remove them. Add
_auto_frozen_nodes() helper and apply it at both mutation call sites
in SwarmEngine (seed and evolve_generation).

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016Ugj23EAAf1HoHcknrBwhL
…don't timeout

The per-item WorkflowExecutor in _execute_data() was created with empty
completed_files, causing _wait_for_reads to poll for 60s then halt if the
subgraph's start node had reads dependencies on upstream artifacts. Copying
the parent's completed_files into each item_executor lets the subgraph
inherit the parent's artifact state.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…Node

DataNode sub-executors timed out (60s) waiting for files that task.setup()
had already created on disk. _wait_for_reads only checked the
completed_files set, which tracks node outputs — not filesystem state.

Before spawning per-item sub-executors, scan all subgraph node reads and
add any that already exist on disk to completed_files.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
DataNode with task_ref resolved items and ran subgraphs but never called
task.verify(), silently scoring every completed subgraph as 1.0 regardless
of the task's actual evaluation. This made task-defined pass/fail invisible.

Changes:
- executor.py: Pair TaskInstances with DataItems so filters apply in lockstep.
  Move setup()/prompt() from eager pre-loop into run_item() for per-item
  fault isolation. Call task.verify() after each subgraph completes and use
  its score/passed/details instead of binary subgraph success.
- inner_loop.py: _step_with_data_node() now aggregates per-item verify
  scores (mean) from DataNode output instead of using binary exec success.
- inline_items and source_path paths remain unchanged (no Task, no verify).

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
# Conflicts:
#	factory/inner_loop.py
- Fix 1: parallelism default 3→1, Field(ge=1) constraint, worktree isolation for parallelism>1
- Fix 2: write current_item.json for subgraph item visibility (cleaned up in finally)
- Fix 3: reject explicit DataNode→subgraph edges in validation
- Fix 4: raise on missing source_path, warn on empty data, isolate JSONL errors
- Fix 5: seeded shuffle via random.Random(seed), shuffle_seed field on DataNode
- Fix 6: direct DataNode score lookup by node ID + populate instance_results
- Fix 7: document spec deviations in PR body (applied via gh pr edit)
- Fix 8: compose() relaxes capability validation for DataNode workflows

executor.py exceeds 500-line gate (was already 1405, now 1507) — _execute_data
method is tightly coupled to executor internals, splitting would hurt readability.

Addresses review feedback from @lambdabaa on PR #1483.
Fixes compose() DataNode gap from issue #1488 Gap 0.
@colehurwitz

Copy link
Copy Markdown
Collaborator Author

Follow-up fixes from code review

Fix 1: Propagate agent_fn to DataNode sub-executors

Added agent_fn=self._agent_fn to the WorkflowExecutor constructor in _execute_data(), matching how _execute_subgraph_fork() already does it at line 592. Without this, custom agent_fn injected by consumers would be silently ignored for DataNode per-item sub-executors.

Fix 2: Add missing test

Added test_agent_fn_propagates_to_data_node_sub_executor in TestAgentFnInjection. Creates a DataNode workflow with an inline item whose subgraph contains an AgentNode, passes a mock agent_fn, and verifies the mock is called by the sub-executor.

Tests: 101 passed, lint clean, type check clean.

@colehurwitz
colehurwitz added this pull request to stack #1495 September 10, 2026 19:59
colehurwitz and others added 5 commits September 10, 2026 20:08
VerdictType(str, Enum) instances ARE strings, so .value is redundant
and mypy flags it as attr-defined on the str union branch.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Reverts str() back to .value (str() produces 'VerdictType.PROCEED'
instead of 'proceed'). Adds type:ignore to silence mypy since
VerdictType(str, Enum) makes mypy see the str branch.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Gap 0 — Defensive composition catch in InnerLoop._step_with_task():
  Wrap validate_composition() in try/except IncompatibleCompositionError.
  If caught, log warning (composition_incompatible event) and return
  CycleRecord with score=0.0 instead of crashing. Handles post-mutation
  composition failures (e.g. NODE_REMOVE stripping Builder).

Gap 1 — _run_agent() output persistence to node.writes:
  After agent returns stdout, write to node.writes paths using the same
  pattern as _run_llm(). Write unconditionally (not gated on exit code)
  to match _run_llm() behavior.

Gap 2 — agent_fn injection in WorkflowExecutor:
  Add keyword-only agent_fn parameter to __init__ with lazy default to
  invoke_agent. Replace hardcoded import in _run_agent() with
  self._agent_fn(). Propagate to SubgraphForkNode branch executors.
  Leave _evaluate_gate() invoke_agent call unchanged (CEO gate eval).

Tests:
  - test_explicit_empty_caps_passes_research_workflow (compose.py)
  - test_run_agent_persists_to_node_writes (executor.py)
  - test_run_agent_no_writes_skips_file_creation (executor.py)
  - test_custom_agent_fn_used (executor.py)
  - test_agent_fn_defaults_to_invoke_agent (executor.py)
- Add agent_fn=self._agent_fn to WorkflowExecutor constructor in
  _execute_data(), matching _execute_subgraph_fork() at line 592
- Add test_agent_fn_propagates_to_data_node_sub_executor verifying
  custom agent_fn reaches DataNode per-item sub-executors
@colehurwitz
colehurwitz marked this pull request as ready for review September 10, 2026 20:46
@github-actions

github-actions Bot commented Sep 10, 2026

Copy link
Copy Markdown

Sentrux Quality Report

Absolute

Scanning ....
[scan] git ls-files: 652 total, 638 kept, 14 dropped (ext:14, meta:0, big:0)
[build_project_map] 638 files, 110 unique dirs, 101 cache misses, 5.5ms
[resolve] 1295 resolved, 1683 unresolved (of 2978 total specs)
[resolve_imports] project_map 5.6ms, suffix_idx 1.6ms, suffix_resolve 20.0ms, total 27.1ms
[build_graphs] 638 files | maps 2.0ms, imports 27.3ms, calls+inherit 7.2ms, total 36.4ms | 1294 import, 10547 call, 4 inherit edges
sentrux check — 3 rules checked

Quality: 4400

✗ [Error] max_cc: 9 function(s) exceed max cyclomatic complexity of 30
    factory/workflow/executor.py:_execute_data (cc=56)
    factory/outer_loop/reflector.py:_extract_eval_patterns (cc=46)
    factory/cli/_ceo_helpers.py:_validate_ceo_flags (cc=43)
    factory/cli/_ceo_helpers.py:_execute_ceo (cc=43)
    factory/outer_loop/mutations.py:_try_mutation (cc=40)
    examples/chess_evolve.py:main (cc=33)
    factory/cli/_task_builder.py:_build_ceo_task (cc=32)
    factory/outer_loop/reflector.py:_llm_reflect (cc=32)
    factory/outer_loop/mutations.py:validate_and_repair (cc=31)

✗ 1 violation(s) found

Diff (vs base branch)

Scanning ....
[scan] git ls-files: 652 total, 638 kept, 14 dropped (ext:14, meta:0, big:0)
[build_project_map] 638 files, 110 unique dirs, 101 cache misses, 5.6ms
[resolve] 1295 resolved, 1683 unresolved (of 2978 total specs)
[resolve_imports] project_map 5.7ms, suffix_idx 1.1ms, suffix_resolve 20.7ms, total 27.5ms
[build_graphs] 638 files | maps 2.4ms, imports 27.7ms, calls+inherit 7.5ms, total 37.6ms | 1294 import, 10547 call, 4 inherit edges
sentrux gate — structural regression check

Quality:      4415 -> 4400
Coupling:     0.81 → 0.80
Cycles:       3 → 4
God files:    3 → 3

Distance from Main Sequence: 0.35

✗ DEGRADED
  ✗ Cycles increased: 3 → 4

@codecov

codecov Bot commented Sep 10, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 73.91304% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 83.74%. Comparing base (a702827) to head (fc6f217).

Files with missing lines Patch % Lines
factory/inner_loop.py 50.00% 6 Missing ⚠️

❌ Your patch check has failed because the patch coverage (73.91%) is below the target coverage (79.00%). You can increase the patch coverage or adjust the target coverage.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1494      +/-   ##
==========================================
- Coverage   83.76%   83.74%   -0.02%     
==========================================
  Files         225      225              
  Lines       25753    25774      +21     
  Branches     4208     4211       +3     
==========================================
+ Hits        21572    21585      +13     
- Misses       3201     3207       +6     
- Partials      980      982       +2     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Base automatically changed from factory/run-3110c50a to main September 11, 2026 16:13
@github-actions

github-actions Bot commented Sep 11, 2026

Copy link
Copy Markdown

Conflicts resolved

This PR no longer has merge conflicts with main.

Resolve conflicts between PR #1494 (3 execution-layer gap fixes) and
latest main (PRs #1491, #1498, #1483).

Conflict resolutions:
- inner_loop.py: Keep PR's Gap 0 fix (validate_composition try/except)
  and structlog logger
- executor.py: Keep main's hashlib seed, source_format validation,
  import; keep PR's agent_fn propagation (Gap 2) with main's
  'item.prompt or None' safety
- population.py: Keep main's STRUCTURAL_AXES-based diversity metric
- cli/overwrite/parity/skill_export/tool: Keep main's condition_label
  property (cleaner than PR's .value with type:ignore)
- test_data_node.py: Include main's additional test classes

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
colehurwitz pushed a commit that referenced this pull request Sep 11, 2026
Resolve conflicts between PR #1494 (3 execution-layer gap fixes) and
latest main (PRs #1491, #1498, #1483).

Conflict resolutions:
- inner_loop.py: Keep PR's Gap 0 fix (validate_composition try/except)
  and structlog logger
- executor.py: Keep main's hashlib seed, source_format validation,
  import; keep PR's agent_fn propagation (Gap 2) with main's
  'item.prompt or None' safety
- population.py: Keep main's STRUCTURAL_AXES-based diversity metric
- cli/overwrite/parity/skill_export/tool: Keep main's condition_label
  property (cleaner than PR's .value with type:ignore)
- test_data_node.py: Include main's additional test classes

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
colehurwitz and others added 5 commits September 11, 2026 18:41
In _execute_data(), disk_reads was pre-computed ONCE before per-item
processing. When resolved_task.setup() creates files on disk (e.g.
writing test fixtures or input data), those files weren't included in
the sub-executor's completed_files, causing _wait_for_reads() to block
for 60s on files that existed on disk.

Fix: After setup() completes for each item, re-scan subgraph node reads
against item_project_path to find newly-created files. The setup_reads
set is local to each run_item() call, avoiding mutation of the shared
disk_reads set (important when items use different worktree paths).

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…e in structural_hash

BUG 1: DesignerAgent.design_minimal/thorough/custom now accept
seed_workflow and frozen_node_ids params. Frozen nodes from the seed
are injected into every designer variant, preserving the immutability
contract. On ID collision, frozen node takes precedence (with warning).
Engine._add_designer_variants() updated to pass these through.

BUG 2: structural_hash() now explicitly includes a SHA-256 of each
AgentNode prompt_template in the canonical form. While model_dump
already includes it implicitly, this makes prompt hashing robust
against future Pydantic config changes.

Fixes #1488

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
structural_hash now includes prompt content (_prompt_hash), so a
different hash IS proof of novelty. Previously, prompt-only mutations
were rejected because graph_edit_distance returned 0 (same topology)
which is below min_edit_distance (5). Now is_novel returns True
immediately when the hash is not in seen_hashes.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
_add_designer_variants() was only using cfg.frozen_node_ids to build
the frozen set, missing DataNodes detected by _auto_frozen_nodes().
The mutation path (line 179) correctly merges both, but the designer
path did not. This caused designer variants to omit DataNode
"positions", producing 0-score workflows every generation.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…variants

After _inject_frozen_nodes() adds a DataNode to a designer template,
the DataNode was orphaned: no edges pointed to it, start_node was not
updated, and subgraph_entry/exit still referenced seed nodes that do
not exist in the template.

Add _rewire_data_nodes() helper that:
1. Updates subgraph_entry to the template original start_node
2. Updates subgraph_exit to the template terminal node
3. Adds an edge from the DataNode to subgraph_entry
4. Returns the DataNode ID as the new start_node

Call _rewire_data_nodes() in design_minimal, design_thorough, and
design_custom after _inject_frozen_nodes().

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
colehurwitz and others added 2 commits September 12, 2026 01:24
…ence

Bug 1: _rewire_data_nodes() was inserting an explicit edge from the DataNode
to its subgraph_entry. The workflow validator (_validate_datanode_edges)
rejects such edges because the executor handles subgraph execution internally
via DataNode.subgraph_entry — explicit edges cause double-execution.

Removed the edges.insert() call. The executor reads subgraph_entry directly
from the DataNode object, not from the edge list.

Bug 2: When a frozen DataNode ID collides with the template's original_start
(e.g., both named 'researcher'), subgraph_entry was set to itself, creating
a self-referential cycle. Now follows edges from original_start to find the
actual first template node, and removes the now-stale edges from
original_start that would become invalid DataNode-to-subgraph edges.

Tests updated:
- Existing edge assertions flipped to assert edges do NOT exist
- test_rewired_workflow_validates_graph: validates rewired workflow passes
- test_data_node_id_collision_with_start: verifies no self-reference

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@colehurwitz colehurwitz changed the title fix: close 3 execution-layer gaps for Task+Workflow consumers fix: remove invalid DataNode edge and guard subgraph_entry self-reference Sep 12, 2026
colehurwitz and others added 3 commits September 12, 2026 03:20
…ging

- Add _validate_datanode_exit() in validation.py: warns when DataNode's
  subgraph_exit points to a GateNode (drops PROCEED edge, silently breaks
  loop termination)
- Add gate_proceed_edge_missing structured warning in executor.py: logs
  when a gate evaluates PROCEED but no PROCEED edge exists
- Add 3 integration tests in test_data_node.py:
  - test_data_node_with_loop_subgraph: end-to-end Loop body runs 3x
  - test_data_node_loop_wrong_exit_warns: wrong subgraph_exit caught
  - test_loop_package_compiled_preserves_edges: all 3 loop edges preserved

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The _validate_datanode_exit function was producing false positives by
warning whenever subgraph_exit pointed to ANY GateNode. Terminal GateNodes
(no RELOOP edges) are valid as subgraph_exit — only GateNodes participating
in a Loop (with outgoing RELOOP edges) cause the PROCEED edge drop issue.

Now checks for RELOOP edges before emitting the warning.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@colehurwitz colehurwitz changed the title fix: remove invalid DataNode edge and guard subgraph_entry self-reference fix: refine DataNode exit validation to only warn for Loop GateNodes Sep 12, 2026
…cutor

- Add data_item_setup_complete log after setup() to list workspace files
- Add setup_read_path_mismatch warning when declared reads differ from actual file paths
- Add wait_for_reads_timeout_diagnostic warning with completed_files snapshot
- Add integration test: DataNode + Loop(AgentNode body, fn GateNode) with 3-iteration RELOOP
- Add test: setup_read_path_mismatch detects file at wrong relative path

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@colehurwitz colehurwitz changed the title fix: refine DataNode exit validation to only warn for Loop GateNodes feat: add diagnostic logging and AgentNode loop test for DataNode executor Sep 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant