Skip to content

chore: post-rename polish — close the deferred items and a required-check gap - #37

Merged
sethbang merged 8 commits into
mainfrom
chore/post-rename-polish
Aug 20, 2026
Merged

chore: post-rename polish — close the deferred items and a required-check gap#37
sethbang merged 8 commits into
mainfrom
chore/post-rename-polish

Conversation

@sethbang

@sethbang sethbang commented Aug 20, 2026

Copy link
Copy Markdown
Owner

Closes the loose ends the venice-py rename surfaced and deliberately deferred. No rename work here — the repo rename (Phase 5) is separate and still deferred.

__version__ had a fallback that made its own test inert

__init__.py fell back to a hardcoded "2.2.0" inside except Exception, and 2.2.0 was the version being shipped. So test_version_matches_installed_metadata — written specifically to catch a broken metadata lookup — passed identically whether the lookup worked or raised.

Fixed structurally rather than by bumping the literal:

  • fallback is now 0.0.0+unknown, which can never equal an installed version
  • handler narrowed to except PackageNotFoundError, so unrelated failures stop being swallowed
  • new test_version_fallback_is_not_a_plausible_release greps the fallback out of the source and fails if it is ever shaped like a release

Verified by flipping the fallback back to 2.2.0 and confirming the new test fails, then restoring it — the gate is not vacuous.

The imports had to stay inside the try block (E402/I001), which is why the original was written that way.

Every Python directory in the repo is now linted

make format-check covered src/ and tests/ only.

  • tools/ was already clean, so widening lint / format / format-check was free.
  • benchmarks/ had 92 ruff findings, so it is staged across two commits — clear the findings first, extend the gate second, because doing it in one step would have turned the required validate job red.

All 92 were non-behavioural: 66 blank lines carrying whitespace, 6 missing trailing newlines, 5 unused imports, 5 unsorted import blocks, and the modern-syntax rewrites (List/Dict/Optional → builtin generics and X | None, a redundant open mode, one getattr with a constant attribute — whose hasattr guard and type: ignore are preserved).

No test covers benchmarks/, so verification was an ignore-whitespace diff review plus compiling and importing all 8 modules. examples/ was already covered by examples.yml.

configure snapshotted the config path at import

from ..config import DEFAULT_CONFIG_PATH bound the value at import, so redirecting the config location reached venice_ai.cli.config but not the configure command — the fixture had to patch both modules. The real hazard was the next test author patching only one and silently writing to the operator's live ~/.venice-py/. Now read at call time; duplicate fixture patch and its now-unused import removed.

The module alias is required — configure() binds a local config that a bare from .. import config would shadow.

Changelog

Four version headings (2.0.1, 2.0.2, 2.1.0, 2.2.0) were written as [x.y.z] links with no link definition, so they rendered as literal bracketed text. Added, along with an [Unreleased] section.

The 2.2.0 entry also described the bridge in the future tense — "a final venice-ai release follows this one… until it does, venice-ai stays at 2.1.0". Both clauses resolved when venice-ai 2.1.1 shipped, so the entry was stating something false.

One decision recorded in docs.yml

--project-name=venice-ai-docs names a live Cloudflare Pages project, not this package. It is not user-visible, and Pages projects cannot be renamed in place. Without a note saying so, a future rename sweep reads it as a stale venice-ai reference and breaks the docs deploy. The comment is the fix.

Verification

make format-check (now src/ tests/ tools/ benchmarks/), make check-all, mypy on 190 files, vulture, bandit — all clean. tests/cli/ + tests/unit/test_client.py: 1448 passed.

Full make test is the operator's to run.

🤖 Generated with Claude Code

sethbang and others added 7 commits August 20, 2026 13:56
The fallback literal had caught up to the version being shipped, so
test_version_matches_installed_metadata passed identically whether the
metadata lookup succeeded or raised — the guard written to catch a broken
lookup had become inert.

Fixed structurally rather than by bumping the literal: 0.0.0+unknown can
never equal an installed version, and the handler narrows from
`except Exception` to `except PackageNotFoundError` so unrelated failures
stop being swallowed. test_version_fallback_is_not_a_plausible_release
greps the fallback out of the source and fails if it is ever shaped like a
release, so the two can never collide again.

