Skip to content

feat: add custom exceptions, runconfig, eventtype enum and async work…#17

Merged
jolovicdev merged 3 commits intomasterfrom
experimental/1.1.9
Feb 25, 2026
Merged

feat: add custom exceptions, runconfig, eventtype enum and async work…#17
jolovicdev merged 3 commits intomasterfrom
experimental/1.1.9

Conversation

@jolovicdev
Copy link
Copy Markdown
Owner

…flow nodes

@jolovicdev
Copy link
Copy Markdown
Owner Author

@codex check this experimental PR

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: c0ac66fde6

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/blackgeorge/config.py Outdated
Comment on lines +28 to +30
current = {**self.__dict__}
current.update(kwargs)
return RunConfig(**current)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Deep-copy mutable fields when cloning RunConfig

with_overrides() copies self.__dict__ shallowly, so mutable fields (notably events, and also dicts like stream_options) are shared between the original and derived configs. When a caller reuses a base config and creates per-run overrides, one run mutates the same events list used by the other, which can mix event histories and produce incorrect report data (especially under concurrent runs).

Useful? React with 👍 / 👎.

@jolovicdev jolovicdev marked this pull request as ready for review February 25, 2026 12:13
@jolovicdev
Copy link
Copy Markdown
Owner Author

@codex check both commits again. with_overrides() copies self.dict shallowly, so mutable fields (notably events, and also dicts like stream_options) are shared between the original and derived configs. When a caller reuses a base config and creates per-run overrides, one run mutates the same events list used by the other, which can mix event histories and produce incorrect report data (especially under concurrent runs).

this is fixed.

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: bfe22b01fa

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/blackgeorge/config.py Outdated
default_model: str | None = None

def with_overrides(self, **kwargs: Any) -> "RunConfig":
current = copy.deepcopy(self.__dict__)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Avoid deep-copying adapter when applying config overrides

with_overrides now deep-copies the entire RunConfig.__dict__, so it also deep-copies adapter and emit instead of just mutable config containers; this breaks common custom adapters that hold non-picklable runtime state (for example, an adapter with a threading.Lock raises TypeError: cannot pickle '_thread.lock' object when with_overrides(...) is called). As a result, per-run override flows can fail at runtime even when only changing scalar fields, so the copy should be limited to mutable fields like events/stream_options rather than deep-copying the whole object graph.

Useful? React with 👍 / 👎.

@jolovicdev jolovicdev merged commit 13782f0 into master Feb 25, 2026
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.

1 participant