fix(runtime): make capture evidence and provider handoffs composable - #468
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 22fe8706be
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b4220a2b8d
ℹ️ 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".
| output = await self._run([str(adjacent), "--version"], min(timeout, 10)) | ||
| if output.decode().strip() == f"py-spy {PY_SPY_VERSION}": | ||
| return self._bind(str(adjacent)), [] |
There was a problem hiding this comment.
When the adjacent py-spy executable is replaced between the version probe and this second _bind, preparation activates the replacement even though that identity was never version-checked; later capture validation only confirms that the replacement remains unchanged. Retain the executable binding used by the probe, or re-run the version check against the exact binding being activated.
AGENTS.md reference: AGENTS.md:L33-L35
Useful? React with 👍 / 👎.
| raise RepositoryError("REPOSITORY_CORRUPTION", "Evidence membership is incomplete.") | ||
| inputs = body["analysis_request"].get("inputs", []) | ||
| indices = layout["analysis_sources"] | ||
| if indices and len(indices) != len(inputs): |
There was a problem hiding this comment.
When a manifest has nonempty analysis_request.inputs but an empty source_layout.analysis_sources, this condition skips the length check and the following zip validates nothing. The manifest is therefore accepted as authoritative, but its resource projection exposes no analysis_sources, making the preserved analysis impossible to replay; require the mapping length to equal the input length even when the mapping is empty.
AGENTS.md reference: AGENTS.md:L40-L41
Useful? React with 👍 / 👎.
|
|
||
| next_action = None | ||
| if preparation.restart_required: | ||
| if preparation.restart_required is not False: |
There was a problem hiding this comment.
When prepare_providers is called for only py-spy, this new logic correctly leaves next_action null because the collector is activated in the current session, but the MCP server instructions at lines 308–310 still direct the agent to reconnect after every missing managed provider. Following that higher-level guidance can unnecessarily terminate the session and lose unpreserved analyses, so make it tell callers to follow the returned activation status instead.
AGENTS.md reference: AGENTS.md:L44-L48
Useful? React with 👍 / 👎.
| if collector is not None: | ||
| self._py_spy = collector |
There was a problem hiding this comment.
When an earlier mixed preparation pauses during server-environment preparation after resolving collector A, a later py-spy-only request can successfully activate a newer collector B; once the earlier request resumes, this unconditional assignment overwrites B with stale A. The next capture then reports an expired collector despite the later preparation succeeding, so serialize binding publication or commit only if the session binding has not advanced since this request began.
AGENTS.md reference: AGENTS.md:L40-L41
Useful? React with 👍 / 👎.
| if adjacent.is_file(): | ||
| output = await self._run([str(adjacent), "--version"], min(timeout, 10)) |
There was a problem hiding this comment.
When a py-spy file exists beside the server interpreter but is not executable or exits nonzero for --version, _run raises SetupFailure here and aborts preparation before trying the pinned uvx environment. This prevents an explicit preparation request from recovering from a stale or broken adjacent installation even when uvx is available; treat a failed adjacent probe as a miss and continue to the isolated preparation path.
Useful? React with 👍 / 👎.
1 participant
Capture and provider-preparation results could lose the information needed for the next investigation step. This change separates authoritative outcomes from bounded diagnostics, durable source identity from storage location, and dependency preparation from server activation.
flameox evidence locationdiagnostic.point_estimate_classificationand an explicit descriptive basis now separate it from interval uncertainty and semantic correctness.The collector design uses uv's isolated, version-selectable tool environments and py-spy's standalone collector model. No custom package cache or persistent provider inventory is introduced. CLI launchers still declare the complete requested provider set; workload packages and external host prerequisites remain separately verified at capture time where supported.
Compatibility:
point_estimate_classificationreplaces experimentdecision. Old path-bound continuation tokens require a fresh analysis. Existing evidence manifests remain readable; new captures add exit-attribution fields and new preservation adds explicit source-layout metadata, both requiring the updated reader. Selectors from an earlier revision of this PR must be re-enumerated from the evidence resource. Same-session independent collector activation currently covers py-spy; Perfetto's native engine and vendor profilers remain external requirements.Review follow-up (
b4220a2): source layout now explicitly records file/directory identity, exact membership, and ordered analysis mapping. This retains empty directories and exact members through repeated preservation without interpreting private filenames as structure. Selectors address public immutable manifest positions. Invocation builders declare exit ownership. Preparation forwards safe uv policy controls and commits collector bindings only when the whole request succeeds; failed concurrent requests cannot roll back successful activation.Validation:
22fe870), covering all six comments. Added coverage includes repeated preservation, reopened sessions, malformed source metadata, and concurrent preparation. Independent exact-diff review and one consolidated follow-up completed; its findings were fixed and covered by regression tests before final full validation.uv run pytest -q -m 'not optional and not performance' --tb=short: 347 passed, 1 skipped, 4 deselected. Includes real subprocess and MCP resource round trips, collector-failure attribution, byte-pressure outcomes, cancellation, and preservation pagination.uv run ruff check src tests tools,uv run ruff format --check src tests tools, anduv run mypy src tests tools: passed.uv run lint-imports,uv run vulture src/flameox --min-confidence 80, and the CI dependency check with deptry: passed.uvx --isolated --python 3.12 --from py-spy==0.4.2probe resolved and executed collector version 0.4.2. The local evidence-location CLI was also exercised.The AIPerf import test was skipped because that optional package is absent. GPU/vendor tooling and non-Linux collector execution were not exercised; preparation fixtures establish lifecycle behavior, not host profiling permission or universal workload compatibility.
Fixes #457
Fixes #458
Fixes #459
Fixes #460
Fixes #461
Fixes #462
Fixes #463
Fixes #464
Fixes #465
Fixes #466
Fixes #467