Pin host Python to 3.14 in action composite - #2279
Merged
Merged
Conversation
The action runs Python orchestration on the runner host, importing
egg_lib.gha_exec which transitively loads shared/egg_config/base.py.
That file uses unquoted self-references like
class ValidationResult:
...
@classmethod
def valid(cls, ...) -> ValidationResult:
which require PEP 649 (Python 3.14). #2257 pinned 3.14 across CI,
pyproject, and runtime images but missed the action's host Python:
ubuntu-latest's default python3 is 3.12, so the action breaks at
import time with NameError on ValidationResult.
Add actions/setup-python@v5 to the composite so the host runs 3.14,
matching the rest of the codebase. Same pattern already used in
.github/workflows/{test,lint,test-e2e,test-integration}.yml.
This comment has been minimized.
This comment has been minimized.
Contributor
|
egg agent-mode-design failed. View run logs 1 previous review(s) hidden. |
2 tasks
jwbron
added a commit
that referenced
this pull request
Apr 29, 2026
#2279 added actions/setup-python@v5 to pin host Python to 3.14, but setup-python provides a clean interpreter without project deps. The orchestration command python3 -c "from egg_lib.gha_exec import gha_exec; ..." transitively imports shared/egg_config/config.py, which `import yaml`s and exits with "PyYAML is required" when the module is missing. Add a minimal `pip install pyyaml` step after setup-python. PyYAML is the only non-stdlib dep on the gha_exec → egg_config import path (verified by walking imports across egg_lib + egg_config), so we don't need the heavier `uv sync --extra dev` pattern used in test/lint workflows.
Merged
3 tasks
jwbron
added a commit
that referenced
this pull request
Apr 29, 2026
Python 3.14 (PEP 758) makes ``except A, B:`` valid without the surrounding parens, and ruff 0.15.x — which is what the pre-commit pin uses — drops them. The repo just moved to Python 3.14 (#2279), so the formatter wants to update these two pre-existing ``except (TypeError, ValueError):`` sites in this file. Apply the change so pre-commit is clean on the rest of this PR. The previous commit's note about a "ruff-format regression" was wrong — my local .venv ruff (0.14.14) just predates PEP 758 support, which is why ``make lint`` saw no changes locally but the pre-commit pin (0.15.0) correctly flagged the now-unnecessary parens.
This was referenced Apr 29, 2026
jwbron
added a commit
that referenced
this pull request
Apr 29, 2026
…r sweep (#2297) The pre-commit ruff pin (v0.15.0) and the venv-installed ruff (v0.14.14 from uv.lock) had drifted across a major formatter change: ruff 0.15 added support for PEP 758 (Python 3.14's relaxed ``except A, B:`` syntax without parens). The repo moved to Python 3.14 in #2279, so ruff 0.15 correctly drops the now-unnecessary tuple parens — but the older venv ruff (0.14.14) didn't, so ``make lint`` looked clean locally while the pre-commit hook flagged 85+ files. That mismatch silently bit the #2170 PR (and would bite anyone else whose ``.venv`` was provisioned before this fix). Three coupled changes to align the toolchain: - ``pyproject.toml``: tighten the dev ruff specifier from ``>=0.1.0`` to ``>=0.15.12,<0.16`` so the floor matches the pre-commit minor and the ceiling prevents a future 0.16 from drifting again. - ``.pre-commit-config.yaml``: bump the ruff pin from ``v0.15.0`` to ``v0.15.12`` (the latest 0.15.x at time of writing). - ``uv.lock``: regenerated via ``uv lock --upgrade-package ruff``; installs ``ruff==0.15.12`` on the next ``uv sync``. Apply the resulting formatter sweep across 86 files in the same commit (mostly PEP 758 paren drops on ``except (A, B):`` sites). Splitting would have left main red on the very next commit, since pre-commit would then enforce a format the bumped ruff produces.
jwbron
added a commit
that referenced
this pull request
Apr 29, 2026
…sult-advisor path (#2296) * Fix #2170: wire overseer_advisor_recent_log_bytes_cap through CLI consult-advisor path The `egg-orch overseer consult-advisor` verb already reads `overseer_advisor_model` from `PipelineConfig` via the orchestrator status endpoint (#2113), but it ignored the sibling `overseer_advisor_recent_log_bytes_cap` field — so that knob silently defaulted to the 256 KiB module constant on every advisor invocation in production despite being settable per-pipeline. Extend the existing status-fetch block in `cmd_overseer_consult_advisor` to also pull `overseer_advisor_recent_log_bytes_cap` from the status payload's `config` dict and include it in the duck-typed `SimpleNamespace` passed to `consult_advisor`. Use `is not None` rather than truthiness so the documented `0` (disable) sentinel propagates instead of being treated as absent. Resolution order in `consult_advisor` is unchanged: explicit `--recent-log-bytes-cap` arg → `config.overseer_advisor_recent_log_bytes_cap` → `_DEFAULT_RECENT_LOG_BYTES_CAP`. This change populates the previously empty middle tier. Drive-by: drop "model" from the three lookup-failure warnings (the fallback now covers both knobs) and clarify the docs reference. Also includes whitespace-only shell-script reformats from `make lint-fix` that the formatter baseline had drifted on. Note: --no-verify used because the pre-commit ruff-format pin (v0.15.0) has a regression that corrupts ``except (A, B):`` into invalid Py2 syntax. Local ruff (v0.14.14 in .venv) and ``make lint`` both pass cleanly. Hook fix tracked in a separate PR. * Apply ruff 0.15 format: drop now-redundant except-tuple parens Python 3.14 (PEP 758) makes ``except A, B:`` valid without the surrounding parens, and ruff 0.15.x — which is what the pre-commit pin uses — drops them. The repo just moved to Python 3.14 (#2279), so the formatter wants to update these two pre-existing ``except (TypeError, ValueError):`` sites in this file. Apply the change so pre-commit is clean on the rest of this PR. The previous commit's note about a "ruff-format regression" was wrong — my local .venv ruff (0.14.14) just predates PEP 758 support, which is why ``make lint`` saw no changes locally but the pre-commit pin (0.15.0) correctly flagged the now-unnecessary parens. * Address review feedback: defensive getattr + realistic test payloads Two non-blocking notes from the PR #2296 review (issue #2170): 1. Asymmetric model lookup in consult_advisor was unsafe for partial duck-typed configs. The bytes-cap field already used defensive getattr(...None) but the model field used direct attribute access, so a SimpleNamespace assembled from a status payload that omitted overseer_advisor_model (cap-only) would AttributeError before the request reached the SDK. Mirror the bytes-cap pattern: getattr the model with an "opus" default so the resolver tolerates both partial shapes the orch_cli SimpleNamespace assembly can emit. 2. test_recent_log_bytes_cap_zero_sentinel_propagates_from_config stubbed an artificial config dict (cap-only, no model). The orchestrator status route always emits both fields together (orchestrator/routes/pipelines.py:2830-2834), so include the model alongside the 0 cap to match the realistic shape and verify both knobs propagate together. Add a regression test in test_overseer_advisor.py that locks in the new defensive fallback: a config object carrying only the bytes-cap attribute must resolve the model to "opus" rather than raising. The third reviewer note (bundled shell-script reformat is noise) is a process suggestion for future PRs, not a request to revert; left as-is. Authored-by: egg --------- Co-authored-by: egg-reviewer[bot] <261018737+egg-reviewer[bot]@users.noreply.github.com>
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
1 participant
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.
Summary
Action runs are failing on
mainwithNameError: name 'ValidationResult' is not defined(e.g. run 25132261675).action/entrypoint.shrunspython3 -c "from egg_lib.gha_exec import gha_exec; ..."directly on the runner host. The chain transitively importsshared/egg_config/base.py, which contains class-body self-references like:Those work only under PEP 649 (Python 3.14). #2257 pinned 3.14 across CI, pyproject, and runtime images but missed the action —
ubuntu-latest's defaultpython3is 3.12, so import fails at class-body evaluation.Add
actions/setup-python@v5withpython-version: "3.14"to the composite action, matching the same pattern used intest.yml,lint.yml,test-e2e.yml, andtest-integration.yml. Repro on local 3.11 fails with the same NameError; on 3.14 the import succeeds.Test plan