Share repos/index state across Action delegates instead of copying it by anandhu-eng · Pull Request #288 · mlcommons/mlcflow

anandhu-eng · 2026-08-07T10:31:51Z

get_action() builds a fresh delegate per dispatch, but every subclass started with self.__dict__.update(vars(parent)) - a snapshot copy of the parent's attributes. The copy shares the reference to repos, so the two stay linked until someone rebinds; register_repo() does exactly that with self.repos = self.load_repos_and_meta(), which points only the delegate's name at the new list and leaves the long-lived root holding the pre-pull one.

Since the delegate is dropped when the call returns and the root is what serves every later search()/find()/rm(), a pull followed by a search in the same process searched a stale repo list and a stale index. The refresh in call_script_module_function() could not help: it wrote to ScriptAction's own copy (and to self.index, while get_index() reads _index).

Move the mutable state onto MLCState, which delegates hold by reference and never copy. repos, local_repo and current_repo_path become properties routed through it, so an assignment inside any delegate updates the object every other delegate sees, and get_index() caches there too. Action.init now takes the parent and does this for every subclass, so RepoAction, ScriptAction, CacheAction and ExperimentAction no longer need a constructor at all; CfgAction keeps one only for its default_parent fallback.

With the state shared, the refresh at the auto-pull site is dead code - register_repo() already reloads the repos and adds the new one to the index - so it is deleted rather than fixed.

tests/test_action_shared_state.py pins the sharing and the original bug; 8 of its 9 cases fail without this change.

✅ PR Checklist

✅ Testing & CI

Have tested the changes in my local environment, else have properly conveyed in the PR description
The change includes a GitHub Action to test the script(if it is possible to be added).
No existing GitHub Actions are failing because of this change.

📚 Documentation

README or help docs are updated for new features or changes.
CLI help messages are meaningful and complete.

📁 File Hygiene & Output Handling

No unintended files (e.g., logs, cache, temp files, pycache, output folders) are committed.

🛡️ Safety & Security

No secrets or credentials are committed.
Paths, shell commands, and environment handling are safe and portable.

🙌 Contribution Hygiene

PR title and description are concise and clearly state the purpose of the change.
Related issues (if any) are properly referenced using Fixes # or Closes #.
All reviewer feedback has been addressed.

github-actions · 2026-08-07T10:32:00Z

MLCommons CLA bot All contributors have signed the MLCommons CLA ✍️ ✅

github-actions · 2026-08-07T10:32:16Z

🤖 AI PR Review Summary

This PR refactors the Action class to centralize mutable shared state in a new MLCState object, which is referenced by all Action instances and their delegates. This ensures that changes to shared data like repos and index persist across delegate lifetimes, fixing visibility issues between actions. The PR also removes redundant init methods from subclasses to avoid bypassing the shared state setup. A comprehensive test suite is added to verify shared state behavior. The design improves state consistency and reduces bugs related to state duplication. Risks include ensuring all subclasses rely on the base init and that state sharing does not introduce unintended side effects in concurrent scenarios.

github-actions · 2026-08-07T10:32:17Z

@@ -24,9 +25,41 @@ class Action:
    cfg = None


Consider adding a docstring or comment explaining the purpose of the _CONFIG_ATTRS tuple for clarity.

github-actions · 2026-08-07T10:32:18Z

+    _CONFIG_ATTRS = ('repos_path', 'local_cache_path', 'cfg', 'logger')
+
+    # Read/written as plain attributes throughout the codebase; routing them
+    # through self.state means `self.repos = ...` in a delegate updates the one


The property setters and getters for repos, local_repo, and current_repo_path are well done to route through shared state. Ensure that all code mutating these attributes uses these properties to avoid bypassing shared state.

github-actions · 2026-08-07T10:32:19Z

@@ -230,7 +265,23 @@ def _item_from_index_entry(self, res, target_name):
            return None


In init, the logic to copy startup config attributes from parent is good. However, consider explicitly documenting the expected attributes on parent to avoid silent failures if a parent lacks them.

github-actions · 2026-08-07T10:32:20Z

@@ -270,7 +321,6 @@ def __init__(self):



The removal of self._index in favor of caching index on state is consistent with the shared state approach. Make sure no other code relies on self._index attribute.

github-actions · 2026-08-07T10:32:20Z

@@ -21,11 +21,6 @@ class CacheAction(Action):



The commented-out init method in CacheAction should be removed to avoid confusion, since the base Action.init handles state sharing properly.

github-actions · 2026-08-07T10:32:21Z

@@ -6,11 +6,7 @@

 class CfgAction(Action):


Good to see super().init used here to ensure proper state initialization. This pattern should be applied consistently across all Action subclasses.

github-actions · 2026-08-07T10:32:22Z

@@ -4,11 +4,6 @@
 from . import utils



The init method is removed here, which is good. Confirm that no subclass overrides init without calling super().init to maintain shared state.

