Skip to content

Run same-level layers in parallel#105

Merged
marklubin merged 3 commits intomainfrom
parallel-layers
Apr 3, 2026
Merged

Run same-level layers in parallel#105
marklubin merged 3 commits intomainfrom
parallel-layers

Conversation

@marklubin
Copy link
Copy Markdown
Owner

Summary

Layers at the same DAG level are independent. The runner now groups layers by level and executes each group concurrently via ThreadPoolExecutor.

For a pipeline with 5 fold layers at level 2 (~30 min each), this reduces total build time from ~2.5 hours to ~30 minutes.

Changes

  • src/synix/build/runner.py: Extract _run_single_layer(), group by level, concurrent execution with _txn_lock for snapshot thread safety
  • tests/unit/test_parallel_layers.py: 4 tests — parallel timing, single-layer inline, dependency ordering, 5-layer concurrent

Test plan

  • 4 new parallel layer tests pass (including timing assertion proving concurrency)
  • 1350 total tests pass (0 regressions)
  • ruff clean

Layers at the same DAG level are independent — their dependencies are
already computed. The runner now groups layers by level and executes
each group concurrently. Single-layer levels still run inline.

Thread safety: snapshot_txn writes protected by threading.Lock.
Stats accumulated from return values, not shared mutation.

For a pipeline with 5 fold layers at level 2 (~30 min each), this
reduces build time from ~2.5 hours to ~30 minutes.
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 3, 2026

Note

Red Team Review — OpenAI GPT-5.4 | Adversarial review (docs + diff only)

Threat assessment — Medium-high risk: this changes execution semantics in the build runner, introduces concurrency around shared state, and the tests only prove speed, not correctness.

One-way doors

  1. Parallel execution by DAG level in src/synix/build/runner.py
    This changes observable build behavior: log ordering, checkpoint timing, snapshot write order, failure timing, and potentially artifact selection under accept_existing. Users and downstream code will start depending on this behavior once shipped. Hard to reverse because “same pipeline, different scheduling” can expose latent nondeterminism and cache/provenance differences. Safe to merge only if snapshot contents, cache decisions, provenance, and release outputs are proven identical to sequential execution for equivalent inputs.

Findings

  1. src/synix/build/runner.py — concurrent mutation of result, slogger, and layer-local callbacks without synchronization
    _txn_lock only protects snapshot writes/checkpoints. But worker threads still call slogger.layer_start, artifact_built, artifact_cached, and mutate stats/layer_built from callbacks that may run inside _execute_transform_concurrent. If StructuredLogger or callback paths are not thread-safe, you get interleaved/corrupt logs or miscounted stats. Severity: [warning]

  2. src/synix/build/runner.py — reliance on layer._level as concurrency boundary
    The PR assumes “same level = independent,” but there’s no evidence _level is a formally validated topological partition rather than a convenience field. If any non-data dependency exists outside depends_on (shared work dir, search surface, projection side effects, implicit ordering), this will break silently. Severity: [critical]

  3. src/synix/build/runner.py — failure semantics changed without rollback strategy
    With as_completed, one layer can finish and checkpoint/materialize surfaces while a sibling at the same level fails later. That creates partially advanced build state for a logically single build phase. I see no explicit cancellation of sibling futures, no cleanup, and no statement that partial same-level completion is valid. Severity: [critical]

  4. src/synix/build/runner.py — search surface materialization now races conceptually with sibling completion
    _finish_layer materializes search surfaces immediately after each layer finishes, while other same-level layers may still be running. If surfaces or downstream work expect a stable view of all artifacts at a level, you now expose transient incomplete state under .synix/work/surfaces/. That directly conflicts with the project’s provenance/build-snapshot story. Severity: [warning]

  5. src/synix/build/runner.py — ThreadPool size equals number of layers
    ThreadPoolExecutor(max_workers=len(group)) is unbounded relative to user config and ignores the existing concurrency parameter. A wide level can oversubscribe threads badly, especially since transforms may themselves run concurrently via _execute_transform_concurrent. This is nested parallelism with no backpressure. Severity: [warning]

  6. src/synix/build/runner.py — nondeterministic completion order may perturb snapshot/artifact ordering
    Same-level futures are consumed via as_completed, so result ordering is now timing-dependent. If any downstream code, snapshot serialization, release receipt, or tests assume deterministic layer/artifact order, this will create flaky behavior. Pre-1.0 doesn’t excuse avoidable nondeterminism in a build system. Severity: [warning]

  7. tests/unit/test_parallel_layers.py — tests assert elapsed time, not semantic equivalence
    These are performance smoke tests. They do not verify identical snapshot contents, identical provenance parent labels, deterministic output ordering, correct checkpointing, correct cache reuse, failure behavior, or release/search behavior. For a scheduler change in a build system, that is inadequate. Severity: [critical]

  8. tests/unit/test_parallel_layers.py — no coverage for concurrent sources, projections, or surface layers
    The code path now wraps all layers via _run_single_layer, but tests only use trivial Transforms. Nothing covers multiple same-level Sources, cached layers, accept_existing, incremental transforms, or release/search interactions. Severity: [warning]

