Skip to content

Improve ModelRegistry item assignment error - #254

Merged
timkpaine merged 2 commits into
Point72:mainfrom
russlan23:agent/model-registry-setitem-162
Aug 20, 2026
Merged

Improve ModelRegistry item assignment error#254
timkpaine merged 2 commits into
Point72:mainfrom
russlan23:agent/model-registry-setitem-162

Conversation

@russlan23

Copy link
Copy Markdown
Contributor

Description

Closes #162.

Adds an explicit ModelRegistry.__setitem__ error that directs users to the controlled add API. The regression verifies the message and confirms that a failed assignment leaves the registry unchanged.

Type of Change

  • Bug fix
  • New feature
  • Documentation update
  • Refactor / code cleanup
  • CI / build configuration
  • Other (describe below)

Validation

  • python -m pytest -q ccflow/tests/test_base_registry.py - 43 passed
  • python -m ruff check ccflow/base.py ccflow/tests/test_base_registry.py - passed
  • python -m ruff format --check ccflow/base.py ccflow/tests/test_base_registry.py - passed
  • git diff --check - passed

A clean full-suite attempt could not install the complete optional development extra because the package download timed out; the repository CI can provide the full declared matrix.

Checklist

  • Linting passes
  • Full make test passes locally
  • New tests added for new functionality
  • Documentation updated (not applicable)
  • Changelog / version bump (not applicable)

Signed-off-by: russlan23 <rrusslanjr@gmail.com>
Signed-off-by: russlan23 <rrusslanjr@gmail.com>
@russlan23

Copy link
Copy Markdown
Contributor Author

I refreshed this branch against current main after the lazy-registry work in #256 caused an adjacent conflict. The resolution keeps upstream's new ModelRegistry.__contains__ behavior and the focused __setitem__ guard from this PR.\n\nValidation on the merged head:\n\n- ccflow/tests/test_base_registry.py: 59 passed, 2 skipped\n- Ruff check and format check on the two affected Python files: passed\n- git diff --check: passed\n\nThe updated head is a1ba0acd; DCO passes and GitHub reports the PR mergeable again.

@timkpaine

timkpaine commented Aug 20, 2026

Copy link
Copy Markdown
Member

guard from this PR.\n\nValidation on the merged head:\n\n- `ccflow/tests/test_base_re

Might need to have codex swap newlines for actual newlines 😉

@codecov

codecov Bot commented Aug 20, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.48%. Comparing base (1a186fd) to head (a1ba0ac).

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #254   +/-   ##
=======================================
  Coverage   93.47%   93.48%           
=======================================
  Files         176      176           
  Lines       20319    20327    +8     
  Branches     1350     1350           
=======================================
+ Hits        18994    19002    +8     
  Misses       1052     1052           
  Partials      273      273           

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

@timkpaine
timkpaine merged commit 6a788a6 into Point72:main Aug 20, 2026
19 checks passed
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.

Raise error directing users to add function on ModelRegistry objects when __setitem__ is invoked

2 participants