fix(evaluator): report skipped and unresolved scenario-table calculations

[arcaputo3]

An INDIRECT-based sensitivity table could seed 1 / 1 / 1 instead of 11 / 21 / 31 without warnings. Source-evaluation failures also discarded unresolved-cone diagnostics and could report zero warnings after seeding no interiors.

Tables with dynamic references in their needed source cone are now left untouched with a named Skipped warning. Substituted input cells and their original ancestors are excluded from that cone. Both seeding paths retain source/member failures and unresolved precedents, count unseeded interiors, and feed those warnings into --strict. Dynamic scenario evaluation remains deferred; ordinary INDIRECT/OFFSET evaluation remains supported.

Validation: 11 new cases, existing Excel-authored table oracles, and the full suite passed (5,601 cases, one existing ignored performance comparison). Native CLI probes confirm explicit skips and strict exit 1.

Closes #498.
Closes #506.

[claude]

Review (1/3): summary, plus the one thing I would fix before merge

Read the full diff plus the surrounding DataTableSeeder, DependencyGraph.dynamicCells, and the CLI warning-rendering path. Overall a good change that fixes a real signal-loss bug: threading XLResult[CellValue] through CellOutcome instead of collapsing with .toOption means a failing source formula now (a) gets counted and (b) keeps its unresolved cone set, which the old None short-circuit silently discarded. The ScenarioTableIntegritySpec case "source failure preserves unresolved cone diagnostics across combinations" pins exactly that, and the Either-first refactor of computeCellIterative’s member fold reads better than the Option-of-Vector[Sheet] it replaces.

Note: ./mill is not runnable in this sandbox, so all of this comes from reading the code — I did not execute the suite. Please confirm the four PR gates ran locally.

1. The ConeUnresolved CLI line now contradicts the Skipped line beside it

SeedTableWarning.ConeUnresolved’s scaladoc was correctly updated here ("source failures can also leave interiors unseeded, counted separately by Skipped"), but the user-facing rendering was not — WriteCommands.scala:1291 still reads ": seeded, but $cells precedent cell(s) ...".

The new GH-506: source failure preserves unresolved cone diagnostics scenario emits both warnings for the same table, so recalc --tables will now print:

WARNING: data table F10:F12 on S: 2 interior cell(s) left unseeded — <reason>
WARNING: data table F10:F12 on S: seeded, but 1 precedent cell(s) in the what-if cone could not be re-derived (S!B1) — ...

"seeded, but ..." is now false in exactly the case this PR introduces. Suggest dropping the claim (e.g. "$cells precedent cell(s) in the what-if cone could not be re-derived (...) — any seeded interior may not reflect the substitution") and adding a CLI-level test asserting the two lines are mutually consistent. That pairing is the new observable behavior and nothing currently guards its wording.

2. Skipped.reason attributes one cell’s error message to a mixed population

case None => (acc, skips + 1, fired, named, unres, firstFailure)   // axis overlay failed: no message
case Some(outcome) => ... firstFailure.orElse(Some(error.message.take(200)))

skipped merges two distinct causes (axis-overlay failure vs. source-formula failure) but the reason renders as one message. If cell 1 fails on the axis and cell 2 on the source, the output reads "2 interior cell(s) left unseeded — Formula error in AND(A1:A1): ...", implying both failed that way. Fixes in increasing order of effort:

• prefix the reason (s"first failure: ...") so it does not read as a universal claim;
• capture a message for the None branch too — the axis evaluation already returns an XLResult, and .toOption inside computeCell is what throws it away, so it is recoverable;
• keep two counts and emit two Skippeds.

Two smaller things in the same expression: take(200) is a third magic truncation width alongside the existing take(120)s and truncates with no ellipsis marker, and reason is now free-form upstream text embedded in a channel documented as "one summary line per seeding warning" — worth normalizing newlines out of it.

[claude]

Review (2/3): the dynamic gate, and performance

3. The dynamic gate is a deliberate capability regression — is the blast radius as narrow as it can be?

ctx.dynamic.intersect(closure) -- inputQ skips the whole table when any precedent in the closure is dynamic. The boundary work is genuinely nice: ctx.deps -- inputQ treats the overlaid inputs as cut points, so an input whose own formula is INDIRECT(...) no longer blocks seeding, and the two tests ("an input formula replaced by the axis overlay", "ancestors of an overlaid input") pin that precisely. Also good: dynamicCells(wb) already resolves defined-name chains (GH-520), so Dynamic+1 is caught, and that is tested.

But a table that previously seeded correctly — corner =INDIRECT("$B$1")+1 where B1 is a cached static cell — is now skipped, and under --strict that becomes a non-zero exit. That is the right conservative default (the graph’s missing edge is real unsoundness) and it is documented in LIMITATIONS.md, but two refinements are worth considering:

• Scope the gate to the axis-dependent cone rather than the whole closure. A dynamic precedent that lands in cone.base (input-independent, uncached) is resolved once and never has to move under the substitution; only beforeCycle/afterCycle cells need trustworthy edges. That is a reordering (compute whatIfCone before the gate), not new machinery. Counter-argument, which may win: because dynamic deps are under-approximated, a cell classified input-independent might in fact be axis-dependent through its INDIRECT string, so the conservative read is defensible. Worth a sentence in the scaladoc either way, so the next reader knows the choice was deliberate.
• A dynamic precedent now masks CircularNotIterated. The gate runs before lane selection, so a table that is both cyclic and dynamic reports only the dynamic reason and the user never learns about the cycle — and the cycle is the more actionable of the two.

