fix(review-gateway): import Any so the module's annotations resolve - #1160
fix(review-gateway): import Any so the module's annotations resolve#1160seonghobae wants to merge 2 commits into
Conversation
review_pool_admissions is annotated `Sequence[Any]` while the module imports
only Mapping and Sequence from typing. `from __future__ import annotations`
keeps annotations as strings, so the module imports cleanly and every existing
test passes -- the break surfaces only for a caller that *resolves* the
annotations:
>>> import typing
>>> from contextual_orchestrator import review_gateway
>>> typing.get_type_hints(review_gateway.review_pool_admissions)
NameError: name 'Any' is not defined
That reaches any runtime type checker, dataclass/schema generator, docs
builder, or validation layer that introspects this module -- which matters
here because review_pool_admissions is the owner-side provenance projection a
consumer is meant to read instead of re-deriving admission itself.
Scope is exactly one name. A package-wide sweep also flags
model_discovery.PriceBook and provider_catalog_store.PrivacyPolicyAssessment,
but both are deliberate `if TYPE_CHECKING:` deferred imports with correctly
quoted annotations -- the documented circular-import remedy, whose cost is
precisely that get_type_hints cannot resolve them. Those are correct as
written and are left alone. review_gateway declares no TYPE_CHECKING block, so
it has no name it is entitled to leave unresolvable.
tests/test_review_gateway_annotation_resolution.py sweeps every function and
class the module defines and asserts its hints resolve, so the next such name
is caught rather than waiting for a consumer to hit it. Verified the sweep
fails without this one-line fix (2 failed) and passes with it (12 passed); it
also pins the no-TYPE_CHECKING premise the sweep rests on, and guards against
passing vacuously on an empty member list.
Suite: 3611 passed, 2 skipped. Two unrelated failures
(test_psychometric_routing, test_spend_analytics) reproduce identically on
unmodified origin/main in this container and are environmental: fast_mlsirm is
a `python_full_version >= '3.12'` dependency and this container runs 3.11.15,
and tiktoken is absent, which moves usage_source from "mixed" to "tokenizer".
interrogate: 100%.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KPmJErfkcHer4UVEgrQxUX
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (2)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthrough
ChangesAnnotation resolution 보강
Priority: ⬇️ Low Estimated code review effort: 2 (Simple) | ~10 minutes Change: Bug fix Merge Risk: ⚪ Minimal · up to The annotation-resolution regression is addressed and covered by focused tests; no actionable merge risk remains. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
The three
|
Correction to one clause above — the cross-repo diagnosis holds, the comparison to
|
|
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
2 participants
The bug
review_pool_admissionsis annotatedSequence[Any], butreview_gatewayimports onlyMappingandSequencefromtyping.from __future__ import annotationskeeps annotations as strings, so the module imports cleanly and every existing test passes. The break surfaces only for a caller that resolves the annotations:That reaches any runtime type checker, dataclass/schema generator, docs builder, or validation layer that introspects this module — which matters here specifically, because
review_pool_admissionsis the owner-side provenance projection a consumer is meant to read instead of re-deriving admission itself. Its own docstring says the point is that a caller sends only the gateway token andmodel: orchestrator/free. A consumer building that integration against a typed contract is exactly the caller that resolves these hints.Scope is one name, deliberately
A package-wide
get_type_hintssweep flags three modules. I checked each rather than fixing what the sweep printed:review_gatewayAnytypingname, never imported, noTYPE_CHECKINGblockmodel_discoveryPriceBookif TYPE_CHECKING: from .cost_ledger import PriceBook, annotations properly quotedprovider_catalog_storePrivacyPolicyAssessmentThe latter two are the documented circular-import remedy, whose cost is precisely that
get_type_hintscannot resolve them. Fixing them would mean undoing the remedy. Left alone.review_gatewaydeclares noTYPE_CHECKINGblock, so it has no name it is entitled to leave unresolvable — which is what makes the sweep below a sound contract for this module and not for those.Test
tests/test_review_gateway_annotation_resolution.pysweeps every function and class the module defines and asserts its hints resolve, so the next such name is caught rather than waiting for a consumer to hit it. It also:TYPE_CHECKINGpremise the sweep rests on, so the contract fails loudly if the module ever adopts deferred imports rather than silently becoming wrong;Verified it actually fails without the one-line fix:
Verification
Two unrelated failures (
test_psychometric_routing,test_spend_analytics) reproduce identically on unmodifiedorigin/mainin this container and are environmental, not regressions:fast_mlsirmis apython_full_version >= '3.12'dependency and this container runs Python 3.11.15, andtiktokenis absent, which movesusage_sourcefrom"mixed"to"tokenizer". CI installs the full lock on the right interpreter.Context
Found while validating
ContextualWisdomLab/.github#2137, which advances the central review sidecar's vendored pin to012beaacand inspects every symbol the sidecar imports from this package at that revision.🤖 Generated with Claude Code
https://claude.ai/code/session_01KPmJErfkcHer4UVEgrQxUX
Generated by Claude Code
Summary by CodeRabbit
버그 수정
테스트