fix: bind Unknown-typed store results silently under gradual typing (#588) - #589
Conversation
…588) `store x as <call>` emitted an ERROR-level "Could not infer type for variable 'x'" whenever the callee's return type was statically `Unknown` — the normal case for any action that returns an expression built from its untyped parameters. The program ran correctly (exit 0); only the static diagnostic was wrong, and it was extremely noisy (104 of 106 diagnostics on a fully-correct Scribe run). Under gradual typing, an inferred `Unknown` means "statically unknown", not "known incompatible" — mirroring #587's treatment of variable references. The `VariableDeclaration` arm now binds the symbol as `Unknown` and continues instead of raising a type_error. The type-compatibility and symbol-recording paths that follow are unchanged and still record a more specific type when one is available, so the now-dead container-property/known-symbol suppression block is removed. Adds regression tests covering the minimal repro (prints 4, no error) and the chained Scribe-style helpers (prints [[x]]). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BPdEBReyK6s9FWYQjwkJvk
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Warning Review limit reached
Next review available in: 26 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe type checker no longer emits a "Could not infer type for variable" error when a variable initializer's inferred type is Unknown, treating it as statically unknown under gradual typing. Two new regression tests verify silent binding for Unknown-returning function calls, including a chained call scenario. ChangesUnknown Type Binding Fix
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/github_issues_batch_test.rs`:
- Around line 366-381: The test store_unknown_call_result_chained_binds_silently
is using wrap, which returns Text via concatenation, so it does not actually
exercise the Unknown-typed bind path. Update the helper in this test to use a
genuinely Unknown-returning action or expression, then keep the chained store
calls in go so the regression around silent Unknown binds is truly covered.
Preserve the existing assertions, but make sure the symbols wrap and go still
clearly locate the scenario being tested.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 22c8de53-1e14-49bc-bf60-e9fd73f3b36f
📒 Files selected for processing (2)
src/typechecker/mod.rstests/github_issues_batch_test.rs
The chained `store_unknown_call_result_chained_binds_silently` test used a `wrap` helper that returned `"[" with s with "]"`. Concatenation always infers as `Text`, so `wrap`'s return type inferred to `Text` (concrete), not `Unknown` — the test passed even against pre-fix code and did not actually cover the silent-Unknown-bind regression. Rewrite `wrap` so its `otherwise` branch returns the untyped parameter `s` (statically `Unknown`), which widens the action's inferred return type to `Unknown` while the taken branch still bracket-wraps at runtime. Verified: against pre-fix code this now flags both `a` and `b` with `Could not infer type for variable`; with the fix it type-checks clean and still prints `[[x]]`. Existing assertions and the `wrap`/`go` symbols are preserved. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BPdEBReyK6s9FWYQjwkJvk
* fix: seed action provisional return type as Unknown so self-recursion type-checks (#590) A self-recursive action that used its own recursive result inside its body (e.g. indexed it) got a false `Cannot index into Nothing` diagnostic. The body is type-checked before the real return type is inferred (#575's ordering), and the provisional return type was seeded as `Nothing`, so a self-reference in the body resolved to `Nothing` and any use/indexing of it raised strict "found Nothing" errors. Seed the provisional return type as `Unknown` instead. After #588/#589 an `Unknown`-typed value degrades gracefully, so self-references resolve cleanly during the body check while post-body inference (#575) still records the concrete return type for external callers. Void actions are still recorded as `Nothing` externally, preserving existing behavior. Adds regression tests covering the reported repro and the Scribe `scribe_p_unary` shape that negates its recursive result. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018Qykg1eQ2bJKx2uoJNBGPj * test: consolidate recursive-action regression tests and tighten assertion (#590) Address review feedback on PR #591: extract the shared lex/parse/typecheck flow into `assert_typechecks_clean`, and assert the programs type-check with zero diagnostics (`result.is_ok()`) instead of only checking for the absence of one error substring. The tighter guard catches both a re-introduced "Cannot index into Nothing" error and any new spurious diagnostic on the recursive path. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018Qykg1eQ2bJKx2uoJNBGPj --------- Co-authored-by: Claude <noreply@anthropic.com>
store x as <call>emitted an ERROR-level "Could not infer type forvariable 'x'" whenever the callee's return type was statically
Unknown— the normal case for any action that returns an expression built from
its untyped parameters. The program ran correctly (exit 0); only the
static diagnostic was wrong, and it was extremely noisy (104 of 106
diagnostics on a fully-correct Scribe run).
Under gradual typing, an inferred
Unknownmeans "statically unknown",not "known incompatible" — mirroring #587's treatment of variable
references. The
VariableDeclarationarm now binds the symbol asUnknownand continues instead of raising a type_error. Thetype-compatibility and symbol-recording paths that follow are unchanged
and still record a more specific type when one is available, so the
now-dead container-property/known-symbol suppression block is removed.
Adds regression tests covering the minimal repro (prints 4, no error)
and the chained Scribe-style helpers (prints [[x]]).
Co-Authored-By: Claude Opus 4.8 noreply@anthropic.com
Claude-Session: https://claude.ai/code/session_01BPdEBReyK6s9FWYQjwkJvk
Summary by CodeRabbit