Skip to content

🧪 Add regression coverage for incomplete model configs - #3816

Open
original4422 wants to merge 4 commits into
ModelEngine-Group:developfrom
original4422:bugfix/3810-safe-model-name
Open

original4422 wants to merge 4 commits into
ModelEngine-Group:developfrom
original4422:bugfix/3810-safe-model-name

Conversation

@original4422

@original4422 original4422 commented Aug 30, 2026 •

Copy link
Copy Markdown

Summary

The safe missing-key handling from #3810 is already upstream, now in management.services.model.names. This PR preserves that implementation and its modelName fallback while retaining the regression coverage.

 get_model_name_from_config tests
+ empty, null and partial mappings
+ quick-config modelName fallback
+ model_name precedence over modelName

Evidence

  • Before: current develop covers only the two complete model mappings in this test class.
  • After: the parameterized regression matrix covers ten additional inputs. Against develop at 9ee52bce, the cumulative change is only this test matrix and the required copyright header in test_config_utils.py; backend and SDK production files are unchanged.
  • backend/.venv/bin/python -m pytest test/backend/utils/test_config_utils.py test/backend/utils/test_model_name_utils.py -q: 48 passed.
  • Focused ruff check --select E9,F63,F7,F82, compileall, the repository copyright-header checker, and current-base-relative git diff --check pass.
  • Validation uses the retained original Python 3.11.16 backend environment and editable sdk[dev]; backend/SDK dependency manifests have not changed since the documented setup. This is unit-regression validation, not a full backend or live-service test claim.

Related to #3810. The original production fix and later helper relocation are preserved. This maintenance syncs the existing tests with current develop; it introduces no product behavior or formal-case changes.

Merge Danger

Door: two-way

Blast Radius: tests

The production helper matches current develop; the effective contribution is regression coverage only.

Copilot AI lite review requested due to automatic review settings August 30, 2026 20:43

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.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@original4422

Copy link
Copy Markdown
Author

Hi @WMC001, when you have a chance, could you please take a look and let me know whether this is ready to merge or needs any changes? Thank you.

@original4422

Copy link
Copy Markdown
Author

Hi @WMC001, when convenient, could you please check whether this PR is ready to merge or needs any changes? Thank you!

@original4422

Copy link
Copy Markdown
Author

Hi @WMC001, when you have a moment, could you please check whether this PR needs any changes before it can be merged? Thank you!

@original4422

Copy link
Copy Markdown
Author

Hi @WMC001, when convenient, could you let me know whether this PR needs any changes before merge? Thank you!

@original4422

Copy link
Copy Markdown
Author

Hi @jeffwu-1999, when convenient, could you please check whether this PR needs any changes before merge? Thank you!

@original4422 original4422 changed the title 🐛 Fix incomplete model config handling 🧪 Add regression coverage for incomplete model configs Oct 10, 2026
@original4422

Copy link
Copy Markdown
Author

Updated in 18f4839 to preserve the upstream modelName fallback and extend regression coverage for empty/partial mappings and name precedence. The branch passes all 43 tests in test_config_utils.py; its prospective merge with current develop matches the locally validated tree with 44 passing tests. Focused Ruff, compileall and whitespace checks also pass. The effective merged change adds tests only; the existing production behavior is preserved.

@original4422

Copy link
Copy Markdown
Author

Hi @WMC001, when you have a chance, could you please review the updated regression coverage and let me know whether this is ready to merge? Thank you!

@original4422

Copy link
Copy Markdown
Author

Hi @YehongPan, when you have a chance, could you please review the regression coverage for incomplete model configs and let me know if any changes are needed? Thank you!

@original4422

Copy link
Copy Markdown
Author

Synced with current develop (9ee52bce) in b56c5ea631b44664a684b03cdc0aab24b7fa32d2. The relocated production helper remains unchanged; the cumulative PR now contains only the existing incomplete-config regression matrix and the required copyright header in its test file. The original config-utils and model-name-utils suites pass 48 tests; focused Ruff, compileall, the copyright checker and cumulative whitespace checks also pass. Both newly published maintenance commits have valid GitHub-verified signatures. This sync does not claim full backend or live-service validation.

@original4422

Copy link
Copy Markdown
Author

Hi @WMC001, when convenient, could you please review the updated regression coverage for incomplete model configs and let me know if any changes are needed? Thank you.

This branch has not been deployed

No deployments
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.

2 participants