Skip to content

ci(tensilelite): enforce only relative tensilelite imports (AIHPBLAS-3989) - #13932

Merged
talumbau merged 1 commit into
developfrom
users/alvasile/namespace-split
Nov 5, 2026
Merged

talumbau merged 1 commit into
developfrom
users/alvasile/namespace-split

Conversation

@Alex-Vasile

@Alex-Vasile Alex-Vasile commented Oct 19, 2026 •

Copy link
Copy Markdown
Contributor

JIRA ID : AIHPBLAS-3989

Motivation

#12162 removes the Tensile compatibility package, so a new import of the old name now fails at import time. The absolute-import pre-commit hook added in #13278 still carries the Tensile package root, a bridge exemption for Tensile/__init__.py, and Tensile as a banned name. All of that is dead after #12162. The rule that remains useful is that tensilelite modules use package-relative imports.

Technical Details

  • check_no_absolute_imports.py now checks only projects/hipblaslt/tensilelite/tensilelite. The Tensile package root, the TEMPORARY_EXEMPTIONS bridge exception and the Tensile banned name are removed, and the docstring and failure message are updated.
  • The tensilelite-no-absolute-imports pre-commit entry matches only the tensilelite package and its Tests/ exclusion, and its stale "move in flight" comment is removed.
  • The checker's unit tests use tensilelite as the banned example. The bridge-exemption and missing-package-root tests are removed. An added test records that Tensile imports are no longer reported by this hook.
  • Behavior is unchanged for tensilelite: only lines added by a change are checked, and Tests/ is excluded.

Test Plan

Run the checker's unit tests, which exercise staged and PR-range changes, the package root, test-directory exclusion, existing imports and unparseable files. Run the repository's pre-commit checks in PR CI. GPU testing is unnecessary because this change only examines Python source.

Test Result

  • Checker unit tests: 29 passed (run with --noconftest; at this point in the stack the package conftest still fails on stale Tensile imports inherited from develop).
  • Pre-commit hook run against the full tree and PR CI: not run locally.

Submission Checklist

Risk level

Low (1/5): only the pre-commit hook and its tests change. It does not change packaged code or GPU behavior.

@therock-pr-bot

therock-pr-bot Bot commented Oct 20, 2026 •

Copy link
Copy Markdown

✅ All Checks Passed — Ready for Review

Check Status Details
📝 PR Description ✅ Pass —
⛔ Forbidden Files ✅ Pass —
🧪 Unit Test ✅ Pass —
🔎 pre-commit ✅ Pass —
🚫 Draft PR 🔜 To Be Enabled —
🚩 Feature Flag 🔜 To Be Enabled —
📊 Code Coverage 🔜 To Be Enabled —
🤖 therock-pr-bot ✅ Pass —

🎉 All checks passed! This PR is ready for review.

📖 Need help? See the Policy FAQ for details on every check and how to fix failures.

🙋 Wish to Override Policy?

@therock-pr-bot

Copy link
Copy Markdown

🎉 All checks passed! This PR is ready for review.

@Alex-Vasile
Alex-Vasile force-pushed the users/alvasile/namespace-split branch from 23238a5 to ae50529 Compare October 21, 2026 13:07
@talumbau
talumbau force-pushed the users/alvasile/namespace-split branch from ae50529 to 0dd56b9 Compare October 21, 2026 17:13
@Alex-Vasile Alex-Vasile added ci:skip Skip all CI builds/tests for this PR NoCI Don't run CI labels Oct 21, 2026
@Alex-Vasile
Alex-Vasile force-pushed the users/alvasile/namespace-split branch from 0dd56b9 to a2c95e0 Compare October 22, 2026 14:12
@Alex-Vasile
Alex-Vasile force-pushed the users/alvasile/namespace-split branch from a2c95e0 to 2cd157f Compare October 22, 2026 15:35
@Alex-Vasile
Alex-Vasile removed this pull request from stack #13933 October 22, 2026 15:35
@Alex-Vasile
Alex-Vasile added this pull request to stack #14201 October 22, 2026 15:36
@talumbau
talumbau force-pushed the users/alvasile/namespace-split branch from 2cd157f to d71943f Compare October 23, 2026 11:02
@Alex-Vasile
Alex-Vasile force-pushed the users/alvasile/namespace-split branch 2 times, most recently from 3694d29 to 94f88fc Compare October 24, 2026 17:49
@Alex-Vasile
Alex-Vasile force-pushed the users/alvasile/namespace-split branch from 94f88fc to 584d893 Compare October 26, 2026 14:40
@Alex-Vasile Alex-Vasile removed NoCI Don't run CI ci:skip Skip all CI builds/tests for this PR labels Oct 26, 2026
@Alex-Vasile
Alex-Vasile force-pushed the users/alvasile/namespace-split branch from 584d893 to 123eef0 Compare October 27, 2026 13:52
@codecov

