refactor(tilib): narrow ti_ar fixture except to TilibParamError (#218) - #220
Merged
Conversation
Spec for backfilling missing python/<mod>/tests/fixtures/<case>_params.py so the Linux-canonical equivalence suite covers the full committed baseline corpus, not just a hand-picked subset. Scope locked across 3 Codex review passes: - 4 new fixtures: fp_jt60, ti_w, tr_m0904, wrx_jt60 - Rename ti_iter01_params.py -> ti_min_params.py (5 importers) - Register wrx_jt60 in test_run/test_definitions.conf - eq_jt60 deferred to follow-up issue (eqjt60.gs never committed) Builds on the Linux-canonical equiv policy (PR #216, 2026-05-26 spec). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…rror (#218) The fixture's _apply_array and apply functions used `except Exception:` to silence registry-rejection errors for UNREGISTERED_KEYS. Codex pre-push review on PR #219 flagged that this would also mask real bugs (bad value type, index OOB, registry returning a different ierr code). ti's C ABI raises specific subclasses (python/tilib/errors.py): - TilibParamError (ierr=1, "invalid parameter name or value") - TilibStateError (ierr=2) - TilibRunError (ierr=3) - TilibNotImplementedError (ierr=4) Registry-rejection of an unregistered name is exactly ierr=1 (TI_ERR_INVALID per ti/ti_api.h). Narrowing the except to TilibParamError preserves the silence-for-unregistered behavior while letting real bugs (state, run, type errors) propagate normally. All 4 except sites in ti_ar_params.py narrowed: - _apply_array dict branch (L75) - _apply_array list branch (L82) - apply SCALARS loop (L92) - apply MATRIX_ARRAYS loop (L102) Local test: 43 passed, 2 skipped (no regression). The same pattern applies to ti_w_params.py which is added by PR #219. That fix is deferred to a follow-up commit (or a quick PR) after #219 merges, so this PR's diff stays clean against develop. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Codex pre-push review noted that ierr=1 (TI_ERR_INVALID) covers both "invalid name" AND "invalid value" per ti/ti_api.h:27; the original "unknown name" wording was technically accurate only for the UNREGISTERED_KEYS path. Update comment to reflect both semantics while documenting the intent. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Owner
Author
|
@cursor review |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 6ca586f. Configure here.
This was referenced Jun 8, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Narrows the broad
except Exception:inti_ar_params.pyto the specificTilibParamError(ierr=1,TI_ERR_INVALID) that the ti C ABI raises when a parameter name is unregistered. Other exception classes —TilibStateError(ierr=2),TilibRunError(ierr=3),TilibNotImplementedError(ierr=4) — now propagate normally instead of being silently swallowed.Closes #218 (in part — see "Deferred" below).
Why
Codex pre-push review on PR #219 flagged that the
except Exception:pattern in the ti fixtures could mask real bugs (bad value type, library-state errors, calculation failures) — only registry-rejection (ierr=1) was meant to be silenced forUNREGISTERED_KEYS. This PR addresses the precedent file ondevelop; the same fix inti_w_params.py(added in PR #219) is deferred.Scope
python/tilib/tests/fixtures/ti_ar_params.py(4 except sites narrowed, import added, comment clarified)ti/ti_api.h:27(TI_ERR_INVALID = 1)ti/ti_param_registry.f90:45-47, 90-91, 190-191(registry returns ierr=1 for unknown name and bad value)python/tilib/errors.py:46-51(_CODE_MAP[1] = TilibParamError)Deferred
python/tilib/tests/fixtures/ti_w_params.pyhas the sameexcept Exception:pattern but only exists on PR #219's branch (feat/fixture-parity-215). Applying the same narrowing toti_w_params.pyis deferred to a small follow-up commit after #219 merges, so this PR's diff stays clean against develop. #218 will be re-resolved (or kept open with a tracking note) after that follow-up lands.ti_min_params.pyhas no try/except — no work needed there.Test plan
PYTHONPATH=python pytest python/tilib/tests/ --timeout=120 -q→ 43 passed, 2 skipped (no regression)Pre-push gate
feature-dev:code-reviewer): READY-TO-PUSH, no HIGH/MEDcodex:codex-rescue): READY-TO-PUSH, no HIGH/MED6ca586f7🤖 Generated with Claude Code
Note
Low Risk
Test-fixture-only change; behavior unchanged for expected invalid names, but real non-param errors will now fail tests loudly instead of being masked.
Overview
Replaces broad
except Exception:handlers inti_ar_params.pywithexcept TilibParamError, so only registry/invalid-parameter failures (C ABI ierr=1) are silenced forUNREGISTERED_KEYS.TilibStateError,TilibRunError, andTilibNotImplementedErrornow surface instead of being swallowed during scalar, 1D array, and 2D matrixset_paramcalls.Adds
from tilib.errors import TilibParamErrorand tightens inline comments at all four catch sites. Same pattern forti_w_params.pyis explicitly deferred to a follow-up after PR #219.Reviewed by Cursor Bugbot for commit 6ca586f. Bugbot is set up for automated code reviews on this repo. Configure here.