Verified the new test fails when the fallback is set back to "2.2.0".

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
format-check covered src/ and tests/ only, so ruff never ran over tools/.
It was already clean, so widening lint, format and format-check is free —
and format-check runs in the required validate job, which makes tools/
genuinely gated rather than formatted by hand.

benchmarks/ has 92 ruff errors and is deliberately left out: adding it
would turn a required check red.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`from ..config import DEFAULT_CONFIG_PATH` bound the value at import, so
redirecting the config location reached venice_ai.cli.config but not the
configure command, and the fixture had to patch both modules. The hazard
was the next test to patch only one and silently write to the operator's
real ~/.venice-py/.

The module alias is required: configure() binds a local `config`, which a
bare `from .. import config` would shadow.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Four version headings (2.0.1, 2.0.2, 2.1.0, 2.2.0) were written as links
with no link definition, so they rendered as literal bracketed text.

The 2.2.0 entry also described the bridge in the future tense; both of its
forward-looking clauses resolved when venice-ai 2.1.1 shipped, so the entry
was stating something false.

docs.yml now records why --project-name=venice-ai-docs stays as it is: it
names a live Cloudflare Pages project, not this package, it is not
user-visible, and Pages projects cannot be renamed in place. Without the
note, a future rename sweep reads it as a stale reference and breaks the
docs deploy.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
92 findings, none of them behavioural: 66 blank lines carrying whitespace,
6 missing trailing newlines, 5 unused imports, 5 unsorted import blocks,
and the modern-syntax rewrites (List/Dict/Optional to builtin generics and
X | None, redundant open mode, getattr with a constant attribute).

The getattr rewrite keeps the hasattr guard and the type: ignore directly
above it. Ignoring whitespace, the diff is line wrapping, import ordering
and those rewrites — nothing else.

Verified all 8 modules compile and import cleanly; no test covers this
directory, so that is the available check.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
With the 92 findings cleared, benchmarks/ can join the gate without
turning the required validate job red — the reason it was staged out of
the tools/ change rather than done alongside it.

Every Python directory in the repo is now linted and format-checked:
src/, tests/ and tools/ and benchmarks/ here, examples/ in examples.yml.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@codecov

codecov Bot commented Aug 20, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

pyright reported `"PackageNotFoundError" is possibly unbound`: the previous
commit imported it inside the try block it is caught from, so if that import
were ever the thing that failed, the except clause would raise NameError
instead of handling anything. The import moved to module scope, where E402
does not apply and the name is always bound; only the version() call stays
in the try.

The import was inside the try because moving it out at the bottom of the
module tripped E402 — putting it with the other module imports satisfies
both checks.

The reason this reached CI is the real defect: `pyright (project)` is one of
the five required checks, but no make target ran it, so `make check-all`
could pass on code a required check rejects. mypy does not substitute — it
does not report this class at all. Added `type-check-pyright` and wired it
into check-all.

Verified by reintroducing the exact bug and watching the new target fail,
then restoring.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@sethbang sethbang changed the title chore: post-rename polish — close the deferred §7b/§7c items chore: post-rename polish — close the deferred items and a required-check gap Aug 20, 2026
@sethbang

Copy link
Copy Markdown
Owner Author

Update: this PR found a gap in the local gate itself

The branch went red on pyright (project) — one of the five required checks — while make check-all, make format-check, mypy, vulture and bandit were all green locally.

Root cause: pyright appeared in no make target at all. check-all chained format, lint, type-check (mypy), check-imports, check-dead-code and security. So a full local run could pass on code a required check rejects, and the first signal was a red PR.

mypy does not stand in for it. The finding was:

error: "PackageNotFoundError" is possibly unbound (reportPossiblyUnboundVariable)

The name was imported inside the try block that its own except clause names — so if that import had ever been the failing thing, the handler would raise NameError instead of handling anything. mypy does not report that class; pyright does. It was written that way because moving the import out at the bottom of the module tripped E402; the correct fix is to put it with the other module-scope imports, where E402 does not apply and the name is always bound, leaving only the version() call in the try.

Fixed both: the binding, and the missing type-check-pyright target now wired into check-allverified by reintroducing the exact bug and watching the new target fail.

make check-all now covers all five required checks.

@sethbang
sethbang merged commit b6b747b into main Aug 20, 2026
22 checks passed
@sethbang
sethbang deleted the chore/post-rename-polish branch August 20, 2026 20:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant