Skip to content

Implement user-defined action overloading - #639

Merged
logbie merged 19 commits into
mainfrom
claude/function-overload-resolution-xizxjb
Jul 21, 2026
Merged

Implement user-defined action overloading#639
logbie merged 19 commits into
mainfrom
claude/function-overload-resolution-xizxjb

Conversation

@logbie

@logbie logbie commented Jul 20, 2026

Copy link
Copy Markdown
Collaborator

Summary

This change implements full support for user-defined action overloading, allowing actions to be defined multiple times with the same name in the same scope when they differ in parameter count or declared parameter types. Calls dispatch automatically on argument count and runtime argument types.

Key Changes

Analyzer (PASS 1):

  • Added SignatureConflict enum and signature_conflict() function to detect exact duplicates and indistinguishable same-arity pairs at definition time
  • Added format_signature() and format_param_type() helpers for diagnostic messages
  • Modified action definition handling to merge same-scope redefinitions into overload sets instead of erroring
  • Implemented check_overloaded_call() to validate calls against multiple signatures: filters by arity, then by static argument types, with detailed error messages listing candidates
  • Added signature_accepts() to check if a signature could accept a call without reporting errors (used for filtering)
  • Added check_call_against_signature() for full validation of a single resolved signature

Interpreter:

  • Added Value::Overloaded(Rc<OverloadedFunction>) variant to wrap overload sets in definition order
  • Extended FunctionValue with param_types: Vec<Option<Type>> to track declared parameter types for runtime dispatch
  • Implemented Environment::define_or_merge_action() to merge same-scope action redefinitions, enforcing the same overload rules as the analyzer
  • Added select_overload() to dispatch at runtime: filters by arity, drops candidates whose concrete parameter types reject argument values, picks the most specific match (ties resolve to definition order)
  • Modified bare overloaded name references to auto-call the zero-argument overload (matching single-function behavior)
  • Updated main entry point to handle overloaded main by running its zero-argument overload if present

Type Checker:

  • Added overload_returns HashMap to track inferred return types per overload (keyed by action name and signature index)
  • Implemented action_signatures() to retrieve registered signatures for a name
  • Implemented signature_index_for() to identify which overload a definition corresponds to
  • Implemented infer_overloaded_call_type() to resolve call types against multiple signatures, mirroring the analyzer's filtering logic
  • Updated call type inference to use overload machinery when multiple signatures exist

Parser:

  • Added type_from_token() helper to map tokens in type position to Type (handles keywords like text and pattern that lex as keywords rather than identifiers)

Documentation & Tests:

  • Added comprehensive analyzer tests (tests/overload_analyzer_test.rs) covering definition-time rules, call-site resolution, and error messages
  • Added comprehensive interpreter tests (tests/overload_interpreter_test.rs) covering runtime dispatch by arity and type, closures, recursion, and error handling
  • Added comprehensive type checker tests (tests/overload_typechecker_test.rs) covering per-overload return type inference and deferred calls
  • Added TestPrograms/action_overloading_comprehensive.wfl end-to-end test
  • Updated language documentation (Docs/03-language-basics/actions-functions.md) with overloading rules and examples
  • Updated syntax and language specification references
  • Added dev diary entry explaining design decisions

Implementation Details

Definition-time strictness: Two same-name definitions are rejected when they are exact duplicates (same arity, same normalized types) or when no parameter position has both versions declaring concrete, different types. This rule is enforced identically in the analyzer and runtime to ensure include-driven or dynamically-constructed programs get consistent behavior.

Deferred static resolution: At a call site the analyzer filters signatures by arity, then by static argument types. Zero survivors produces an error listing candidates; exactly one gets full validation; several survivors (only possible with Unknown-typed arguments) defer silently to runtime dispatch.

Runtime dispatch: Value::Overloaded wraps the overload set in definition order. Dispatch filters by arity, drops candidates whose concrete parameter types reject an argument value, then picks the candidate with the most concrete type match (ties resolve to definition order).

Return type tracking: The type checker maintains a per-

https://claude.ai/code/session_01SdLFM2t8Efd89LbaFh7Mkm


Open in Devin Review

Summary by CodeRabbit

  • New Features
    • Added same-scope action overloading by arity and declared parameter types, with most-specific overload selection.
    • Stored action references now snapshot the visible overload set at binding time, including zero-argument auto-invocation.
    • Container-typed overload dispatch supports inheritance-based matches.
  • Documentation
    • Expanded language basics and reference spec with typed-parameter overloading rules and updated examples.
  • Bug Fixes
    • Improved correctness of overload dispatch and runtime parameter-type enforcement across control-flow boundaries and alias mutations.
    • Refined typechecking to track inferred returns per overload.
  • Tests
    • Added comprehensive end-to-end and diagnostic coverage; updated expectations for earlier error wording.
  • Known Limitations
    • Container methods still do not support overloading.

claude added 3 commits July 20, 2026 09:04
…dispatch

Actions may now be defined more than once with the same name in the same
scope, dispatching on argument count and declared parameter types. This
implements the analyzer's long-standing TODO ("Implement proper overload
resolution based on argument types and count") and extends it to full
user-defined overloading across the pipeline.

- Analyzer: same-scope action redefinitions merge into the existing
  Vec<FunctionSignature>; exact duplicates and indistinguishable same-arity
  pairs (no position where both declare concrete, different types) are
  definition-time errors. Both call forms (`of` and `call ... with`) resolve
  through a shared checker that filters candidates by arity then by static
  argument types, with Elm-style candidate-listing errors; statically
  ambiguous calls defer to runtime silently. The `of` form now gets the same
  named-argument and type validation the `call` form already had.