Missing

  • A correctness test that sequential and parallel execution produce identical snapshot state, provenance, and release outputs.
  • Tests for failure in one same-level layer while siblings are running.
  • Tests for cache behavior under accept_existing and _supports_incremental in parallel mode.
  • Tests for deterministic ordering of result.layer_stats / snapshot manifests if ordering is part of the contract.
  • Documentation update: README/design docs say DAG-based/incremental, but nothing mentions same-level parallel scheduling or its limits.
  • Validation that StructuredLogger, snapshot transaction, and surface materialization are thread-safe.

VerdictBlock: this is a scheduler change in the core build engine, and the PR does not establish semantic safety, only that some toy transforms finish faster.

Review parameters
  • Model: gpt-5.4
  • Context: README.md, DESIGN.md, synix.dev, PR diff
  • Diff size: 462 lines
  • Prompt: .github/prompts/openai_review.md
  • Timestamp: 2026-04-03T23:25:05Z

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 3, 2026

Note

Architectural Review — Claude Opus | Blind review (docs + diff only)

Summary

This PR parallelizes execution of independent layers within the build runner. Layers at the same DAG level (no dependency edges between them) now run concurrently via ThreadPoolExecutor, with a threading.Lock protecting shared mutable state (snapshot_txn, DLQ). The former sequential loop is refactored into _run_single_layer (pure execution) and _finish_layer (state updates), then dispatched by level groups.

Alignment

Strong fit. The DAG is the core structural concept (DESIGN.md §3, §1.2), and exploiting level-parallelism is the natural optimization the design anticipated ("transforms are embarrassingly parallel" — §4.4.2). Artifacts remain immutable and content-addressed; provenance chains are unmodified. Cache keys still capture all inputs. The change respects the build-system mental model exactly: independent compilation units parallelize, dependent ones don't.