4. Performance: an extra whole-workbook pass on every recalc --tables

CycleContext now eagerly computes DependencyGraph.dynamicCells(wb). dynamicFunctions is never empty, so the candidateTokens.isEmpty short-circuit never fires and the workbook variant always walks every formula cell in the book, allocating an uppercased copy of each formula string, plus a sheets x definedNames name-resolution pass. Two mitigations are already in place: it sits behind lazy val cycles (forced only once a group survives the inputs guard) and is computed once per run, and recalc already pays a comparable pass in SheetEvaluator. So this is roughly one more pass of an order we already pay, not a new complexity class.

Still, the gate only ever consumes the intersection of closure and dynamic, and closure is tiny relative to the book. Classifying just the closure’s cells on demand (closure.exists(isDynamic), memoized across groups) would make the cost proportional to what is asked rather than to workbook size — a meaningful win on the 100k+ formula models this library targets.

5. Readability: the fold accumulators have outgrown tuples

Both lanes went from 6-tuples to 7-tuples, destructured positionally inside a nested match:

) { case ((acc, fails, skips, fired, named, unres, firstFailure), cellRef) =>

Two positional Int fields (fails, skips) now sit adjacent with no type-level distinction — a transposition compiles cleanly and yields a wrong warning count. A private final case class SeedAcc(sheet: Sheet, unconverged: Int, skipped: Int, guarded: Int, guardName: Option[String], unresolved: Set[QualifiedRef], firstFailure: Option[String]) with .copy updates would make each branch self-documenting, and it is squarely in the house style for a file this carefully written.

Nit in the same block: Option.when(...).toList.toVector round-trips through List and leans on option2Iterable; Vector.from(Option.when(...)) says it directly.

[claude]

Review (3/3): docs, process, test coverage

6. Docs and process

• The object scaladoc does not mention the new gate. For a file that documents MaxInteriorArea down to the cell count and enumerates "Such tables gate three ways", a brand-new skip category (Data-table seeder: INDIRECT-mediated intermediates are invisible to the what-if cone (silently FLAT) #498) belongs in the object DataTableSeeder scaladoc, not only in the enum case comment. Relatedly, the bullet "a Left (unsupported function, unresolvable axis) leaves that cell untouched and continues" is now incomplete — it also counts and warns, which is the whole point of this PR.
• The CLI note still reads as unqualified success. "Data table cache evaluation completed" is more honest than "Seeded data table interior caches", but it is also vaguer for the clean case and still reads as all-good immediately above a stack of WARNING lines. warnings is in scope at that line — consider if warnings.isEmpty then "Seeded data table interior caches" else s"Data table cache evaluation completed with ${warnings.size} warning(s)", which keeps the specific message for clean runs and stops over-claiming on dirty ones.
• PR body is the unedited template: no description, no checked gates, and no Closes #498, closes #506. Per .claude/rules/workflow.md the closing keywords belong in the PR body, one keyword per issue.
• Test counts not updated: this adds 9 specs plus a CLI test, while CLAUDE.md:409 and docs/STATUS.md:212 still say 5,455. Reasonable to defer to wave integration since this targets wave-26-cache-invalidation — flagging so it is not lost.

7. Test coverage

The new spec is well-targeted: it pins the untouched-workbook invariant with assertEquals(report.workbook, wb) (stronger than checking individual cells), separates "an Excel error value is a successful interior" from a host failure, and covers dynamic-via-intermediate, dynamic-via-defined-name, and dynamic-but-unrelated. Gaps I would add:

• A 2D table (dt2D = true, both r1/r2). Every new test is 1-D, so the inputQ = Set(input1) ++ input2 boundary and the two-axis overlay path are untested for both the dynamic gate and the new skip counting.
• The Skipped/ConeUnresolved pairing rendered through the CLI (finding 1).
• A dynamic dependent — a static-edge reader downstream of the source — to pin that the gate is precedent-only. The Z1 = INDIRECT("B1") case tests "unrelated" rather than "downstream", since INDIRECT contributes no static edge and Z1 never enters the closure at all.
• The existing DataTableSeederSpec uses exact case Vector(SeedTableWarning.Skipped(...)) matches in several places, which is a useful implicit guard that this PR does not emit extra warnings on the old paths — worth confirming those still pass.

Things I checked that look correct

• (interior.width.toLong * interior.height.toLong).toInt in the new dynamic-skip warning is safe: the oversized guard above already caps the interior at MaxInteriorArea (1,000,000), and it matches the existing budget-warning idiom.
• Narrowing closure via ctx.deps -- inputQ can only shrink relevantCore, and only along paths that run exclusively through an overlaid input — which the axis overlay replaces anyway. whatIfCone already removed inputQ from precedents, so the change additionally drops uncached ancestors of the input from cone.base, which were being needlessly re-derived before. That reads as a small correctness-plus-perf win.
• GH-493: an unresolvable cone reports the CONE, never left unseeded still holds under the new counting: B1 stays on its loaded cache, B1+1 evaluates to a Right, so no Skipped is produced and only ConeUnresolved fires.
• No new var/null/.get; the Option to Either migration moves in the direction the purity charter wants.

Nothing here looks like a blocker. Finding 1 is the one I would fix before merge (a two-line change to a message that is now provably wrong); findings 2 and 4 deserve an explicit decision rather than a silent deferral.