Merged
Conversation
Contributor
|
Size Change: 0 B Total Size: 129 MB ℹ️ View Unchanged
|
Contributor
Prompt To Fix All With AIThis is a comment left during a code review.
Path: products/tasks/backend/serializers.py
Line: 230-260
Comment:
**`parse_run_state` called 4× per serialized object**
Each of the four `get_*` methods independently calls `parse_run_state(obj.state)`, which runs Pydantic JSON-coercion on the same dict every time. When a list endpoint returns many `TaskRun` objects, the state is parsed 4× per row. Extracting the parsed state once — e.g. with a private helper — would satisfy OnceAndOnlyOnce and avoid the repeated work.
```python
def _get_run_state(self, obj: TaskRun) -> RunState:
return parse_run_state(obj.state)
def get_runtime_adapter(self, obj: TaskRun) -> str | None:
state = self._get_run_state(obj)
return state.runtime_adapter.value if state.runtime_adapter is not None else None
# … and so on for get_provider, get_model, get_reasoning_effort
```
How can I resolve this? If you propose a fix, please make it concise.
---
This is a comment left during a code review.
Path: products/tasks/backend/tests/test_api.py
Line: 377-381
Comment:
**Single-case `@parameterized.expand`**
The decorator wraps only `("high",)`. Since Codex supports `low`, `medium`, and `high`, the parameterized machinery adds overhead (name mangling, extra test node) with no coverage benefit over a plain test. Either expand to all valid efforts or remove the decorator.
```python
@parameterized.expand(
[
("low",),
("medium",),
("high",),
]
)
```
How can I resolve this? If you propose a fix, please make it concise.
---
This is a comment left during a code review.
Path: products/tasks/backend/tests/test_api.py
Line: 431-450
Comment:
**Incomplete coverage for missing-field validation**
The payload omits both `provider` and `model`, so the `validate()` loop adds errors for both. The assertion only checks `attr: "provider"`, relying implicitly on Python dict insertion order to make that error win. There is no test that sending `provider` alone (without `model`) also returns a validation error. A second parameterised case (or a separate test) that omits `model` instead of `provider` would make the requirement explicit.
How can I resolve this? If you propose a fix, please make it concise.Reviews (1): Last reviewed commit: "feat(cloud-agent): codex support" | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
tatoalo
force-pushed
the
feat/cloud-agent/codex-cloud-run-support
branch
2 times, most recently
from
8ad5e7d to
a276aac
Compare
Contributor
|
⏭️ Skipped snapshot commit because branch advanced to The new commit will trigger its own snapshot update workflow. If you expected this workflow to succeed: This can happen due to concurrent commits. To get a fresh workflow run, either:
|
Contributor
Query snapshots: Backend query snapshots updatedChanges: 1 snapshots (1 modified, 0 added, 0 deleted) What this means:
Next steps:
|
Contributor
|
🎭 Playwright report · View test results →
These issues are not necessarily caused by your changes. |
joshsny
approved these changes
Apr 15, 2026
Contributor
joshsny
left a comment
joshsny
left a comment
There was a problem hiding this comment.
LGTM - this is a nice cleanup
tatoalo
force-pushed
the
feat/cloud-agent/codex-cloud-run-support
branch
2 times, most recently
from
091f6d3 to
14ef67e
Compare
Contributor
|
⏭️ Skipped snapshot commit because branch advanced to The new commit will trigger its own snapshot update workflow. If you expected this workflow to succeed: This can happen due to concurrent commits. To get a fresh workflow run, either:
|
Contributor
Query snapshots: Backend query snapshots updatedChanges: 1 snapshots (1 modified, 0 added, 0 deleted) What this means:
Next steps:
|
chore: cleaner test(backend): update query snapshots
tatoalo
force-pushed
the
feat/cloud-agent/codex-cloud-run-support
branch
from
078fb59 to
93e18e6
Compare
Contributor
Query snapshots: Backend query snapshots updatedChanges: 1 snapshots (1 modified, 0 added, 0 deleted) What this means:
Next steps:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
cloud runs need an explicit runtime contract so the selected LLM adapter, provider, model, and reasoning effort are preserved across task creation, resume, and sandbox startup.
This allows us to have
codexsupport E2E also for flows outside of PostHog Code (slack interaction and PostHog Tasks UI directly, logic is not plumbed but after this rework it would be an easy addition)Changes
Companion: PostHog/code#1644