Fix warnings capture, hook-failure tracing, and a class-shadow false positive - #101
Open
gauravprasadgp wants to merge 4 commits into
Open
Fix warnings capture, hook-failure tracing, and a class-shadow false positive#101gauravprasadgp wants to merge 4 commits into
gauravprasadgp wants to merge 4 commits into
Conversation
warnings.warn() writes through Python's warning machinery rather than sys.stderr.write() directly, so it bypassed the ContextVarStream wrapper used to capture stdout/stderr from generated code. Under pytest this was extra confusing because the warnings-capture plugin replaces warnings.showwarning for the duration of every test, which would swallow the warning before it ever reached our wrapper. Install a showwarning hook that writes formatted warnings to sys.stderr (and therefore into the active capture buffer, if any), re-installed before every code execution so it isn't left stale by whatever else touched warnings.showwarning in the meantime. Signed-off-by: gauravprasad <prasadgaurav559@gmail.com>
call_before_hook swallows exceptions from a hooks implementation and returns None, which was indistinguishable from before_agent_call simply never running (e.g. when agent_call middleware short-circuits). Both wrapper paths gated after_agent_call on "hook_context is not None", so a hook that raised silently lost after_agent_call too - the call happened, but nothing about it was ever traced. Track whether before_agent_call was actually attempted separately from whether it returned usable context, and fire after_agent_call in either case. OpenInferenceHooks.after_agent_call now falls back to a minimal ERROR span when it has no context to complete, so a failed hook still leaves a trace record instead of vanishing entirely. Signed-off-by: gauravprasad <prasadgaurav559@gmail.com>
eval_pipeline is already a uv workspace member and ships in the "dev" dependency group, so uv sync --group dev installs it editable along with everything else. The README still told contributors to pip install -e it separately, which contradicts the repo's uv-only convention and is just unnecessary. Point external consumers at the git+ install instead. Signed-off-by: gauravprasad <prasadgaurav559@gmail.com>
ClassAssignmentValidator blocks ClassName.attr = value to keep generated
code from corrupting shared class state, but it matched purely on
variable name. Once code did ClassName = {} (or any other reassignment),
ClassName was a local variable for the rest of the block, yet the
validator still rejected ClassName.attr = value as if it were touching
the class - a known false positive the tests documented as "acceptable"
rather than fixed.
Track the line of the first top-level ClassName = <expr> reassignment
per known class name and treat attribute writes after that line as safe.
Restricted to top-level statements only: a reassignment nested inside
if/for/try could be on a branch that never executes, and honoring it
there would let generated code fake a shadow to bypass the check on a
real corruption elsewhere. Added a test covering exactly that bypass
attempt to make sure it stays rejected.
Signed-off-by: gauravprasad <prasadgaurav559@gmail.com>
gauravprasadgp
force-pushed
the
contrib/local-fixes
branch
from
August 6, 2026 07:00
9d6a9a2 to
2600035
Compare
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.
What's in here
Warnings capture —
warnings.warn()bypassed theContextVarStreamstdout/stderr capture since it goes through Python's warning machinery, notsys.stderr.write()directly. Installs ashowwarninghook that writes intosys.stderr, re-installed before every execution since pytest resetswarnings.showwarningper test.Hook-failure tracing ->
after_agent_callonly fired whenhook_context is not None, which can't tell "middleware short-circuited" apart from "before_agent_callraised andcall_before_hookswallowed it." The second case silently dropped the trace. Now tracks whether the before-hook actually ran, andOpenInferenceHooks.after_agent_callemits aminimal ERROR span when it gets no context instead of skipping the call entirely.
Class-assignment false positive ->
ClassAssignmentValidatorblocksClassName.attr = valueto stop generated code corrupting shared class state, but matched on name only.ClassName = {}followed byClassName.attr = value(a plain local dict) got flagged too — a known false positive the tests had marked "acceptable" instead of fixed. Now tracks the first top-level reassignment of a class name and treats writes after that line as safe. Only top-level statements count, on purpose: a reassignment insideif/for/trycould be on a branch that never runs, and honoring it there would let generated code fake a shadow to slip a real corruption past the check.eval_pipeline docs -> it's already a uv workspace member in the
devgroup, souv sync --group devinstalls it editable. The README still saidpip install -e.Testing
pytest tests/runtime— 1055 passedpytest tests/integration/test_hook_failure_traces.py— 4 passedpytest tests/runtime/test_code_validator.py— 228 passedpytest(full suite, minustest_mcpwhich fails on main too from a missing optionaldependency) — 5222 passed
ruff checkandpyrightclean on touched files