- Typechecker: per-overload inferred return types recorded in a side table
  keyed by (name, signature index) and resolved at call sites; deferred
  calls take the common return type of surviving candidates, else Unknown.
- Interpreter: new Value::Overloaded wraps the overload set in definition
  order; FunctionValue now carries declared parameter types (previously
  erased). Runtime dispatch filters by arity, then by argument values
  against concrete parameter types, preferring the most concretely-matched
  candidate (ties resolve to definition order). Zero-arg auto-call, main
  entry, exports, and first-class function values handle overload sets.
- Parser: `as text` / `as pattern` parameter annotations now parse (these
  type names lex as keywords, which the type position previously rejected).
- Container methods intentionally do not overload yet (documented).

Docs: new overloading section in actions-functions.md, normative dispatch
rules in the language specification, syntax-reference example, Dev Diary
entry. Tests: 29 new cases across analyzer/typechecker/interpreter suites
plus TestPrograms/action_overloading_comprehensive.wfl.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SdLFM2t8Efd89LbaFh7Mkm
- Replace the first-class-value overloading example (the analyzer has never
  accepted calling a function-valued variable, for single actions either)
  with a direct overloaded-call example, in both the docs and the
  comprehensive TestProgram.
- Fix pre-existing syntax-reference bug: `add_numbers with 5 and 3` is
  concatenation, not a call — it printed "action add_numbersyes" instead
  of 8. Use the `of` call form.

