Skip to content

fix(versioning): narrow nullable uuid/description reads flagged by mypy in versioning tests - #44424

Open
kokhlo wants to merge 1 commit into
apache:masterfrom
kokhlo:fix/versioning-test-mypy-uuid-narrowing
Open

kokhlo wants to merge 1 commit into
apache:masterfrom
kokhlo:fix/versioning-test-mypy-uuid-narrowing

Conversation

@kokhlo

@kokhlo kokhlo commented Sep 18, 2026

Copy link
Copy Markdown
Contributor

Summary

Fixes the 23 mypy UUID | None errors the nightly pre-commit (next) sweep flagged in the four versioning test files (#44423). UUIDMixin.uuid (superset/models/helpers.py:751) is a nullable sa.Column, so mypy infers UUID | None for instance reads; the tests passed it (and BaseDatasource.description, str | None) unnarrowed into strictly-typed callees.

Change is test-only: hoist the read into an annotated local and assert <local> is not None before the flagged call, matching the idiom from #44394/#42654. Where a narrowed local already existed (chart_uuid, dataset_uuid, slc_uuid), the assert lands right after its assignment and downstream call sites use the local; restore_fails_closed_tests.py:375 narrows the removesuffix receiver with (dataset.description or "") since the target-version suffix is expected on a present description.

Behavioral analysis

No runtime behavior change intended; every touched entity (chart/slice/dashboard/dataset fixtures) mints its uuid via default=uuid.uuid4 on insert and is committed before the asserted read, so the asserts are statically-load-bearing but unreachable in practice.

Test plan

  • mypy --check-untyped-defs (1.15.0, the pinned pre-commit rev) clean on the 4 files
  • python -c "import ast; ast.parse(...)" on all 4 files
  • CI green on this PR

Additional context

Closes #44423. Same defect class as #44384/#44394; surfaced now because the next mypy leg only runs on push/schedule (first --all-files sweep since #44258/#44251/#44091 merged).

@bito-code-review

bito-code-review Bot commented Sep 18, 2026

Copy link
Copy Markdown
Contributor

Code Review Agent Run #6ed1df

Actionable Suggestions - 0
Additional Suggestions - 2
  • tests/integration_tests/versioning/conditional_token_lock_tests.py - 1
    • Repeated uuid narrowing idiom · Line 153-154
      The two-line narrowing idiom `chart_uuid: UUID = chart.uuid` + `assert chart_uuid is not None` is now repeated in all four tests (`153-154`, `185-186`, and pre-existing `244-246`/`309-311`). Centralizing it — e.g. returning the narrowed uuid from `_versioned_chart` — removes four copies of the same guard and keeps future tests from re-stating it.
  • tests/integration_tests/versioning/creation_row_tests.py - 1
    • duplicated uuid guard · Line 145-147
      The `X_uuid: UUID = X.uuid; assert X_uuid is not None` guard is duplicated verbatim at 8 sites (145-147, 171-174, 197-200, 227-229, 255-258, 275-277, 304-307, 317-320). A single `_require_uuid(entity)` helper would centralize the guard and the `get_activity` call, removing the divergence risk if the guard ever changes.
Review Details
  • Files reviewed - 4 · Commit Range: 26f8fb5..26f8fb5
    • tests/integration_tests/versioning/activity_write_after_read_tests.py
    • tests/integration_tests/versioning/conditional_token_lock_tests.py
    • tests/integration_tests/versioning/creation_row_tests.py
    • tests/integration_tests/versioning/restore_fails_closed_tests.py
  • Files skipped - 0
  • Tools
    • MyPy (Static Code Analysis) - ✔︎ Successful
    • Astral Ruff (Static Code Analysis) - ✔︎ Successful
    • Whispers (Secret Scanner) - ✔︎ Successful
    • Detect-secrets (Secret Scanner) - ✔︎ Successful

Bito Usage Guide

Commands

Type the following command in the pull request comment and save the comment.

  • /review - Manually triggers an incremental AI Review.

  • /review full - Manually triggers a full AI Review.

  • /pause - Pauses automatic reviews on this pull request.

  • /resume - Resumes automatic reviews.

  • /resolve - Marks all Bito-posted review comments as resolved.

  • /abort - Cancels all in-progress reviews.

Refer to the documentation for additional commands.

Configuration

This repository uses Superset You can customize the agent settings here or contact your Bito workspace admin at evan@preset.io.

Documentation & Help

AI Code Review powered by Bito Logo

@codecov

codecov Bot commented Sep 18, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 80.56%. Comparing base (bd786e2) to head (26f8fb5).

Additional details and impacted files
@@            Coverage Diff             @@
##           master   #44424      +/-   ##
==========================================
- Coverage   80.57%   80.56%   -0.01%     
==========================================
  Files        2940     2940              
  Lines      175316   175316              
  Branches    40696    40696              
==========================================
- Hits       141253   141251       -2     
- Misses      31398    31400       +2     
  Partials     2665     2665              
Flag Coverage Δ
hive 37.25% <ø> (ø)
mysql 56.50% <ø> (ø)
postgres 56.52% <ø> (-0.01%) ⬇️
presto 39.14% <ø> (ø)
python 84.87% <ø> (-0.01%) ⬇️
sqlite 56.23% <ø> (ø)
unit 76.61% <ø> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

nightly pre-commit (next) mypy fails: UUID | None passed unnarrowed in 4 versioning test files (23 errors)

1 participant