fix: suppress expected lazy skill read errors in traces#3891
fix: suppress expected lazy skill read errors in traces#3891Sakshamm-Goyal wants to merge 2 commits into
Conversation
![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: d49a1d8b84
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| with expected_sandbox_error(WorkspaceReadNotFoundError): | ||
| handle = await session.read(skill_md_path, user=user) |
There was a problem hiding this comment.
For sandbox clients or injected sessions that implement BaseSandboxSession.read as documented and raise FileNotFoundError for a missing path, this probe is still treated as a failed sandbox.read event/span even though the next line catches FileNotFoundError as the expected lazy-load miss. Built-in backends mostly wrap this as WorkspaceReadNotFoundError, but custom backends following the base contract will still produce the false error telemetry this change is trying to eliminate.
Useful? React with 👍 / 👎.
|
Thanks for sharing this idea. The issue was resolved by #3893 |
2 participants
Summary
Lazy skill materialization probes for
SKILL.mdbefore copying a skill into the workspace. A first load expects that read to be absent, butSandboxSessionrecorded the caughtWorkspaceReadNotFoundErroras a failedsandbox.readspan.This change scopes expected-error telemetry to that probe only, so the read still raises to the loader and triggers materialization, while its completion event and span no longer carry an error. Other missing reads retain their normal error telemetry.
Test plan
uvx --from uv>=0.10.0 uv sync --all-extras --all-packages --group devuvx --from uv>=0.10.0 uv run make formatuvx --from uv>=0.10.0 uv run make lintuvx --from uv>=0.10.0 uv run make typecheckuvx --from uv>=0.10.0 uv run make testsIssue number
Closes #3889
Checks