[Fix] Circular Import Bug

[Saga4]

PR Type

Bug fix, Enhancement

---

Description

• Centralize LSP execution context storage

• Remove server-scoped contextvars attribute

• Update context usage in beta and messages

• Introduce shared context module

---

Diagram Walkthrough

flowchart LR
  beta["beta.py (execution_context)"] -- "uses" --> context["context.py (execution_context_vars)"]
  lspmsg["lsp_message.py (serialize)"] -- "reads" --> context
  server["server.py"] -- "removes per-server contextvars" --> context

Loading

File Walkthrough

	Relevant files
Bug fix	beta.py Use shared execution context in beta                                          codeflash/lsp/beta.py Import shared execution_context_vars Replace server.execution_context_vars usage Adjust context set/get/reset in manager +4/-3      lsp_message.py Decouple message serialization from server context              codeflash/lsp/lsp_message.py Import execution_context_vars from context module Remove dependency on server for context access Read task_id from shared context +2/-2
Enhancement	context.py Introduce shared LSP execution context module                        codeflash/lsp/context.py Add new module Define global execution_context_vars ContextVar Provide default empty dict +9/-0      server.py Remove server-scoped execution contextvars                              codeflash/lsp/server.py Remove per-instance execution_context_vars Eliminate local contextvars import +0/-5

---

[github-actions]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪

🧪 No relevant tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Context Reset Safety Ensure the context manager always has a valid token before calling execution_context_vars.reset(token). If an exception occurs before set() (e.g., unexpected kwargs types), reset could reference an uninitialized token. Consider guarding or initializing token to None and checking before reset. def execution_context(**kwargs: str) -> None: """Temporarily set context values for the current async task.""" # Create a fresh copy per use current = {**execution_context_vars.get(), **kwargs} token = execution_context_vars.set(current) try: yield finally: execution_context_vars.reset(token) Mutable Default Context The ContextVar default is a shared mutable dict. Callers that mutate the retrieved dict without setting a fresh copy could unintentionally share state across tasks. Consider using an immutable default or always cloning on get/use, and documenting usage. # Shared execution context for tracking task IDs and other metadata execution_context_vars: contextvars.ContextVar[dict[str, str]] = contextvars.ContextVar( "execution_context_vars", default={}, # noqa: B039 )

[github-actions]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Copy context before merging Ensure the retrieved context is copied before merging to prevent accidental mutation of the shared dict. This avoids cross-task leakage when the stored dict instance is reused. codeflash/lsp/beta.py [340-346] @contextlib.contextmanager def execution_context(**kwargs: str) -> None: """Temporarily set context values for the current async task.""" - # Create a fresh copy per use - current = {**execution_context_vars.get(), **kwargs} + base = execution_context_vars.get().copy() + current = {**base, **kwargs} token = execution_context_vars.set(current) try: yield finally: execution_context_vars.reset(token) Suggestion importance[1-10]: 6 __ Why: Copying the dict returned from execution_context_vars.get() avoids potential mutation of a shared instance. It's a reasonable safety improvement with moderate impact given current usage.	Low
	Avoid shared mutable default Using a mutable dict as a ContextVar default can be shared across contexts and mutated unexpectedly. Replace with a factory that returns a fresh dict per call and ensure callers copy before mutating. codeflash/lsp/context.py [6-9] +def _new_ctx() -> dict[str, str]: + return {} + execution_context_vars: contextvars.ContextVar[dict[str, str]] = contextvars.ContextVar( "execution_context_vars", - default={}, # noqa: B039 + default=_new_ctx(), # fresh instance per import; callers should copy before mutating ) Suggestion importance[1-10]: 5 __ Why: The concern about a mutable default dict in a ContextVar is valid style-wise, but ContextVar defaults are not mutated directly in the PR and callers set a merged copy. Improvement lowers risk of accidental mutation, but impact is moderate.	Low
General	Validate context var type Guard against non-dict values in the context to avoid AttributeError on .get. Coerce to an empty dict if the ContextVar holds an unexpected type. codeflash/lsp/lsp_message.py [47-53] def serialize(self) -> str: if not is_LSP_enabled(): return "" from codeflash.lsp.context import execution_context_vars - execution_ctx = execution_context_vars.get() - current_task_id = execution_ctx.get("task_id", None) + raw_ctx = execution_context_vars.get() + execution_ctx = raw_ctx if isinstance(raw_ctx, dict) else {} + current_task_id = execution_ctx.get("task_id") data = self._loop_through(asdict(self)) ordered = {"type": self.type(), "task_id": current_task_id, **data} return message_delimiter + json.dumps(ordered) + message_delimiter Suggestion importance[1-10]: 4 __ Why: Adding a type guard prevents rare AttributeErrors if the context holds an unexpected type, but the ContextVar is typed to a dict with a default dict, making this edge case unlikely; modest benefit.	Low