github-actions · 2026-08-07T10:32:23Z

-
    def _build_pull_command(self, repo_path, branch=None, clone_depth=None,
                            fast_forward_only=False):
        pull_command = ['git', '-C', repo_path, 'pull']


Same as above: removing the init override is good. Ensure all subclasses follow this pattern to avoid state sharing issues.

github-actions · 2026-08-07T10:32:24Z

@@ -38,11 +37,6 @@ class ScriptAction(Action):
    Using both alias and UID: <script_alias>,<script_uid> (e.g., detect-os,5b4e0237da074764)



Remove the commented-out init method to avoid confusion and ensure consistent state sharing via base class.

github-actions · 2026-08-07T10:32:25Z

@@ -0,0 +1,21 @@
+class MLCState:


The MLCState class is well documented and clearly designed to hold shared mutable state. This is a good design to avoid state duplication across delegates.

github-actions · 2026-08-07T10:32:25Z

@@ -0,0 +1,154 @@
+import os


The new test suite is comprehensive and well structured to verify shared state behavior across delegates and root Action. This greatly improves confidence in the refactor.

get_action() builds a fresh delegate per dispatch, but every subclass started with `self.__dict__.update(vars(parent))` - a snapshot copy of the parent's attributes. The copy shares the *reference* to `repos`, so the two stay linked until someone rebinds; `register_repo()` does exactly that with `self.repos = self.load_repos_and_meta()`, which points only the delegate's name at the new list and leaves the long-lived root holding the pre-pull one. Since the delegate is dropped when the call returns and the root is what serves every later search()/find()/rm(), a pull followed by a search in the same process searched a stale repo list and a stale index. The refresh in call_script_module_function() could not help: it wrote to ScriptAction's own copy (and to `self.index`, while get_index() reads `_index`). Move the mutable state onto MLCState, which delegates hold by reference and never copy. `repos`, `local_repo` and `current_repo_path` become properties routed through it, so an assignment inside any delegate updates the object every other delegate sees, and get_index() caches there too. Action.__init__ now takes the parent and does this for every subclass, so RepoAction, ScriptAction, CacheAction and ExperimentAction no longer need a constructor at all; CfgAction keeps one only for its default_parent fallback. With the state shared, the refresh at the auto-pull site is dead code - register_repo() already reloads the repos and adds the new one to the index - so it is deleted rather than fixed. tests/test_action_shared_state.py pins the sharing and the original bug; 8 of its 9 cases fail without this change. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

anandhu-eng requested a review from a team as a code owner August 7, 2026 10:31

anandhu-eng temporarily deployed to ai-review August 7, 2026 10:31 — with GitHub Actions Inactive

github-actions Bot reviewed Aug 7, 2026

View reviewed changes

anandhu-eng marked this pull request as draft August 7, 2026 10:33

anandhu-eng force-pushed the shared-action-state branch from 0f05653 to 84d71d7 Compare August 7, 2026 10:52

Navigation Menu

Uh oh!

Share repos/index state across Action delegates instead of copying it - #288

Share repos/index state across Action delegates instead of copying it#288
anandhu-eng wants to merge 1 commit into
mainfrom
shared-action-state

anandhu-eng commented Aug 7, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Aug 7, 2026

Uh oh!

github-actions Bot commented Aug 7, 2026

Uh oh!

github-actions Bot Aug 7, 2026

Uh oh!

github-actions Bot Aug 7, 2026

Uh oh!

github-actions Bot Aug 7, 2026

Uh oh!

github-actions Bot Aug 7, 2026

Uh oh!

github-actions Bot Aug 7, 2026

Uh oh!

github-actions Bot Aug 7, 2026

Uh oh!

github-actions Bot Aug 7, 2026

Uh oh!

github-actions Bot Aug 7, 2026

Uh oh!

github-actions Bot Aug 7, 2026

Uh oh!

github-actions Bot Aug 7, 2026

Uh oh!

github-actions Bot Aug 7, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

		@@ -230,7 +265,23 @@ def _item_from_index_entry(self, res, target_name):
		return None

		@@ -38,11 +37,6 @@ class ScriptAction(Action):
		Using both alias and UID: <script_alias>,<script_uid> (e.g., detect-os,5b4e0237da074764)

Uh oh!

Conversation

anandhu-eng commented Aug 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ PR Checklist

✅ Testing & CI

📚 Documentation

📁 File Hygiene & Output Handling

🛡️ Safety & Security

🙌 Contribution Hygiene

Uh oh!

github-actions Bot commented Aug 7, 2026

Uh oh!

github-actions Bot commented Aug 7, 2026

🤖 AI PR Review Summary

Uh oh!

github-actions Bot Aug 7, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Aug 7, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Aug 7, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Aug 7, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Aug 7, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Aug 7, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Aug 7, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Aug 7, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Aug 7, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Aug 7, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Aug 7, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

anandhu-eng commented Aug 7, 2026 •

edited

Loading