fix(ci): green builds — real bugs, not guards - #61
Merged
Conversation
Every check on every PR has been red at the repo level. Four causes, four fixes: - python lint (ruff): fixed all 22 violations instead of absorbing them with continue-on-error (which never worked anyway — the job still reports failure). Includes genuine F821 bugs: wer/der never imported in util/utils.py (both mirrors), process_losses called without self in hebrew/trainer.py, yaml never imported in hebrew's ONNX converter, name_of never defined in nakdimon_hebrew_model.py (now a unicodedata-based helper). Also the duplicate untyped make_src_mask, bare excepts narrowed to KeyError/Exception, one isinstance fix, import sorting. - python infer/train: pip install -e . failed at metadata — packages=['rababa'] was commented out, so auto-discovery tripped on the flat layout (models/config/modules/log_dir). Declared the namespace packages explicitly (util*, modules*); editable install verified locally. - ruby build: standardrb — super-arguments, empty method-body lines, redundant line continuation, gemspec spacing. rake: 23 examples, 0 failures (unchanged). - codeql: the advanced workflow conflicts with the repo's configured default setup (SARIF rejected) — default setup already covers ruby/python/actions, so the redundant workflow is removed.
…rotobuf for train CI's ruff (latest) collapses the short find_namespace_packages call that older local ruff accepted multi-line. train (3.9) hit the tensorboard-vs-protobuf>=4 'Descriptors cannot be created directly' clash — PROTOCOL_BUFFERS_PYTHON_IMPLEMENTATION=python on the train job avoids it without pinning protobuf in packaging.
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.
Every check on every recent PR has been red at the repo level (PRs
merge anyway). This makes CI actually green by fixing causes, not by
adding guards.
Four causes, four fixes
python lint (
ruff check+ruff format --check)The job carried
continue-on-error: truefor "36 pre-existingviolations" — which never absorbed the failure (the job reported red
anyway). Fixed all 22 remaining violations, removed the guard.
Several were genuine bugs, not style:
wer/derused but never imported inutil/utils.py(arabic +hebrew mirrors) —
calculate_error_rateswould NameError if calledprocess_losses(...)withoutselfin hebrewtrainer.pyyamlnever imported in hebrewconvert_torch_model_to_onnx.pyname_ofnever defined innakdimon_hebrew_model.py(assertdiagnostics) — now
unicodedata.name-basedmake_src_mask(kept the typed one)except:narrowed toKeyError(loss-type lookup) /Exception(model-load recovery)type(x) == int→isinstance, import sorting, formattingpython infer/train (
pip install -e .at metadata)packages=['rababa']was commented out, so modern setuptoolsauto-discovery hit "Multiple top-level packages discovered in a
flat-layout: ['models', 'config', 'modules', 'log_dir']" and the
install died before any step ran. Both setup.py files now declare
find_namespace_packages(include=["util*", "modules*"])— theabsolute-import style the code uses. Verified locally: editable
install succeeds and
import util.text_encodersresolves.ruby build (
bundle exec standardrb)Five offenses: super-with-identical-arguments, empty lines at method
body starts, redundant line continuation, gemspec alignment spacing.
rakeunchanged: 23 examples, 0 failures, 9 pending.codeql (
analyze (ruby))The advanced workflow's SARIF is rejected because the repo also has
CodeQL default setup configured (ruby/python/actions — verified
via the code-scanning API). The default-setup jobs pass; the advanced
one can only fail. Removed the redundant advanced workflow; default
setup continues to cover all three languages.
Local verification
ruff check python/+ruff format --check python/— cleanbundle exec standardrb— clean;bundle exec rake— 0 failures