All snippets and TestPrograms/action_overloading_comprehensive.wfl verified
end-to-end with target/release/wfl.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SdLFM2t8Efd89LbaFh7Mkm
Add TODO(#638) markers at the four sites a future implementation must
touch (analyzer MethodInfo registration x2, interpreter method insert,
ContainerDefinitionValue method maps) and link the issue from the docs
callout and Dev Diary.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SdLFM2t8Efd89LbaFh7Mkm
Copilot AI review requested due to automatic review settings July 20, 2026 10:08
@coderabbitai

coderabbitai Bot commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Warning

Review limit reached

@logbie, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 18 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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 configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 9a4b39a6-3263-44d6-b14a-f822166f639b

📥 Commits

Reviewing files that changed from the base of the PR and between 82444a3 and 5937006.

📒 Files selected for processing (10)
  • Dev diary/2026-07-20-action-overloading.md
  • Docs/03-language-basics/actions-functions.md
  • Docs/reference/language-specification.md
  • Docs/reference/syntax-reference.md
  • src/analyzer/mod.rs
  • src/interpreter/mod.rs
  • src/typechecker/mod.rs
  • tests/execution_budget_test.rs
  • tests/include_of_form_resolution_test.rs
  • tests/overload_test.rs
📝 Walkthrough

Walkthrough

Adds same-scope user-defined action overloading with typed and arity-based dispatch, analyzer and typechecker resolution, runtime overload selection, stored-action support, parser updates, documentation, and comprehensive regression coverage.

Changes

Action overloading

Layer / File(s) Summary
Overload contracts and syntax
src/interpreter/value.rs, src/parser/stmt/actions.rs
Adds overload values, declared parameter types, overload formatting/equality behavior, and shared type-token parsing.
Analyzer registration and call validation
src/analyzer/mod.rs
Registers distinguishable overloads, rejects conflicts, tracks aliases through control flow, and validates calls by arity and static argument types.
Per-overload return inference
src/typechecker/mod.rs
Stores inferred return types per overload and resolves call result types for both call forms and aliases.
Runtime overload binding and dispatch
src/interpreter/environment.rs, src/interpreter/mod.rs
Merges same-scope actions, scopes runtime type enforcement, selects matching overloads, and supports zero-argument auto-calls.
Overload behavior coverage
tests/overload_test.rs, TestPrograms/*, src/interpreter/memory_tests.rs
Adds analyzer, typechecker, interpreter, full-pipeline, comprehensive-program, memory, and regression coverage.
Overload semantics and examples
Docs/*, Dev diary/*
Documents declaration rules, dispatch, compatibility, limitations, examples, and implementation findings.

Estimated code review effort: 5 (Critical) | ~90 minutes

Possibly related PRs

Suggested reviewers: copilot

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely summarizes the main change: adding user-defined action overloading.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch claude/function-overload-resolution-xizxjb

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 090ad3ef8b

ℹ️ 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".

Comment thread src/interpreter/mod.rs
Comment thread src/interpreter/mod.rs
.await?,
);
}
let func = Self::select_overload(&overloaded, &arg_values, *line, *column)?;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Respect named arguments during overload dispatch

For overloaded action calls with named arguments, dispatch receives only arg_values in source order, so a call such as call f with b: 1 and a: "x" against overloads (a as number, b as text) and (a as text, b as number) is accepted by the analyzer for the second signature but runtime selects/binds as if the values were positional. Named arguments need to be mapped to the selected signature's parameter order before calling select_overload/call_function, otherwise overload resolution can run the wrong body.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Named call arguments are not producible from WFL source today: the parser never constructs Argument { name: Some(_) } — every real construction site (12 across src/parser/expr/* and src/parser/stmt/*) passes name: None; only hand-built ASTs in unit tests set a name. So runtime dispatch cannot receive named arguments, and the analyzer's named-argument mapping is defensive only. The pre-existing single-function call path has the same positional-only behavior.

Agreed the mapping will be needed if named-call syntax lands — deferring the runtime change until then rather than adding dead reordering logic now. (The sibling comment about zero-arg auto-call during callee resolution was real and is fixed in be2a1c9.)


Generated by Claude Code

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 2 potential issues.

Open in Devin Review

Comment thread src/typechecker/mod.rs
Comment thread src/analyzer/mod.rs
Comment on lines +2996 to +3004
} else {
self.check_overloaded_call(
name,
&signatures,
arguments,
true,
*line,
*column,
);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔍 of-form calls now get type validation they never had

The old of-form (FunctionCall with a bare Variable callee) only performed an arity check on the first signature and never validated argument types (src/analyzer/mod.rs old lines 2600-2628). The new path routes both of and call forms through check_overloaded_call -> check_call_against_signature, which now emits per-argument type-compatibility errors for the of form too. This is called out as intentional in the dev diary, and the unit test test_function_call_type_checking was updated. However, it is a behavior change: an existing program that passed a statically-concrete argument whose type mismatches a declared parameter type via the of form (e.g. f of some_text where f takes x as number) would now fail static analysis where it previously ran. is_type_compatible is lenient (Unknown/Any/Nothing pass), so only concrete mismatches are affected. Worth confirming all TestPrograms/ still pass given the repo's backward-compatibility rule.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adds end-to-end support for user-defined action overloading in WFL: multiple same-name action definitions in the same scope can coexist when distinguishable by arity and/or declared parameter types, with calls resolved via analyzer/typechecker filtering and interpreter runtime dispatch.

Changes:

  • Implemented overload-set signature registration/validation in the analyzer, including call-site candidate filtering and improved diagnostics.
  • Added runtime representation (Value::Overloaded) and overload selection logic in the interpreter, plus environment support for merging same-scope redefinitions.
  • Extended the typechecker to track per-overload inferred return types and resolve overloaded call expression types; added comprehensive tests, docs, and an end-to-end TestPrograms case.

Reviewed changes

Copilot reviewed 15 out of 15 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
src/analyzer/mod.rs Adds signature conflict detection, overload signature formatting, and overload-aware call validation in analyzer pass.
src/typechecker/mod.rs Tracks per-overload inferred return types and resolves call types against multiple signatures.
src/interpreter/value.rs Introduces overloaded function runtime value representation and stores declared param types for dispatch.
src/interpreter/environment.rs Adds same-scope action merge logic to build overload sets and enforce distinctness rules at runtime.
src/interpreter/mod.rs Implements runtime overload dispatch selection and preserves zero-arg auto-call behavior for overloaded names (incl. main).
src/interpreter/memory_tests.rs Updates interpreter memory tests to account for param_types on FunctionValue.
src/parser/stmt/actions.rs Fixes typed parameter parsing for keyword-lexed types via type_from_token (e.g., text, pattern).
tests/overload_analyzer_test.rs Adds analyzer tests for overload definition rules and call-site resolution/diagnostics.
tests/overload_typechecker_test.rs Adds typechecker tests for per-overload return inference and deferred/forward-reference behavior.
tests/overload_interpreter_test.rs Adds interpreter tests for runtime dispatch, errors, recursion, and overload value behavior.
TestPrograms/action_overloading_comprehensive.wfl Adds an end-to-end WFL program covering overload scenarios.
Docs/03-language-basics/actions-functions.md Documents overloading rules, dispatch semantics, and current limitations (e.g., container methods).
Docs/reference/syntax-reference.md Updates syntax reference examples to include typed overload usage.
Docs/reference/language-specification.md Adds specification text describing overload validity and dispatch rules.
Dev diary/2026-07-20-action-overloading.md Captures design decisions, constraints, and test coverage for the feature.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/analyzer/mod.rs

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🧹 Nitpick comments (1)
src/parser/stmt/actions.rs (1)

17-24: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Divergent type-name casing across the three type parsers.

type_from_token recognizes lowercase primitive spellings (text, number, …), but parse_container_action_definition (Lines 397-404) and parse_parameter_list (Lines 475-482) recognize only capitalized spellings (Text, Number, …), mapping everything else to Type::Custom. This means a top-level action accepts as text while a container method with : Text needs a capital letter, and x as text inside parse_parameter_list would silently become Type::Custom("text"). Consider consolidating onto a single case-insensitive helper to avoid the inconsistency, even while container overloading remains deferred (#638).

🤖 Prompt for 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.

In `@src/parser/stmt/actions.rs` around lines 17 - 24, Consolidate primitive
type-name parsing across type_from_token, parse_container_action_definition, and
parse_parameter_list by reusing a shared case-insensitive helper. Ensure text,
number, boolean, nothing, and pattern map to their corresponding primitive Type
variants regardless of casing, while unrecognized names remain Type::Custom with
the original name.
🤖 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 `@Dev` diary/2026-07-20-action-overloading.md:
- Around line 69-72: Update the bullet’s wording to state that typed parameter
annotations using “as text” are now parsed via type_from_token, while “returns
text” remains unsupported; remove the contradictory claim that both forms never
parsed.

In `@src/analyzer/mod.rs`:
- Around line 70-79: Update format_param_type to add an explicit Type::Pattern
arm returning the lowercase source spelling "pattern", keeping the existing
fallback for other unsupported types unchanged.

In `@src/parser/stmt/actions.rs`:
- Around line 13-27: Add direct handling for Token::NothingLiteral in
type_from_token so it returns Type::Nothing, while preserving the existing
identifier and keyword mappings.

---

Nitpick comments:
In `@src/parser/stmt/actions.rs`:
- Around line 17-24: Consolidate primitive type-name parsing across
type_from_token, parse_container_action_definition, and parse_parameter_list by
reusing a shared case-insensitive helper. Ensure text, number, boolean, nothing,
and pattern map to their corresponding primitive Type variants regardless of
casing, while unrecognized names remain Type::Custom with the original name.
🪄 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: 1bc9149c-2618-495b-b0a9-18beac281df3

📥 Commits

Reviewing files that changed from the base of the PR and between 7d24ae3 and 090ad3e.

📒 Files selected for processing (15)
  • Dev diary/2026-07-20-action-overloading.md
  • Docs/03-language-basics/actions-functions.md
  • Docs/reference/language-specification.md
  • Docs/reference/syntax-reference.md
  • TestPrograms/action_overloading_comprehensive.wfl
  • src/analyzer/mod.rs
  • src/interpreter/environment.rs
  • src/interpreter/memory_tests.rs
  • src/interpreter/mod.rs
  • src/interpreter/value.rs
  • src/parser/stmt/actions.rs
  • src/typechecker/mod.rs
  • tests/overload_analyzer_test.rs
  • tests/overload_interpreter_test.rs
  • tests/overload_typechecker_test.rs

Comment thread Dev diary/2026-07-20-action-overloading.md Outdated
Comment thread src/analyzer/mod.rs
Comment thread src/parser/stmt/actions.rs
- of-form calls no longer auto-call a zero-argument overload while
  resolving the callee: a bare-Variable callee naming an overload set is
  used directly as the call target, so 'g of 5' dispatches instead of
  calling g's zero-arg version and then failing to call its result
- clear the per-overload return-type table at the start of check_types so
  a reused TypeChecker (editor/LSP session) never resolves calls against
  a previous program's recorded returns
- accept 'nothing' (NothingLiteral token) in type position, so
  'as nothing' parameter annotations parse
- render Pattern and Any parameter types in surface syntax in overload
  diagnostics instead of Debug output
- reword the Dev Diary bullet to distinguish the fixed 'as text'
  parameter parsing from the still-unsupported 'returns <type>' clause

Named-argument dispatch (also raised in review) is unreachable today:
the parser never constructs named Arguments outside unit tests, so no
runtime change is made for it.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SdLFM2t8Efd89LbaFh7Mkm
Copilot AI review requested due to automatic review settings July 20, 2026 10:41

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 15 out of 15 changed files in this pull request and generated 1 comment.

Comment thread src/parser/stmt/actions.rs
…or overloads

Copilot review follow-up: 'any' lexes as KeywordAny, so type_from_token
rejected 'x as any'. Accept the keyword (and 'any' identifier) mapping to
Type::Any — and since an 'any' annotation accepts every value, treat it
(and Unknown) like an untyped parameter in the overload distinctness rule
(analyzer + runtime) and in dispatch specificity, so 'f(x as any)' vs
'f(x as number)' is rejected as ambiguous instead of dispatching
unpredictably.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SdLFM2t8Efd89LbaFh7Mkm
Copilot AI review requested due to automatic review settings July 20, 2026 11:07

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 15 out of 15 changed files in this pull request and generated 1 comment.

Comment thread src/parser/stmt/actions.rs Outdated
…nking

CI on this branch failed twice with 'ld terminated with signal 7 [Bus
error]' (runner out of disk while linking) — main is green, and the
tipping factor is the three new integration-test crates, each linking
its own copy of the debug-info-heavy wfl library. Merge them into a
single tests/overload_test.rs (analyzer/typechecker/interpreter
modules), cutting the new test binaries from three to one. All 32 tests
unchanged and passing.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SdLFM2t8Efd89LbaFh7Mkm
Copilot AI review requested due to automatic review settings July 20, 2026 11:13
Copilot review follow-up: 'as Text' parsed as Custom("Text"), which the
analyzer tolerates (case-insensitive compatibility) but runtime overload
dispatch treated as a container type and failed to match. Normalize the
identifier to lowercase for primitive names (text/number/boolean/
nothing/pattern/any); true custom types keep their original spelling.
Adds a dispatch test using capitalized annotations.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SdLFM2t8Efd89LbaFh7Mkm

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated 1 comment.

Comment thread src/typechecker/mod.rs
Comment on lines +377 to +389
let compatible: Vec<usize> =
arity_matches
.iter()
.copied()
.filter(|&i| {
signatures[i].parameters.iter().zip(arg_types.iter()).all(
|(param, arg_type)| {
let param_type =
param.param_type.as_ref().cloned().unwrap_or(Type::Unknown);
self.are_types_compatible(&param_type, arg_type)
},
)
})

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same situation as the runtime-dispatch thread (#639 (comment)): named call arguments are not producible from WFL source — the parser never constructs Argument { name: Some(_) } (all real construction sites pass name: None), so infer_overloaded_call_type can only ever see positional arguments. The analyzer's name-aware signature_accepts is defensive. Deferring name-aware mapping here until named-call syntax exists, at which point the analyzer/typechecker/runtime paths should all gain it together.


Generated by Claude Code

Copilot AI review requested due to automatic review settings July 20, 2026 11:18

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated no new comments.

@logbie logbie left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Deep review — merge blockers found

Reviewed current head 769e898. GitHub does not permit this account to submit “Request changes” on its own PR, so this comment records the equivalent review verdict: changes required before merge.

1. P1 — Custom/container-typed overloads are rejected before runtime

Runtime dispatch explicitly supports Type::Custom and descendant container instances, but static overload filtering does not consider Type::Custom("Dog") compatible with the inferred Type::ContainerInstance("Dog"). The analyzer's is_type_compatible and the type checker's are_types_compatible both lack this pairing, so exact custom-type and inherited-type calls can be rejected before the runtime logic at value_matches_type ever runs.

Minimal case:

define action called describe with parameters value as Dog:
    return "dog"
end action

define action called describe with parameters value as Cat:
    return "cat"
end action

create instance rover from Dog
display describe of rover

Please add exact custom/container matching, descendant-to-parent compatibility through the container registry, and full-pipeline tests for exact and inherited dispatch.

2. P1 — nothing passes static resolution but fails runtime dispatch

The analyzer and type checker intentionally accept Nothing as compatible with every target type, but runtime overload matching accepts null only for an explicit as nothing parameter.

define action called f with parameters x as number:
    return "number"
end action

define action called f with parameters x as text:
    return "text"
end action

display f of nothing

This passes the static stages and then fails with “No version matches.” It is also a compatibility regression: a single typed action previously received nothing, while adding another overload changes that call into a runtime error. Analyzer, type checker, and interpreter need one consistent rule. Under the existing compatibility semantics, null should likely remain compatible with each candidate, with the documented definition-order tie break.

3. P2 — First-class overload sets lose their signatures in the type checker

Runtime supports storing an overload set and calling it through another variable:

store h as f
store result as h of 5

However, infer_expression_type only invokes overload resolution when the callee name directly resolves to an action symbol. After assignment to h, it is an ordinary variable whose Type::Function describes only the most recently checked overload because each action definition overwrites symbol.symbol_type. Calling a different overload through h therefore emits a false type warning and can infer the wrong return type.

The current first-class-value coverage is interpreter-only, so it misses this. Please propagate overload-set type information through variable declarations and add a full analyzer/type-checker/interpreter regression test.

Additional notes

Current CI is green. Several still-open automated threads are already addressed in the current head (zero-argument callee handling, pattern, any, case normalization, and clearing overload_returns) and can be resolved after verification. Named-call arguments are not source-reachable today, so those threads are future-proofing rather than current blockers.

logbie commented Jul 20, 2026

Copy link
Copy Markdown
Collaborator Author

Deep review — round 2

Reviewed current head 769e898. The head has not changed since the previous deep-review comment, so its three findings remain open. This pass found two additional behavioral defects.

1. P1 — A call made before the second overload definition can execute the wrong body

PASS 1 registers every top-level signature before analysis, so the call below resolves statically to the later text overload:

define action called choose with parameters value as number:
    return "number body"
end action

store selected as choose of "hello"

define action called choose with parameters value as text:
    return "text body"
end action

display selected

At runtime, however, only the first definition exists when the call executes. The environment still stores it as Value::Function, not Value::Overloaded, so both FunctionCall and ActionCall bypass select_overload. call_function checks arity but does not enforce param_types, and the number overload runs with a text argument. The program therefore produces "number body" even though static overload resolution selected the text signature.

This contradicts the dev-diary statement that calls operate on “the overloads defined so far”: with one defined candidate whose type does not match, the runtime should reject the call, not execute that candidate. It can also run arbitrary side effects from the wrong action body.

Please add a full-pipeline regression test with a call interleaved between overload definitions and ensure type-based matching is applied even while only one member of a future overload set has been defined.

Relevant paths:

  • src/analyzer/mod.rs: PASS 1 registers all top-level signatures.
  • src/interpreter/mod.rs: Value::Function call branches invoke call_function directly.
  • src/interpreter/mod.rs: call_function validates argument count only.

2. P2 — An overloaded action value is not equal to itself

Value::PartialEq preserves pointer-identity equality for ordinary Value::Function values, but neither its fast path nor eq_with_visited handles Value::Overloaded. It falls through to false, even when both sides contain the same Rc<OverloadedFunction>.

define action called f with parameters x as number:
    return "number"
end action

define action called f with parameters x as text:
    return "text"
end action

store alias as f
display alias is equal to alias

This prints no, whereas a stored non-overloaded action compares equal to itself. That violates the documented rule that overloads behave like one action and can break identity checks after adding a second overload.

Please add Value::Overloaded(a), Value::Overloaded(b) => Rc::ptr_eq(a, b) consistently to both equality paths and cover self-identity plus distinct-overload-set comparisons.

Verdict

Changes still required before merge. Current CI is green, but existing tests do not exercise either temporal dispatch or overload-set identity.

Round 1:
- container-typed overloads: params annotate as Custom("Dog") but
  instances infer as ContainerInstance("Dog"); add
  Analyzer::container_is_or_extends (depth-guarded extends walk) and
  Custom<->ContainerInstance ancestry arms to both is_type_compatible and
  are_types_compatible, so exact and inherited container dispatch pass
  static analysis (runtime already matched via the parent-instance chain)
- nothing arguments: value_matches_type now accepts Null/Nothing for
  every parameter type, mirroring the static (_, Nothing) => true rule;
  ties fall to definition order
- stored actions: new analyzer action_aliases map records bare
  'store h as f' references (cleared on reassignment); analyzer call arms
  and typechecker call paths resolve through the alias, so calls through
  a stored action get full overload resolution and return types

Round 2:
- temporal dispatch: call_function now enforces declared parameter types
  (nothing/any/untyped still accept everything), so a call interleaved
  between overload definitions errors instead of silently running the
  lone first overload's body with a non-matching argument
- Value::Overloaded equality: Rc::ptr_eq arms in both the PartialEq fast
  path and eq_with_visited, so an overload set equals itself

Adds full-pipeline (analyze -> typecheck -> interpret) regression tests
for all five findings, extends the comprehensive TestProgram, and
updates the actions guide, language spec, and Dev Diary.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SdLFM2t8Efd89LbaFh7Mkm
Copilot AI review requested due to automatic review settings July 20, 2026 13:36

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated no new comments.

logbie commented Jul 20, 2026

Copy link
Copy Markdown
Collaborator Author

Fifth-pass review of 6a2ab25. The three round-4 findings are fixed, and CI is green. I found two remaining P1 correctness gaps:

  1. [P1] Include existing same-scope actions when entering a nested statement blockscan_block_overload_dups only marks names occurring twice in the immediate slice. Consider a top-level f(number), then an executed check if block which calls the existing f with a runtime-unknown value before defining f(text) later in that same block. The branch executes against the same environment, so that later definition merges with the outer member; however, the branch scan sees only one local f, leaves the existing member's enforce_param_types false, and the interleaved call can run the number body with a text value. The merge flips the flag only after the bad call. At block entry, definitions that will merge with a function/overload already present in the current environment must also mark the existing members for enforcement (without treating inherited names in a child environment as mergeable, since those remain shadowing errors).

  2. [P1] Account for abrupt exits/intermediate states in alias flow joins — the new FlowState join compares only construct entry and the state after analyzing the entire body. A body can restore an alias lexically after a break (or after a statement that throws into a when handler), making those endpoints equal even though runtime exits at the intermediate binding. Repro shape: bind h = f; in a one-iteration loop do change h to g; break; change h to f; then call h. Analysis walks past break, sees body-final h = f, joins it with entry h = f, and records the call as bound to f; runtime leaves h = g. If f accepts the call but overloaded g does not, static analysis accepts a call that runtime rejects. The same endpoint-only issue affects try bodies whose error occurs after an alias change but before a later restoration. Flow analysis needs to collect states at break/continue and potential handler-transfer points, or conservatively degrade aliases mutated in such bodies to Dynamic.

I reviewed the full delta from dc72754; the new nothing specificity rule and its explicit-nothing exception are internally consistent.

Addresses the maintainer's fifth-pass review (two P1 findings):

1. Cross-block merges no longer leave a temporal window: entering a
   statement block arms enforce_param_types on same-scope existing
   function members for every name the block defines
   (enter_block_overloads, wired through _execute_block, the top-level
   program loop, and the describe/test executors), so an interleaved
   dynamic call before the in-block definition dispatches strictly.
   Inherited names stay untouched — defining over them is a shadowing
   error, not a merge.

2. Alias flow joins now account for abrupt exits: the analyzer tracks
   which alias names are written at any point inside each loop/try body
   (alias_mutation_frames; popping merges into the parent frame) and
   degrades exactly those names to AliasState::Dynamic after loop joins
   and at try-handler entry. A break/continue or handler transfer can
   expose an intermediate binding the body's endpoint state has already
   restored — previously that made static analysis reject calls the
   runtime accepts. Untouched names keep precise state; branch
   constructs keep endpoint joins (no mid-branch exit); overload
   counters are monotonic so endpoint joins already catch them.

Three new red-first regressions (60 total in tests/overload_test.rs),
a TestProgram loop-reassignment section, and docs/spec/diary updates in
the same change.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SdLFM2t8Efd89LbaFh7Mkm
Copilot AI review requested due to automatic review settings July 20, 2026 17:34

logbie commented Jul 20, 2026

Copy link
Copy Markdown
Collaborator Author

Fifth-pass findings addressed in 82444a3 — both fixed, red-first (three new regressions, 60 total in tests/overload_test.rs):

1. Block-entry arming for cross-block merges (P1) — New enter_block_overloads replaces the bare duplicate scan at every block-entry site (_execute_block, the top-level program loop, and the describe/test executors): besides computing the block's own duplicate set, it arms enforce_param_types on any same-scope existing function member whose name the entering block defines — the coming merge makes that member overloaded, so its temporal window now starts when the block starts, not at the later merge. Your exact scenario (executed branch, interleaved dynamic call, then the merging definition) is the regression: the call now hits the temporal dispatch rejection instead of running the number body with a text value. Only the local scope is consulted (get_local), so inherited names — where a definition is a shadowing error, not a merge — are untouched; the top-level site also covers a REPL interpreter reused across snippets. Arming is lexical, consistent with the established "defined more than once in the block" rule.

2. Abrupt-exit-aware alias joins (P1) — I took your "conservatively degrade" option, but scoped precisely: the analyzer keeps a stack of alias-mutation frames, one per loop/try body, recording every alias name written at any point in that body (popping a frame merges it into the parent, so an inner body's mutation counts for the outer one — which also attributes a break inside nested constructs correctly without exit-point bookkeeping). After each loop join, and at try-handler entry, exactly those mutated names degrade to AliasState::Dynamic; names the body never touched keep their precise state. Both your repro shapes are regressions — loop with change h to g; break; change h to f, and a when handler entered mid-body — and both were previously false static rejections of calls the runtime accepts (the failure direction that breaks working programs). Branch constructs keep endpoint joins (no mid-branch exit is possible; a break inside a branch is the enclosing loop's frame), and the overload counters need no frames — they're monotonic, so endpoint disagreement already catches every definition.

One test-shape note: reassigning an alias across differently-shaped actions trips the pre-existing function-type assignment check (positional parameter compatibility; untyped parameters are compatible with anything), so the regressions reassign to an untyped-parameter action — the same leniency real dynamic code would use. Recorded in the diary.

Docs (actions-functions.md enforcement + dynamic-dispatch bullets, spec sentences for both rules), a TestProgram loop-reassignment section (validated end-to-end), and the diary ship in the same commit. Gates: fmt, clippy -D warnings, 618 lib + 60 overload tests green locally.


Generated by Claude Code

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated 3 comments.

Comment thread src/analyzer/mod.rs Outdated
Comment thread src/analyzer/mod.rs Outdated
Comment thread src/typechecker/mod.rs Outdated
…types

nothing arguments and container-inheritance overlap also leave several
statically-valid overload candidates; the three deferral comments now
say so (Copilot review nits).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SdLFM2t8Efd89LbaFh7Mkm
Copilot AI review requested due to automatic review settings July 20, 2026 17:41

logbie commented Jul 20, 2026

Copy link
Copy Markdown
Collaborator Author

Sixth-pass review of 37e7e14. The two round-5 findings are fixed on the paths now covered. I found two remaining P1 gaps:

  1. [P1] Apply alias flow analysis to every loop variantAnalyzer::analyze_statement now pushes mutation frames for ForEachLoop, CountLoop, and WhileLoop, but it has no arms at all for RepeatWhileLoop, RepeatUntilLoop, ForeverLoop, or MainLoop (the match moves from WhileLoop directly to display/expression cases). Their bodies therefore do not update action_aliases, record per-call-site resolutions, or participate in abrupt-exit degradation. For example, bind h = f, then in repeat while yes do change h to g; break, and call h afterward. Static analysis still records the call against f; runtime calls g. Choosing overload sets where f accepts the argument and g does not reproduces a statically accepted runtime dispatch failure. Handle these four AST variants with the same flow-entry/mutation-frame/join logic (with the appropriate zero/one-or-more path semantics).

  2. [P1] Roll back speculative enforcement when the promised merge never executesenter_block_overloads and the top-level pre-arm loop set an existing FunctionValue.enforce_param_types cell to true merely because a later definition is present lexically. That mutation is not part of BlockDupsScope and is never reverted if execution errors/returns before reaching the definition. This permanently converts a still-single action into a runtime-guarded one. A reusable-interpreter/REPL repro is: run snippet 1 defining lone typed f(number); run snippet 2 containing 1 divided by 0 before a later f(text) definition (the run fails before merging); then run snippet 3 calling the still-single f with a dynamic text value. The third run now rejects it, violating the documented legacy leniency. Track which existing members were armed and restore their prior flag on block/run exit unless they actually became part of an overload set.

Current config lint and automated review passed; the main CI workflow is still in progress on this head.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated 1 comment.

Comment thread Docs/reference/syntax-reference.md Outdated
Addresses the maintainer's sixth-pass review (two P1 findings):

1. repeat while / repeat until / forever / main loop now have analyzer
   arms with the same scope handling, flow-entry/mutation-frame/join,
   and mutated-alias degradation as the other loop forms — a break can
   no longer leave runtime alias state that static analysis contradicts.
   Because these bodies were never analyzed before, diagnostics arising
   inside them demote to warnings: a full semantic sweep of all 129
   TestPrograms showed five web-server programs (main-loop bodies
   referencing handler-provided names) would otherwise start failing,
   and backward compatibility wins. The sweep is back to its pre-change
   baseline (two deliberate error-demonstration programs).

2. Block-entry enforcement arming is now speculative-and-revertible:
   arm_block_members records each armed member's prior flag in an
   ArmedEnforcementGuard whose Drop (block exit and top-level run exit,
   including error paths) restores members that never actually merged
   into an overload set — so a run that fails before the merging
   definition executes no longer permanently converts a still-single
   action into a runtime-guarded one (the REPL repro from the review).
   Restoration runs in reverse order so re-armed duplicates unwind to
   the oldest flag; merged members keep enforcement.

Also labels the two call forms separately in the syntax reference
(Copilot nit). Three new red-first regressions (63 total), docs/spec
enforcement-revert wording, and the diary updated in the same change.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SdLFM2t8Efd89LbaFh7Mkm
Copilot AI review requested due to automatic review settings July 20, 2026 18:01

logbie commented Jul 20, 2026

Copy link
Copy Markdown
Collaborator Author

Sixth-pass findings addressed in 3ad243e — both fixed, red-first (three new regressions, 63 total in tests/overload_test.rs):

1. All loop variants participate in alias flow analysis (P1)repeat while, repeat until, forever, and main loop now have analyze_statement arms with the same scope handling, flow-entry/mutation-frame/join, and mutated-alias degradation as the other loops. The pre-checked forms join with the skip path; the infinite forms exit only through break, which the mutation-frame degradation covers, so including the entry path there is merely conservative. Your repeat while yes + break repro is a regression, plus a main loop variant.

One consequence needed a deliberate call, and I want to flag it for your judgment: these bodies were never analyzed before, so walking them surfaces diagnostics in existing programs. A full semantic sweep of all 129 TestPrograms showed five web-server programs would start failing — their main loop bodies reference handler-provided names (method, path, client_ip) this analyzer cannot model. Per the backward-compatibility policy, diagnostics arising inside these four body kinds demote to warnings (same channel as the try-body downgrade): alias tracking and per-call-site recording work, statically-detected alias misuse inside them defers to runtime, and the sweep is back to its pre-change baseline (only the two deliberate error-demonstration programs, scoped.wfl and test_redefinition_error.wfl, which fail on main too). If you'd rather model the web-server handler scope properly and make these bodies fully strict, that feels like follow-up work on the analyzer's HTTP-context knowledge rather than this PR.

2. Speculative arming is revertible (P1)arm_block_members records each armed member's prior flag in an ArmedEnforcementGuard; its Drop (block exit and top-level run exit, including error paths) restores every member that did not actually end up in an overload set — checked by membership in the name's current Value::Overloaded, not just by name, so rebinding can't confuse it. Restoration runs newest-first so re-armed duplicates unwind to the oldest flag; merged members keep enforcement (permanent by doctrine). Your exact three-snippet REPL scenario is the regression: snippet 2's failed run no longer leaves the still-single f guarded, and snippet 3's dynamic call gets legacy leniency again. The round-5 cross-block test still passes — enforcement holds during the window while the block is live, which is when it matters.

Also took Copilot's syntax-reference nit (the two call forms are now labeled statement vs expression). Docs/spec state the revert rule; diary has the round-6 section. Gates: fmt, clippy -D warnings, 618 lib + 63 overload tests, the comprehensive TestProgram end-to-end, and the 129-program semantic sweep at baseline.


Generated by Claude Code

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated 1 comment.

Comment thread src/interpreter/mod.rs

logbie commented Jul 20, 2026

Copy link
Copy Markdown
Collaborator Author

Seventh-pass / merge-readiness review of 3ad243e.

The two round-6 P1s are fixed, but I found one new merge blocker:

  1. [P1] Never demote a latched analyzer budget breach to a warning — the new RepeatWhileLoop/RepeatUntilLoop and ForeverLoop/MainLoop arms snapshot errors_before, analyze the body, then unconditionally drain every new self.errors entry into self.warnings. If analyze_statement or analyze_expression exhausts/cancels the shared execution budget inside one of those bodies, it pushes the rendered budget error and sets budget_error; the new drain removes the only fatal diagnostic. Analyzer::analyze then sees an empty error list and returns Ok(()) despite the latched fatal breach, so direct analyzer/--analyze callers can report success after analysis was aborted. A regression test can mirror analyzer_polls_the_budget_inside_nested_bodies, but build a repeat forever or main loop with many body statements and assert analyze returns Err containing the operation-budget message. Before demoting compatibility diagnostics, detect budget_error and retain/recreate its fatal SemanticError (or demote diagnostics selectively rather than draining the entire tail).

Merge recommendation: not yet. This is a fatal resource-limit channel regression, and the main CI workflow is also still in progress. Config lint, CodeRabbit, and the automated review workflow are green; there are no unresolved inline review threads. Once this P1 is fixed and the new head's CI is green, I would do one short final verification before merging.

The maintainer's merge-readiness review caught that round 6's
compatibility drain moved every new error into warnings — including the
rendered fatal error a shared-budget breach pushes when analysis aborts
inside a repeat/forever/main-loop body. analyze() then returned Ok with
budget_error latched, so --analyze and direct callers could report
success after an aborted analysis. The demotion now runs only when no
budget breach is latched; a breach keeps the whole error tail fatal.
Two red-first regressions (main loop + repeat while bodies under a
5-operation cap) mirror analyzer_polls_the_budget_inside_nested_bodies.

Also: the pre-existing pin test
main_loop_body_is_currently_not_statically_analyzed (which invited a
deliberate update when analyzer coverage arrived) is now
main_loop_body_diagnostics_demote_to_warnings, asserting the round-6
contract — the reference is surfaced as a warning, never a fatal error
(this was the Integration/Build CI failure on 3ad243e); and the
describe-teardown guard binding destructures both scopes (Copilot nit).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SdLFM2t8Efd89LbaFh7Mkm
Copilot AI review requested due to automatic review settings July 20, 2026 18:15

logbie commented Jul 20, 2026

Copy link
Copy Markdown
Collaborator Author

Seventh-pass blocker fixed in 5937006, red-first:

Budget breaches never demote (P1) — Both demotion sites now drain the error tail into warnings only when budget_error is unlatched. A breach keeps every error from the aborted body analysis fatal — analysis was aborted, not diagnosed, and analyze correctly returns Err carrying the rendered operation-budget message. Two regressions mirror analyzer_polls_the_budget_inside_nested_bodies with main loop and repeat while bodies under a 5-operation cap; both were red against 3ad243e (analyze returned Ok(()) exactly as you described) and are green now.

Also in this commit, closing out the CI failures you saw in progress: the Integration/Build failures on 3ad243e were the pre-existing pin test main_loop_body_is_currently_not_statically_analyzed, whose own message invites a deliberate update when analyzer coverage of these bodies arrives — it's now main_loop_body_diagnostics_demote_to_warnings, asserting the round-6 contract (the undefined reference is surfaced by --analyze, as a warning, never a fatal error). And the describe-teardown guard binding destructures both scopes per Copilot's nit (behavior was already correct — the tuple kept both guards alive — but the naming was misleading).

Gates on this head: fmt, clippy -D warnings, 618 lib + 63 overload + 41 budget tests, the updated pin-test crate, and the analyzer-sensitive integration crates green locally; the release-binary-driven crates are CI's to confirm (local release build is stale, noted in the diary). Ready for your final verification once this head's CI completes.


Generated by Claude Code

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 15 out of 15 changed files in this pull request and generated no new comments.

logbie commented Jul 20, 2026

Copy link
Copy Markdown
Collaborator Author

CI is fully green on 5937006 — Build/Test/Clippy, Integration (ubuntu + windows), Run WFL Programs (ubuntu + windows), Database, Fuzz, formatting, config-lint, CodeQL, and both automated reviews all passed. The budget-breach P1 from your merge-readiness pass is fixed on this head, all inline threads are resolved, and the branch is ready for your final verification.


Generated by Claude Code

logbie commented Jul 21, 2026

Copy link
Copy Markdown
Collaborator Author

Eighth-pass / final merge-readiness review of 5937006.

No new findings. The round-7 blocker is fixed correctly: compatibility diagnostics inside the newly analyzed loop bodies are demoted only when no fatal budget breach is latched, so an aborted analysis retains its fatal error. The added main loop and repeat while budget regressions cover both affected arms. I also rechecked the round-6 loop-flow and speculative-arming fixes in the current tree and found no remaining correctness gap.

Merge recommendation: yes. The PR is open, non-draft, and mergeable; CI, WFL Config Lint, Claude Code Review, and CodeRabbit are all green on this exact head. GitHub reports no unresolved inline review threads.

@logbie
logbie merged commit 6a7884d into main Jul 21, 2026
21 checks passed
@logbie
logbie deleted the claude/function-overload-resolution-xizxjb branch July 21, 2026 02:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants