Harden generated-model runtime and unblock 2024 FABLE conversion - #250
Conversation
- Add static INDIRECT(ADDRESS(ROW()+-k, COLUMN()+-k)) resolution and repair the corrupted structured-reference prefix defect so the 2024 Canada FABLE workbook converts end to end (410,299 formulas, 0 untranslated, 0 output-closure blockers). - Execute the generated model scenario (10,274 outputs) and fix all 349 runtime output errors: Excel-faithful error strings from _sf_arith, _sf_compare, _sf_sum, _sf_average, _sf_ln, VALUE, and NUMBERVALUE; coercion-safe ordering comparisons; _SfRangeView resolution in _sf_index. - Keep the 1,741 static circular-dependency warnings documented as phantom whole-column SUMIFS cycles that never form runtime cycles. - Add regression tests covering the corrupted-ref repair, static INDIRECT, arithmetic/comparison coercion, SUM error propagation, and INDEX range views. Full test run: 217 passed, 1 skipped.
CI picked up ruff 0.16.3 via the unbounded ruff>=0.8 constraint; its new lint rules flag 139 pre-existing violations repo-wide. Constrain ruff to >=0.8,<0.16 so the quality job matches the version the codebase passes cleanly against.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 530ba0ec67
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| " continue", | ||
| " total += value", | ||
| " count += 1", | ||
| " return total / count", |
There was a problem hiding this comment.
When an AVERAGEIF has no matching numeric cells, count remains zero and this division raises ZeroDivisionError, aborting the entire generated calculate() call unless the formula happens to be wrapped in IFERROR. Excel returns #DIV/0! in this case; _sf_averageifs repeats the same unconditional division and needs the same guard.
Useful? React with 👍 / 👎.
| " for value in (left_value, right_value):", | ||
| " if _sf_is_error(value):", | ||
| " return value", |
There was a problem hiding this comment.
When either operand is an Excel error, _sf_compare returns the nonempty error string, but generated IF formulas use comparisons directly as Python ternary conditions at generation.py:1264. Consequently, a formula such as =IF(#N/A<1,1,2) treats '#N/A' as truthy and returns 1 instead of propagating #N/A, silently changing workbook results.
Useful? React with 👍 / 👎.
| match = _STATIC_INDIRECT_ADDRESS_RE.search(raw_formula) | ||
| if match is None: | ||
| return None |
There was a problem hiding this comment.
When one formula contains two supported INDIRECT(ADDRESS(...)) expressions, search() records only the first target in FormulaRecord.raw_references, although the parser later translates both references. Contract inference follows the graph, so the second target can be omitted from the generated constants/formulas and _get then supplies a blank, producing a silently incorrect result; collect all matches rather than returning only one.
AGENTS.md reference: AGENTS.md:L50-L53
Useful? React with 👍 / 👎.
| " if matched:", | ||
| " candidate = index + 1", | ||
| " break", |
There was a problem hiding this comment.
For MATCH(...,-1) on a descending array, the required result is the smallest value greater than or equal to the lookup value, but this branch returns after the first qualifying element. For example, lookup 7 in [10, 8, 5] returns position 1 instead of position 2; it should retain qualifying candidates until the first value below the lookup, as the match_type == 1 branch does in the opposite direction.
Useful? React with 👍 / 👎.
1 participant
Summary
Unblocks full-workbook conversion and scenario execution of the 2024 Canada FABLE Calculator workbook
(Zenodo 14755928). The pipeline (extract, graph, translate, infer-contract, generate) completes cleanly
(410,299 formulas translated, 0 untranslated, 0 output-closure blockers), and the generated model now
executes all 10,274 declared outputs without runtime errors.
Changes
INDIRECT(ADDRESS(ROW()+-k, COLUMN()+-k))to the target cellreference (used by 55 cells as a "value of the cell directly above" pattern), creating the proper
dependency edge.
name[] name[[#This Row],[Column]]during tokenization while preserving raw-formula provenance.generation.py:_sf_arithcoerces numeric strings, returns#DIV/0!/#NUM!/#VALUE!instead of raising; ordering comparisons route through a newcoercion-safe
_sf_compare;VALUE/NUMBERVALUEreturn#VALUE!,LNreturns#NUM!;_sf_sum/_sf_averagepropagate errors and ignore text cells._sf_indexreconstructs rows from_SfRangeViewinstead of treating therange as a scalar.
static_circular_dependencywarnings are phantomwhole-column SUMIFS cycles (year-offset criteria exclude the self-cell); lazy sum-range evaluation
means no runtime cycles. Kept as warnings and documented.
Validation
tolerance), no systematic defects.
git diff --checkclean.