Observations

  1. [concern] _run_single_layer captures layer_artifacts (the shared dict mapping layer names to outputs) via closure and reads it in _gather_inputs without holding _txn_lock. This is safe only because same-level layers are guaranteed independent — they never read each other's outputs. But the invariant is implicit. If resolve_build_order or _level assignment ever has a bug, you get a silent data race. A defensive assertion (e.g., confirming no layer in the group depends on another layer in the same group) would make this load-bearing assumption explicit.

  2. [concern] _finish_layer is called from as_completed — meaning layers at the same level may finish in arbitrary order. _finish_layer appends to result.layer_stats and mutates result.built/result.cached without a lock. These are written from the main thread but only after as_completed yields, so this is actually single-threaded. Fine — but the inline single-layer path and the concurrent path both call _finish_layer from the main thread, so this holds. Worth a comment noting this invariant.

  3. [concern] _save_artifact (the inner closure inside _run_single_layer for transforms) appends to layer_built and reads/writes result.dlq indirectly via dlq_before. layer_built is local to each _run_single_layer invocation (safe). But result.dlq is shared — if an exception handler elsewhere appends to result.dlq during concurrent execution, dlq_before comparisons could be wrong. Verify that DLQ writes only happen inside _run_single_layer scope (appears true from the diff, but only partially visible).

  4. [question] ThreadPoolExecutor(max_workers=len(group)) — for a wide DAG (e.g., 50 independent layers), this spawns 50 threads. Given that transforms may each use their own concurrency parameter for internal parallelism, this could create thread explosion. Should max_workers be capped at concurrency or some configurable limit?

  5. [positive] Clean decomposition into _run_single_layer / _finish_layer. The extraction makes the control flow significantly more legible than the previous monolithic loop, independent of the parallelism benefit.

  6. [positive] The test suite covers the important cases: actual timing-based parallelism verification, single-layer degenerate case, sequential dependency ordering, and N-wide fan-out. The timing assertion (< 0.8s for 3×0.3s sleeps) has reasonable margin.

  7. [nit] Several comment removals (e.g., # Source layer — call load(), # Split inputs into work units) reduce inline documentation. These were useful orientation markers for new readers. The refactored structure is clearer, but _run_single_layer is still ~100 lines with nested closures — some section headers would help.

  8. [question] No test covers error propagation from a concurrent layer. If one of three parallel layers raises, does future.result() propagate the exception? What happens to the other in-flight layers? The ThreadPoolExecutor context manager will wait, but the exception will surface only after all futures complete.

Verdict

Good incremental step — correct exploitation of the DAG structure with reasonable thread safety, but needs a cap on max_workers and a defensive assertion that same-level layers are truly independent before merging.

Review parameters
  • Model: claude-opus-4-6
  • Context: README.md, DESIGN.md, synix.dev, PR diff
  • Diff size: 462 lines
  • Prompt: .github/prompts/claude_review.md
  • Timestamp: 2026-04-03T23:25:20Z

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 3, 2026

Note

Red Team Review — OpenAI GPT-5.4 | Adversarial review (docs + diff only)

Threat assessment — Moderate risk: this changes execution semantics in the build runner and the tests are too shallow for the concurrency hazards introduced.

One-way doors

  1. Parallel execution by DAG level in src/synix/build/runner.py — This changes observable build behavior: layer completion order, log order, snapshot/checkpoint timing, and likely any side effects in transforms. Users and internal code will start depending on “same-level layers run concurrently.” Hard to reverse once templates, docs, and plugins assume performance/ordering characteristics. Safe to merge only if layer API explicitly guarantees thread-safe, side-effect-free execution and snapshot/release semantics are proven deterministic under reordering.

Findings

  1. src/synix/build/runner.py — grouping by layer._level and running with ThreadPoolExecutor(max_workers=len(group))
    You’re assuming “same level = independent.” That only holds for declared DAG dependencies, not hidden shared resources: same source dir, same work dir, same LLM client, same temp files, same search surface paths, same snapshot transaction. The diff adds locks only around snapshot writes/checkpoints, not around any other shared mutable state. Failure mode: nondeterministic corruption, flaky builds, provider client races, temp-file collisions. Severity: [critical]

  2. src/synix/build/runner.py_run_single_layer mutates shared result.dlq length without synchronization (dlq_before = len(result.dlq))
    stats.dlq_count is now race-prone. If parallel layers append to DLQ concurrently, per-layer counts become garbage. That undermines observability/debugging exactly where concurrency makes debugging harder. Severity: [warning]

  3. src/synix/build/runner.py_finish_layer() materializes search surfaces after each future completes
    This introduces order nondeterminism at the surface/materialization boundary. If two level-N layers both feed later surfaces or write under shared work paths, completion timing now affects what exists when. The design emphasizes provenance and content-addressed artifacts; nondeterministic side-effect timing is the opposite direction. Severity: [warning]

  4. src/synix/build/runner.py — checkpoint timing changed from strictly build-order to completion-order
    write_layer_checkpoint(snapshot_txn, layer.name) used to happen in deterministic traversal order; now same-level checkpoints occur in whichever thread finishes first. If recovery code assumes monotonic layer progression, interrupted builds may resume inconsistently. No evidence here that checkpoint semantics were audited for out-of-order same-level completion. Severity: [warning]

  5. tests/unit/test_parallel_layers.py — only tests wall-clock overlap, not correctness under contention
    These tests prove “sleep overlaps,” not that real transforms remain correct. No assertions on deterministic snapshot contents, provenance parent labels, checkpoint recovery, DLQ accounting, or search surface materialization under parallel execution. This is not enough coverage for a runner semantic change. Severity: [warning]

  6. templates/03-team-report/golden/*.stdout.txt — output order changed
    This confirms user-visible nondeterminism in CLI logs. Maybe acceptable, maybe not, but there’s no docs update explaining that same-level layer order is no longer stable. If users diff CI logs or rely on golden output ordering, you’ve created churn without contract clarification. Severity: [minor]

Missing

  • Tests for deterministic snapshot contents/provenance when same-level layers run in parallel.
  • Tests for checkpoint/restart behavior after interruption mid-level.
  • Tests for DLQ accounting under concurrent failures.
  • Tests for shared work_dir/surface materialization collisions.
  • Validation that Source.load, Transform.execute, logger usage, and any provider clients are thread-safe.
  • Documentation update for changed runner semantics and nondeterministic layer log ordering.
  • Any configurable switch/feature flag to disable parallel-by-level execution if it breaks a user pipeline.

VerdictBlock: the speedup idea is reasonable, but this PR changes core execution semantics with insufficient correctness guarantees, insufficient test coverage, and obvious race surfaces outside the narrow lock scope.

Review parameters
  • Model: gpt-5.4
  • Context: README.md, DESIGN.md, synix.dev, PR diff
  • Diff size: 510 lines
  • Prompt: .github/prompts/openai_review.md
  • Timestamp: 2026-04-03T23:30:47Z

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 3, 2026

Note

Architectural Review — Claude Opus | Blind review (docs + diff only)

Summary — This PR parallelizes independent layers at the same DAG level. It refactors the sequential build loop into _run_single_layer + _finish_layer, groups layers by their _level, and dispatches same-level layers via ThreadPoolExecutor. A threading lock protects snapshot transaction writes.

Alignment — Fits well. The DAG structure already encodes independence — layers at the same level have no dependency edges between them. Parallelizing them is a natural optimization for the build-system model. Immutability of artifacts means concurrent production is safe in principle. Content-addressed storage and provenance chains are not altered by execution order. This advances the "expensive LLM transforms" concern from DESIGN.md §2.1 (dbt comparison) — parallelism directly reduces wall-clock time.

Observations

  1. [concern] Race condition on result.dlq. _run_single_layer reads dlq_before = len(result.dlq) and later computes stats.dlq_count = len(result.dlq) - dlq_before. When multiple layers execute concurrently, DLQ appends from sibling threads corrupt this delta. result.dlq is not protected by _txn_lock. This will silently miscount DLQ items under parallel error conditions.

  2. [concern] Thread pool nesting. Within-layer concurrency (_execute_transform_concurrent uses its own ThreadPoolExecutor with the concurrency param) can now run inside the between-layer ThreadPoolExecutor. With N parallel layers × M concurrency, you get N×M threads. For a pipeline with 5 parallel transform layers each at concurrency=5, that's 25 threads hitting the LLM provider simultaneously. No aggregate cap exists.

  3. [concern] _gather_inputs reads layer_artifacts without a lock. This is currently safe because sibling layers only depend on prior levels (already written). But nothing enforces this invariant explicitly — a future bug in level assignment would silently produce wrong inputs. A defensive assertion (assert all deps in layer_artifacts) before launching the parallel group would make the contract explicit.

  4. [question] max_workers=len(group) — is unbounded thread creation intentional? A pipeline with many independent sources (20+ files → 20+ Source layers at level 0) spawns that many threads. A cap like min(len(group), concurrency) would be more predictable.

  5. [nit] Control flow in _run_single_layer. Source and Transform return explicitly, but projection/surface layers fall through the if/elif to a default return at the bottom. An explicit else branch would make the three-way dispatch clearer and prevent silent bugs if a new layer type is added.

  6. [positive] The refactoring into _run_single_layer / _finish_layer is clean. Separating execution from post-processing makes the concurrency boundary obvious. _finish_layer runs sequentially on the main thread, keeping layer_artifacts and result accumulation single-threaded (except for the DLQ issue above).

  7. [positive] Golden file updates and test coverage are solid. The timing-based parallel test is well-designed (generous margin, clear assertion message). Tests cover the key cases: parallel, single, sequential, and fan-out. The golden output change (order swap of bios/project_brief) correctly reflects non-deterministic completion order.

  8. [nit] No test for error/DLQ behavior under parallelism. Given observation Implement Synix v0.1 - declarative pipeline for AI conversation exports #1, a test where one of N parallel layers fails would exercise the racy path.

Verdict — Good incremental step that exploits the DAG structure naturally, but the result.dlq race condition is a real bug that should be fixed before merge, and the nested thread pool interaction deserves at minimum a cap or a documented decision.

Review parameters
  • Model: claude-opus-4-6
  • Context: README.md, DESIGN.md, synix.dev, PR diff
  • Diff size: 510 lines
  • Prompt: .github/prompts/claude_review.md
  • Timestamp: 2026-04-03T23:31:20Z

- ThreadPoolExecutor workers capped at min(group_size, concurrency)
  to avoid oversubscription with nested transform parallelism
- Search surfaces materialize AFTER all same-level layers complete,
  not per-layer — prevents exposing incomplete state when a surface
  depends on multiple layers at the same level
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 3, 2026

Note

Red Team Review — OpenAI GPT-5.4 | Adversarial review (docs + diff only)

Threat assessment — Medium-high risk: this changes core build execution semantics with concurrency, but the tests only cover happy-path timing and miss correctness, determinism, and failure behavior.

One-way doors

  1. Parallel execution by _level in src/synix/build/runner.py — This changes observable build behavior (log order, checkpoint timing, surface materialization timing, race profile). Users and tests will start depending on it once shipped. Safe only if layer independence is formally guaranteed by the graph model, output ordering is explicitly documented as nondeterministic within a level, and correctness is proven under concurrent execution.
  2. Using layer._level as the scheduling boundary_level is now effectively part of execution semantics, not just metadata. Hard to reverse if other code/templates/plugins assume “same level means parallel-safe.” Safe only if _level is canonical, stable, and impossible to desynchronize from actual dependencies.

Findings

  1. src/synix/build/runner.py — shared mutable layer_artifacts read during concurrent execution
    _run_single_layer() calls _gather_inputs(layer, layer_artifacts, store) while sibling layers are running and _finish_layer() / completion handlers mutate layer_artifacts. You’re assuming same-level layers never read each other, but that’s implicit and unvalidated. If a bad _level assignment or future layer type violates that assumption, you get heisenbugs. Severity: [critical]

  2. src/synix/build/runner.py — snapshot transaction shared across threads with partial locking
    Only _record_snapshot_artifact() and write_layer_checkpoint() are locked. Everything else around snapshot_txn lifecycle is assumed thread-safe without evidence. If start_build_transaction() / commit path / internal caches are not thread-safe, you can corrupt snapshot state silently. Severity: [critical]

  3. src/synix/build/runner.py — nondeterministic result ordering introduced
    result.layer_stats.append(stats) now happens in as_completed() order for same-level layers. The golden file update already shows output order changed. That means stdout and probably API-visible stats are now timing-dependent. This is exactly the kind of flaky behavior that poisons tests and downstream tooling. Severity: [warning]

  4. src/synix/build/runner.py — exception handling in parallel path is brittle
    future.result() will raise and abort the whole level, but there’s no cleanup/rollback logic shown for partially completed sibling layers. You can end up with some artifacts/checkpoints written and others not, which is dangerous in a build system claiming immutable snapshots and recoverability. Severity: [critical]

  5. src/synix/build/runner.py — comment “layers at the same level are independent” is asserted, not enforced
    There is no validation step proving same-level nodes have no cross-layer coupling via surfaces, shared labels, shared work_dir state, or transform side effects. Given Synix’s Python-first model, user code can absolutely have side effects. This is a leaky abstraction. Severity: [warning]

  6. src/synix/build/runner.py — possible oversubscription remains
    You cap outer workers, but each transform can still use internal concurrency (_execute_transform_concurrent). A few same-level layers can explode thread count and API parallelism. This will fail under rate limits long before it helps throughput. Severity: [warning]

  7. tests/unit/test_parallel_layers.py — tests validate speed, not correctness
    These are benchmark-ish timing tests with sleeps. They do not verify snapshot contents, provenance parent labels, checkpoint correctness, deterministic releases, or behavior on cache hits/failures. Fast tests that ignore state integrity are the wrong tests for this PR. Severity: [warning]

  8. tests/unit/test_parallel_layers.py — flaky wall-clock assertions
    assert elapsed < 0.8 is environment-sensitive. CI jitter, slow filesystems, or loaded runners will randomly fail. Timing-based unit tests are a maintenance tax. Severity: [minor]

Missing

  • Tests for failure in one parallel layer while siblings succeed: snapshot consistency, rollback/recovery, and rerun behavior.
  • Tests for cached builds under parallel same-level execution.
  • Tests that provenance and parent labels are identical between sequential and parallel execution.
  • Tests for surface materialization correctness when surfaces depend on multiple same-level layers.
  • Validation/documentation that same-level execution order is intentionally nondeterministic.
  • Evidence that snapshot_txn and store operations are thread-safe under concurrent access.
  • Any guardrail preventing side-effectful transforms from trampling shared work_dir state.

VerdictBlock: core execution semantics changed without enough correctness proof, and the current tests are too shallow to justify concurrent writes in the build engine.

Review parameters
  • Model: gpt-5.4
  • Context: README.md, DESIGN.md, synix.dev, PR diff
  • Diff size: 534 lines
  • Prompt: .github/prompts/openai_review.md
  • Timestamp: 2026-04-03T23:32:33Z

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 3, 2026

Note

Architectural Review — Claude Opus | Blind review (docs + diff only)

Summary

This PR introduces parallel execution of independent layers within the build runner. Layers at the same DAG level (no mutual dependencies) now run concurrently via ThreadPoolExecutor, while cross-level ordering is preserved. The core run() loop is refactored into _run_single_layer and _finish_layer helpers, with a threading.Lock protecting snapshot transaction writes.

Alignment

Strong fit. The DAG is the foundational abstraction (DESIGN.md §3.1), and exploiting its structure for parallel execution is a natural build-system optimization — Bazel does the same thing. Layers remain immutable artifacts with provenance; the DAG topology determines what can parallelize. Content-addressed caching and materialization keys are untouched. This directly addresses the "LLM transforms are expensive" observation (§2.1) by reducing wall-clock time without changing semantics.

Observations

  1. [concern] result object mutations are unprotected. In the parallel as_completed loop (~line 410), result.built += stats.built, result.cached +=, result.layer_stats.append() are modified outside _txn_lock. CPython's GIL makes this work today, but it's brittle and inconsistent with the stated threading discipline. Either wrap these in the lock or document the GIL assumption.

  2. [concern] DLQ counting is racy. _run_single_layer computes dlq_before = len(result.dlq) then stats.dlq_count = len(result.dlq) - dlq_before. Two concurrent layers will interleave DLQ appends and get incorrect per-layer counts.

  3. [concern] Logic duplication between _finish_layer and the parallel code path. The parallel branch inlines the same stat-accumulation and checkpoint logic that _finish_layer encapsulates, except it defers surface materialization. If _finish_layer gains new steps later, the parallel path will silently diverge. Consider refactoring into _finish_layer(layer, stats, built, defer_surfaces=True).

  4. [concern] No test for failure in a parallel layer. If one layer raises inside the thread pool, future.result() will propagate the exception, but sibling layers are still running. The snapshot transaction may be left in an inconsistent state (some layers checkpointed, others not). This needs at least a test, and ideally explicit cancellation or rollback logic.

  5. [question] Thread oversubscription. max_workers = min(len(group), concurrency), but each layer may internally spawn up to concurrency threads via _execute_transform_concurrent. With 5 same-level layers at concurrency=5, you get up to 25 threads hitting the LLM. Is this intentional? Rate-limit pressure could cause cascading retries.

  6. [question] Golden test ordering. The 03-team-report golden files now show project_brief before bios (both level 0). Since as_completed returns in finishing order, this could flip between runs and cause flaky golden comparisons. Is there a normalizer in the test harness?

  7. [nit] _run_single_layer returns (layer, stats, []) as a fallthrough for non-Source/non-Transform layers. An explicit elif isinstance(layer, ...) or a comment clarifying this catch-all would improve readability for newcomers.

  8. [positive] The timing-based parallelism test (test_same_level_layers_run_in_parallel) is a practical way to verify the behavior, and the generous margin (0.8s vs 0.9s sequential) should avoid most CI flakiness.

  9. [positive] Surface materialization is correctly deferred until all same-level layers complete — this handles cross-layer search surfaces cleanly.

Verdict

Good incremental step that delivers meaningful wall-clock speedup while respecting the DAG structure, but the thread-safety gaps around result mutations and DLQ counting should be fixed before merge, and the missing error-path test is a real gap for a build system that promises recoverability.

Review parameters
  • Model: claude-opus-4-6
  • Context: README.md, DESIGN.md, synix.dev, PR diff
  • Diff size: 534 lines
  • Prompt: .github/prompts/claude_review.md
  • Timestamp: 2026-04-03T23:33:18Z

@marklubin marklubin merged commit ff6546d into main Apr 3, 2026
19 of 23 checks passed
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