codecov Bot commented Oct 27, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop   #13932      +/-   ##
===========================================
- Coverage    77.72%   77.71%   -0.01%     
===========================================
  Files         2963     2904      -59     
  Lines       513494   491013   -22481     
  Branches     79567    74424    -5143     
===========================================
- Hits        399071   381547   -17524     
+ Misses       86278    84470    -1808     
+ Partials     28145    24996    -3149     
Flag Coverage Δ *Carryforward flag
TensileLite-CPP 53.94% <ø> (ø)
TensileLite-Unit 84.76% <ø> (+<0.01%) ⬆️
hipBLAS 90.62% <ø> (ø) Carriedforward from 736a02f
hipBLASLt 70.05% <ø> (ø)
hipCUB 82.68% <ø> (+3.51%) ⬆️ Carriedforward from 736a02f
hipDNN 87.25% <ø> (ø) Carriedforward from 736a02f
hipFFT 42.57% <ø> (ø) Carriedforward from 736a02f
hipRAND 76.12% <ø> (ø) Carriedforward from 736a02f
hipSOLVER 69.67% <ø> (ø) Carriedforward from 736a02f
hipSPARSE 87.03% <ø> (ø) Carriedforward from 736a02f
rocBLAS 69.20% <ø> (ø) Carriedforward from 736a02f
rocFFT 46.49% <ø> (ø) Carriedforward from 736a02f
rocRAND 57.32% <ø> (ø) Carriedforward from 736a02f
rocSOLVER 76.65% <ø> (ø) Carriedforward from 736a02f
rocSPARSE 83.54% <ø> (ø) Carriedforward from 736a02f
rocSPARSE-integration ?
rocSPARSE-unit ?
rocThrust 93.15% <ø> (ø) Carriedforward from 736a02f

*This pull request uses carry forward flags. Click here to find out more.
see 146 files with indirect coverage changes

🚀 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.

@talumbau
talumbau force-pushed the users/alvasile/namespace-split branch from 123eef0 to e9b1e5a Compare October 27, 2026 18:10
@Alex-Vasile
Alex-Vasile force-pushed the users/alvasile/namespace-split branch 2 times, most recently from 5160310 to fa330f7 Compare November 2, 2026 21:23
@Alex-Vasile
Alex-Vasile force-pushed the users/alvasile/namespace-split branch from fa330f7 to de9628c Compare November 2, 2026 22:07
@Alex-Vasile
Alex-Vasile marked this pull request as ready for review November 2, 2026 22:13
@Alex-Vasile
Alex-Vasile requested a review from a team as a code owner November 2, 2026 22:13
@Alex-Vasile
Alex-Vasile force-pushed the users/alvasile/namespace-split branch from de9628c to 98968a8 Compare November 3, 2026 05:47
@Alex-Vasile
Alex-Vasile force-pushed the users/alvasile/namespace-split branch from 98968a8 to 7819544 Compare November 3, 2026 14:59
@Alex-Vasile
Alex-Vasile force-pushed the users/alvasile/namespace-split branch from 7819544 to 67f3a20 Compare November 4, 2026 16:48
Base automatically changed from users/alvasile/AIHPBLAS-3989-namespace-split/16-remove-compat-bridge to develop November 4, 2026 17:03
@talumbau
talumbau requested review from a team as code owners November 4, 2026 17:03
@talumbau
talumbau force-pushed the users/alvasile/namespace-split branch from 67f3a20 to 9eb0234 Compare November 4, 2026 17:03
@Alex-Vasile
Alex-Vasile removed this pull request from stack #14201 November 4, 2026 17:38
@Alex-Vasile
Alex-Vasile added this pull request to stack #15078 November 4, 2026 17:39
@Alex-Vasile
Alex-Vasile enabled stacked auto-merge (squash) November 4, 2026 19:18
The Tensile compatibility package is gone, so a new import of the old name
fails at import time. Drop the Tensile package root, the bridge exemption
and the Tensile banned name from the absolute-import hook, keeping the
rule that tensilelite modules use package-relative imports.
@Alex-Vasile
Alex-Vasile force-pushed the users/alvasile/namespace-split branch from 9eb0234 to ec65098 Compare November 4, 2026 20:59
@peymanr

peymanr commented Nov 4, 2026

Copy link
Copy Markdown
Contributor

PR Pundit review

