Skip to content

ci: Fix model change detection script and registry parsing logic - #169

Merged
tadani3 merged 9 commits into
mainfrom
tommasoadani/detection_script_fixing
Apr 15, 2026
Merged

ci: Fix model change detection script and registry parsing logic#169
tadani3 merged 9 commits into
mainfrom
tommasoadani/detection_script_fixing

Conversation

@tadani3

@tadani3 tadani3 commented Apr 14, 2026

Copy link
Copy Markdown
Contributor

This pull request significantly improves the logic for detecting which models are affected by changes in the codebase, particularly for changes to component and task files. Instead of always triggering a full rebuild/test (run_all) when these files change, the script now traces their impact via the import graph to determine exactly which models are affected. The registry parsing is also enhanced to support a new declarative registration pattern. Comprehensive tests have been added and updated to validate these behaviors.

Improvements to affected model detection:

  • Component and task files (under src/mobius/components/ and src/mobius/tasks/) are now classified as "traceable" instead of "shared_infra", and their impact is determined by tracing the import graph to find affected models, rather than always triggering run_all. (scripts/detect_affected_models.py, scripts/detect_affected_models_test.py) [1] [2] [3] [4] [5] [6]
  • If a traceable file is deleted, the script conservatively triggers run_all. (scripts/detect_affected_models.py, scripts/detect_affected_models_test.py) [1] [2]

Enhancements to registry parsing:

  • The registry mapping logic now supports a fourth pattern: declarative dictionary-based registration (e.g., _REGISTRATIONS = {"name": ModelRegistration(ClassName, ...)}), ensuring all registered models are correctly discovered. (scripts/detect_affected_models.py) [1] [2] [3]

Test coverage and validation:

  • Tests have been updated and expanded to cover the new traceable logic, including integration tests for the import graph tracing and various edge cases (e.g., combined changes, subset/superset relationships, deleted files). (scripts/detect_affected_models_test.py) [1] [2]
  • Test assertions have been fixed and clarified, including registry expectations and affected model checks. (scripts/detect_affected_models_test.py)

These changes make the affected model detection more accurate and efficient, reducing unnecessary full runs and providing better granularity for CI/CD workflows.

@tadani3 tadani3 self-assigned this Apr 14, 2026
@tadani3
tadani3 requested review from a team and Copilot April 14, 2026 22:42
@github-actions

github-actions Bot commented Apr 14, 2026

Copy link
Copy Markdown

Performance Comparison

Comparing 3777c1855e25cb

Model Metric Baseline Current Delta
bert (feature-extraction) model_size_bytes 359 KB 359 KB +0.0%
bert (feature-extraction) num_nodes 61 61 +0.0%
falcon model_size_bytes 364 KB 364 KB +0.0%
falcon num_nodes 66 66 +0.0%
gemma2 model_size_bytes 428 KB 428 KB +0.0%
gemma2 num_nodes 107 107 +0.0%
gpt2 model_size_bytes 388 KB 388 KB +0.0%
gpt2 num_nodes 53 53 +0.0%
llama model_size_bytes 425 KB 425 KB +0.0%
llama num_nodes 61 61 +0.0%
llama (static-cache) model_size_bytes 425 KB 425 KB +0.0%
llama (static-cache) num_nodes 58 58 +0.0%
mamba (ssm-text-generation) model_size_bytes 360 KB 360 KB +0.0%
mamba (ssm-text-generation) num_nodes 98 98 +0.0%
phi3 model_size_bytes 421 KB 421 KB +0.0%
phi3 num_nodes 59 59 +0.0%
phi3 (static-cache) model_size_bytes 421 KB 421 KB +0.0%
phi3 (static-cache) num_nodes 56 56 +0.0%
qwen2 model_size_bytes 425 KB 425 KB +0.0%
qwen2 num_nodes 61 61 +0.0%
qwen2 (static-cache) model_size_bytes 425 KB 425 KB +0.0%
qwen2 (static-cache) num_nodes 58 58 +0.0%
qwen3_5_moe (hybrid-text-generation) model_size_bytes 506 KB 506 KB +0.0%
qwen3_5_moe (hybrid-text-generation) num_nodes 275 275 +0.0%
qwen3_5_text (hybrid-text-generation) model_size_bytes 458 KB 458 KB +0.0%
qwen3_5_text (hybrid-text-generation) num_nodes 129 129 +0.0%
qwen3_5_vl (hybrid-qwen-vl) model_size_bytes 977 KB 977 KB -0.0%
qwen3_5_vl (hybrid-qwen-vl) num_nodes 408 409 +0.2%
t5 (seq2seq) model_size_bytes 836 KB 836 KB -0.0%
t5 (seq2seq) num_nodes 166 174 +4.8%
whisper (speech-to-text) model_size_bytes 1008 KB 1008 KB +0.0%
whisper (speech-to-text) num_nodes 128 128 +0.0%

No performance regressions.

@codecov

codecov Bot commented Apr 14, 2026

Copy link
Copy Markdown

The author of this PR, tadani3, is not an activated member of this organization on Codecov.
Please activate this user on Codecov to display this PR comment.
Coverage data is still being uploaded to Codecov.io for purposes of overall coverage calculations.
Please don't hesitate to email us at support@codecov.io with any questions.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Improve CI scoping by making the “affected model” detection smarter for component/task changes and by enhancing registry parsing to discover model registrations defined via a declarative _REGISTRATIONS dict.

Changes:

  • Update classify_file() to treat src/mobius/components/ and src/mobius/tasks/ as “traceable” rather than always forcing run_all.
  • Extend _build_registry_class_to_types() to parse declarative _REGISTRATIONS = {...} registrations in src/mobius/_registry.py.
  • Expand/update unit and integration-style tests to cover traceable behavior and the new registry parsing pattern.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.

File Description
scripts/detect_affected_models.py Adds traceable classification and import-graph-based impact tracing; updates registry parsing to support _REGISTRATIONS dict pattern.
scripts/detect_affected_models_test.py Updates classification expectations and adds/adjusts tests for traceable behavior and registry parsing.

Comment thread scripts/detect_affected_models.py
Comment thread scripts/detect_affected_models.py Outdated
Comment thread scripts/detect_affected_models_test.py Outdated
Comment thread scripts/detect_affected_models.py Outdated
Comment thread scripts/detect_affected_models.py Outdated
Comment thread scripts/detect_affected_models.py Outdated
@tadani3
tadani3 enabled auto-merge (squash) April 15, 2026 17:06
@tadani3
tadani3 disabled auto-merge April 15, 2026 17:17
@tadani3
tadani3 merged commit e0a8b40 into main Apr 15, 2026
20 of 21 checks passed
@tadani3
tadani3 deleted the tommasoadani/detection_script_fixing branch April 15, 2026 17:56
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.

3 participants