|
| 1 | +# Sable audit — AUD-OVERNIGHT-02: parallel evaluator import handling |
| 2 | + |
| 3 | +*By Sable. Gate: severity rating and fix/accept decision before v3.0 parallel work begins.* |
| 4 | +*Date: 2026-05-14* |
| 5 | + |
| 6 | +--- |
| 7 | + |
| 8 | +## Scope |
| 9 | + |
| 10 | +The parallel evaluator in `crates/codifide-interpreter/src/interpreter.rs` |
| 11 | +(`eval_parallel_exprs`) creates branch interpreters with `resolved_imports: |
| 12 | +HashMap::new()`. This means imported symbols are not available inside parallel |
| 13 | +branches. The gap is documented in the source with a `// Note:` comment and |
| 14 | +tracked as AUD-OVERNIGHT-02. This audit rates the severity, characterises the |
| 15 | +failure mode, and decides: fix before v3.0 or formally accept as a known |
| 16 | +limitation. |
| 17 | + |
| 18 | +--- |
| 19 | + |
| 20 | +## The gap — exact location |
| 21 | + |
| 22 | +`interpreter.rs`, `eval_parallel_exprs`, inside the `rayon::scope` closure: |
| 23 | + |
| 24 | +```rust |
| 25 | +let mut branch_interp = Interpreter { |
| 26 | + module, |
| 27 | + max_depth, |
| 28 | + depth: current_depth, |
| 29 | + prims: build_default_registry(), |
| 30 | + trace: EffectTrace::fresh(), |
| 31 | + // Note: resolved_imports is not passed to branch interpreters. |
| 32 | + // Imported symbols are not available in parallel branches. |
| 33 | + // This is a known limitation (AUD-OVERNIGHT-02). If a parallel |
| 34 | + // branch calls an imported symbol, it will fail with |
| 35 | + // unknown_callable. Fix: pass resolved_imports here when the |
| 36 | + // parallel evaluator gains full import support. |
| 37 | + resolved_imports: HashMap::new(), |
| 38 | +}; |
| 39 | +``` |
| 40 | + |
| 41 | +The Python interpreter has no parallel evaluator, so this gap is Rust-only. |
| 42 | + |
| 43 | +--- |
| 44 | + |
| 45 | +## Failure mode |
| 46 | + |
| 47 | +A program that: |
| 48 | +1. Imports a symbol by content hash (`import f = sha256:...`), AND |
| 49 | +2. Calls that symbol inside a `list(...)` or `++` expression that the |
| 50 | + parallel evaluator decides to parallelize |
| 51 | + |
| 52 | +will fail at runtime with: |
| 53 | + |
| 54 | +``` |
| 55 | +runtime error: unknown callable: "f" |
| 56 | +``` |
| 57 | + |
| 58 | +The error is deterministic — it fires every time the parallel path is taken, |
| 59 | +not intermittently. The sequential fallback path (which fires when |
| 60 | +`should_parallelize` returns false) would succeed, because the sequential |
| 61 | +`call()` method checks `self.resolved_imports` correctly. |
| 62 | + |
| 63 | +--- |
| 64 | + |
| 65 | +## Severity assessment |
| 66 | + |
| 67 | +### Is the parallel path currently reachable with imports? |
| 68 | + |
| 69 | +The `should_parallelize` threshold requires **all args to be direct `Call` |
| 70 | +nodes to user-defined functions** (`is_direct_user_call`). A user-defined |
| 71 | +function is one found in `module.symbols` — the local module's symbol table. |
| 72 | +Imported symbols are not in `module.symbols`; they are in `resolved_imports`. |
| 73 | + |
| 74 | +Therefore: `is_direct_user_call` returns `false` for a call to an imported |
| 75 | +symbol. `should_parallelize` returns `false`. The parallel path is **never |
| 76 | +taken** when any arg is a call to an imported symbol. |
| 77 | + |
| 78 | +This is the key finding. The gap is real but currently unreachable by |
| 79 | +construction: the threshold that gates parallelism also excludes the case |
| 80 | +that would expose the gap. |
| 81 | + |
| 82 | +### Severity: P3 (low) |
| 83 | + |
| 84 | +- **Not reachable today.** The threshold check prevents the parallel path |
| 85 | + from firing on imported-symbol calls. No program can currently hit this |
| 86 | + error through normal use. |
| 87 | +- **Deterministic when reachable.** If the threshold were relaxed (e.g., to |
| 88 | + support imported-symbol parallelism in v3.0), the failure would be |
| 89 | + immediate and obvious — not a data race or intermittent failure. |
| 90 | +- **Well-documented.** The source comment names the limitation and the fix |
| 91 | + path. The CHANGELOG v2.0 known-limitations section documents it. This |
| 92 | + audit was scheduled before v3.0 parallel work begins. |
| 93 | +- **No security surface.** The gap cannot be exploited to bypass effect |
| 94 | + checks or access unintended symbols — it fails closed (unknown callable |
| 95 | + error), not open. |
| 96 | + |
| 97 | +Downgrade from the initial "unknown severity" to **P3**. It is a latent |
| 98 | +gap, not an active defect. |
| 99 | + |
| 100 | +--- |
| 101 | + |
| 102 | +## Fix path |
| 103 | + |
| 104 | +When v3.0 parallel work begins and the threshold is relaxed to support |
| 105 | +imported-symbol calls in parallel branches, the fix is: |
| 106 | + |
| 107 | +```rust |
| 108 | +let mut branch_interp = Interpreter { |
| 109 | + module, |
| 110 | + max_depth, |
| 111 | + depth: current_depth, |
| 112 | + prims: build_default_registry(), |
| 113 | + trace: EffectTrace::fresh(), |
| 114 | + resolved_imports: resolved_imports.clone(), // pass parent's imports |
| 115 | +}; |
| 116 | +``` |
| 117 | + |
| 118 | +`resolved_imports` is a `HashMap<String, Definition>`. `Definition` derives |
| 119 | +`Clone`. The clone cost is proportional to the number of imports — for the |
| 120 | +current pipeline programs (3 imports), negligible. For programs with large |
| 121 | +import sets, a shared `Arc<HashMap<...>>` would be preferable to avoid |
| 122 | +repeated cloning. |
| 123 | + |
| 124 | +The `eval_parallel_exprs` method signature would need to accept |
| 125 | +`resolved_imports` as a parameter (or `Interpreter` would need to expose it |
| 126 | +as a field accessible to the closure). The `rayon::scope` closure already |
| 127 | +transmits `module` as a raw pointer; `resolved_imports` can be transmitted |
| 128 | +the same way (it is immutable during parallel evaluation). |
| 129 | + |
| 130 | +**Prerequisite:** before relaxing the threshold, add a regression test that: |
| 131 | +1. Publishes a symbol to the store |
| 132 | +2. Writes a module that imports it and calls it inside `list(...)` |
| 133 | +3. Asserts the result is correct (not `unknown callable`) |
| 134 | + |
| 135 | +This test will fail before the fix and pass after it, pinning the behavior. |
| 136 | + |
| 137 | +--- |
| 138 | + |
| 139 | +## Effect check interaction |
| 140 | + |
| 141 | +One subtlety: the transitive effect check (`check_transitive_effects`) runs |
| 142 | +on the local module only. It does not walk into imported definitions. This |
| 143 | +means a parallel branch that calls an imported effectful function would not |
| 144 | +be caught by the static check — it would be caught at runtime by the effect |
| 145 | +budget check in `call_with_vals`. This is the same behavior as the sequential |
| 146 | +path. No new risk introduced by the fix. |
| 147 | + |
| 148 | +--- |
| 149 | + |
| 150 | +## Decision |
| 151 | + |
| 152 | +**Formally accept as P3 — no fix required before v3.0 planning.** |
| 153 | + |
| 154 | +Rationale: |
| 155 | +- The gap is unreachable by construction under the current threshold. |
| 156 | +- The fix is straightforward and well-understood. |
| 157 | +- The correct time to apply the fix is when the threshold is relaxed as |
| 158 | + part of v3.0 parallel work — not before, because fixing it now would |
| 159 | + add code that is never exercised and cannot be tested. |
| 160 | +- The regression test described above should be written as part of the |
| 161 | + v3.0 parallel work spec, not now. |
| 162 | + |
| 163 | +**Action item for v3.0 spec:** include "pass `resolved_imports` to branch |
| 164 | +interpreters" and the regression test as explicit requirements when the |
| 165 | +parallel evaluator threshold is relaxed. |
| 166 | + |
| 167 | +--- |
| 168 | + |
| 169 | +## Summary |
| 170 | + |
| 171 | +| Item | Finding | |
| 172 | +|---|---| |
| 173 | +| Gap location | `eval_parallel_exprs`, branch interpreter construction | |
| 174 | +| Failure mode | `unknown callable` when imported symbol called in parallel branch | |
| 175 | +| Currently reachable | No — threshold excludes imported-symbol calls | |
| 176 | +| Severity | P3 (latent, not active) | |
| 177 | +| Fix complexity | Low — clone `resolved_imports` into branch interpreter | |
| 178 | +| Fix timing | At v3.0 threshold relaxation, not before | |
| 179 | +| Decision | Formally accepted as P3; fix deferred to v3.0 parallel work | |
| 180 | + |
0 commit comments