(Testing PR Pundit Code Review agent. Not all comments may be relevant. This is a one-time automated pass: please address what is useful, no need to request a re-review. Please reply to each point, whether it applies or not: we harvest these replies, and both kinds are enormously helpful for tuning the agent. Reach out to @peymanr for suggestions.)

# Finding Where
1 🟠 Pre-commit hook silently passes legacy Tensile absolute imports: this PR unbanned Tensile projects/hipblaslt/tensilelite/scripts/check_no_absolute_imports.py:42

1. 🟠 Should fix: Pre-commit hook silently passes legacy Tensile absolute imports: this PR unbanned Tensile

projects/hipblaslt/tensilelite/scripts/check_no_absolute_imports.py:42

This PR narrowed BANNED_TOP_LEVEL_NAMES to {"tensilelite"}, so a newly added import Tensile or from Tensile.Common import x no longer fails the pre-commit hook; it is only caught later by test_production_modules_do_not_import_the_legacy_tensile_package in unit CI. The in-tree Tensile package was already removed (#12162), and Tests/unit/source_only/test_canonical_imports.py still rejects absolute Tensile imports, so the narrower scope is deliberate but loses early pre-commit feedback. The test's test_allowed_forms_are_ignored now lists these legacy statements without any comment, so a reader may misread them as permitted. Restore Tensile to the hook or add a comment noting enforcement moved to test_canonical_imports.py.

Who is affected: Developers who add a stale Tensile import locally; they learn of it from the pytest/CI run instead of from pre-commit.

Checked against the code by an independent pass.

Related past reviews: #6946 on projects/hipblaslt/tensilelite/Tensile/Tests/unit/test_PlaceholderMergeRegression.py, #12161 on projects/hipblaslt/utilities/geko/tests/config_generator/test_cluster_and_postproc_unit.py

Shared from a pilot of PR Pundit, an agentic code reviewer that builds and tests the change and reads the project's past reviews. Reviewed at ec65098c; findings checked and posted by @peymanr; points already raised on this PR are left out.

@talumbau
talumbau disabled stacked auto-merge November 5, 2026 16:13
@talumbau
talumbau disabled stacked auto-merge November 5, 2026 16:14
@Alex-Vasile

Copy link
Copy Markdown
Contributor Author

PR Pundit review

(Testing PR Pundit Code Review agent. Not all comments may be relevant. This is a one-time automated pass: please address what is useful, no need to request a re-review. Please reply to each point, whether it applies or not: we harvest these replies, and both kinds are enormously helpful for tuning the agent. Reach out to @peymanr for suggestions.)

Finding Where

1 🟠 Pre-commit hook silently passes legacy Tensile absolute imports: this PR unbanned Tensile projects/hipblaslt/tensilelite/scripts/check_no_absolute_imports.py:42

1. 🟠 Should fix: Pre-commit hook silently passes legacy Tensile absolute imports: this PR unbanned Tensile

projects/hipblaslt/tensilelite/scripts/check_no_absolute_imports.py:42

This PR narrowed BANNED_TOP_LEVEL_NAMES to {"tensilelite"}, so a newly added import Tensile or from Tensile.Common import x no longer fails the pre-commit hook; it is only caught later by test_production_modules_do_not_import_the_legacy_tensile_package in unit CI. The in-tree Tensile package was already removed (#12162), and Tests/unit/source_only/test_canonical_imports.py still rejects absolute Tensile imports, so the narrower scope is deliberate but loses early pre-commit feedback. The test's test_allowed_forms_are_ignored now lists these legacy statements without any comment, so a reader may misread them as permitted. Restore Tensile to the hook or add a comment noting enforcement moved to test_canonical_imports.py.

Who is affected: Developers who add a stale Tensile import locally; they learn of it from the pytest/CI run instead of from pre-commit.

Checked against the code by an independent pass.

Related past reviews: #6946 on projects/hipblaslt/tensilelite/Tensile/Tests/unit/test_PlaceholderMergeRegression.py, #12161 on projects/hipblaslt/utilities/geko/tests/config_generator/test_cluster_and_postproc_unit.py

Shared from a pilot of PR Pundit, an agentic code reviewer that builds and tests the change and reads the project's past reviews. Reviewed at ec65098c; findings checked and posted by @peymanr; points already raised on this PR are left out.

The legacy Tensile imports will fail on their own now that #12162 has merged (this PR was stacked ontop of it).

@Alex-Vasile
Alex-Vasile disabled stacked auto-merge November 5, 2026 16:15
@talumbau
talumbau merged commit ecd684f into develop Nov 5, 2026
464 of 477 checks passed
@talumbau
talumbau deleted the users/alvasile/namespace-split branch November 5, 2026 16:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants