Add actionable hint for undefined symbols in load-module programs

[logbie]

Summary

When a WFL program uses load module and references an undefined action or variable, the analyzer now attaches an actionable note explaining that load module runs in an isolated scope and suggesting include from as the correct alternative. This addresses issue #584 by making a fatal error (which correctly cannot be relaxed) more helpful to users.

Key Changes

• Analyzer enhancement (src/analyzer/static_analyzer.rs):

  • Detect when a program contains load module statements
  • For undefined-symbol errors in such programs, attach a note explaining that load module is isolated and recommending include from as the mechanism to share definitions
  • The error remains fatal (as it must — the call cannot resolve at runtime either), but is now actionable

• Helper function (src/analyzer/mod.rs):

  • Added program_has_load_module() to detect top-level load module statements
  • Mirrors the existing program_has_includes() pattern

• Documentation update (Docs/04-advanced-features/modules.md):

  • Restructured the "Basic Module Loading" section with a decision table clarifying when to use include from (to share libraries) vs. load module (for side-effect-only files)
  • Added a prominent warning about the common mistake of trying to call actions from a loaded module
  • Expanded "What Modules Cannot Do" to explain the error and show the diagnostic output
  • Clarified that load module isolation is intentional and documented

• Comprehensive test suite (tests/load_module_undefined_hint_test.rs):

  • Tests the exact repro from issue Actions/containers from load module are invisible to the static analyzer (fatal "not defined") #584 (both of-form and call-form references)
  • Verifies the hint appears under --analyze
  • Confirms that include from is the working alternative
  • Guard rails: ensures the hint only appears when load module is present (not for unrelated undefined-symbol errors)

Implementation Details

The fix leverages the fact that load module isolation is by design — a caller referencing a loaded module's definition fails at runtime too, not only in the analyzer. Rather than relaxing the error (which would be wrong), the solution keeps it fatal but makes it actionable by pointing users at the correct mechanism (include from) that actually shares definitions across files.

The hint is only shown when the program contains a load module statement, avoiding false guidance for unrelated undefined-symbol errors.

https://claude.ai/code/session_01RAtsuADakMwNuDy6n3967r

Summary by CodeRabbit

• Documentation

  • Clarified how the two module-loading mechanisms differ, with a clearer comparison and stronger guidance on when to use each one.
  • Expanded module isolation notes and added an example diagnostic for undefined names.

• Bug Fixes

  • Error messages now include a helpful note when load module from leads to missing actions or variables, pointing to the correct way to share definitions.

• Tests

  • Added coverage for undefined-symbol hints, --analyze behavior, and successful behavior when using the shared-definition approach.

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

[coderabbitai]

Warning

Review limit reached

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

Next review available in: 50 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: 3779c21d-e3ec-4735-b11f-1e67d5792ebe

📥 Commits

Reviewing files that changed from the base of the PR and between d937f4a and d3b4368.

📒 Files selected for processing (3)

• Docs/04-advanced-features/modules.md
• src/analyzer/static_analyzer.rs
• tests/load_module_undefined_hint_test.rs

📝 Walkthrough

Walkthrough

Adds a program_has_load_module helper to detect load module from usage, wires it into the static analyzer to attach an "include from" explanatory note to fatal undefined-action/variable diagnostics in that context, updates module documentation, and adds a dedicated integration test suite.

Changes

Load module diagnostic hint

Layer / File(s)	Summary
Load module detection helper src/analyzer/mod.rs	Adds public program_has_load_module function scanning top-level statements for LoadModuleStatement.
Diagnostic note wiring src/analyzer/static_analyzer.rs	Computes load-module presence and attaches an "include from" note to undefined action/variable diagnostics instead of always using None.
Integration tests tests/load_module_undefined_hint_test.rs	New test suite verifying fatal exit codes and the presence/absence of the include-from hint across of-form, call-form, analyze mode, control, and guard-rail scenarios.
Documentation updates Docs/04-advanced-features/modules.md	Adds an include-from vs load-module comparison table, broadens the "cannot do" isolation rule, and adds a sample diagnostic with guidance.

Estimated code review effort: 2 (Simple) | ~15 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Program
  participant Analyzer as Analyzer::analyze_static
  participant Detector as program_has_load_module
  participant Diagnostic as WflDiagnostic

  Program->>Analyzer: analyze(program)
  Analyzer->>Detector: program_has_load_module(program)
  Detector-->>Analyzer: true/false
  Analyzer->>Analyzer: classify error as undefined action/variable
  Analyzer->>Diagnostic: new(error, note)
  Diagnostic-->>Program: fatal diagnostic with include-from hint

Loading

Possibly related issues

• Actions/containers from load module are invisible to the static analyzer (fatal "not defined") #584: Adds the diagnostic hint and tests directly addressing the undefined-name/visibility issue described for load module.

🚥 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 accurately summarizes the main change: adding actionable hints for undefined symbols in load-module programs.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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/issue-584-33tugf

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[coderabbitai]

🧹 Nitpick comments (2)

tests/load_module_undefined_hint_test.rs (1)

143-163: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Missing guard-rail test for mixed load-module + unrelated-typo case.

Given the note-attachment logic in static_analyzer.rs applies the hint to any undefined-symbol error whenever load module is present anywhere in the file, it would be valuable to add a test asserting the hint still fires (correctly or not) when the undefined symbol is unrelated to the loaded module, to pin the current behavior.

🤖 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 `@tests/load_module_undefined_hint_test.rs` around lines 143 - 163, Add a
guard-rail test in undefined_without_load_module_has_no_include_hint or a nearby
test that covers a file containing load module plus an unrelated undefined
of-callee, using run_file_status and checking the static_analyzer.rs
note-attachment behavior. The test should assert whether the include from hint
is present for the mixed case so the current undefined-symbol hint behavior
stays pinned when load module appears anywhere in the file.

src/analyzer/static_analyzer.rs (1)

158-165: 🎯 Functional Correctness | 🔵 Trivial | ⚖️ Poor tradeoff

Note is attached to any undefined-symbol error whenever load module appears anywhere in the file, not just symbols tied to the loaded module.

If a file legitimately uses load module for side effects and separately has an unrelated typo (e.g., a misspelled local variable), this note will still fire, telling the user to use include from — which wouldn't fix the typo. This is a coarse heuristic; consider scoping the check to whether the undefined name is a plausible export of the loaded file, if that information is available to the analyzer.

Also applies to: 183-201

🤖 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/analyzer/static_analyzer.rs` around lines 158 - 165, The undefined-symbol
note is currently triggered too broadly by program_has_load_module, so it gets
attached to unrelated typos whenever any load module exists in the file. Update
the static_analyzer logic around the undefined-symbol handling to scope the note
more precisely—ideally only when the missing name is plausibly an export from
the loaded module, or otherwise when the error is actually related to the loaded
file. Use the existing has_load_module check and the undefined-symbol path in
static_analyzer to narrow when the “include from” guidance is emitted, including
the same adjustment in the later matching block referenced by the analyzer
comment.

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

Nitpick comments:
In `@src/analyzer/static_analyzer.rs`:
- Around line 158-165: The undefined-symbol note is currently triggered too
broadly by program_has_load_module, so it gets attached to unrelated typos
whenever any load module exists in the file. Update the static_analyzer logic
around the undefined-symbol handling to scope the note more precisely—ideally
only when the missing name is plausibly an export from the loaded module, or
otherwise when the error is actually related to the loaded file. Use the
existing has_load_module check and the undefined-symbol path in static_analyzer
to narrow when the “include from” guidance is emitted, including the same
adjustment in the later matching block referenced by the analyzer comment.

In `@tests/load_module_undefined_hint_test.rs`:
- Around line 143-163: Add a guard-rail test in
undefined_without_load_module_has_no_include_hint or a nearby test that covers a
file containing load module plus an unrelated undefined of-callee, using
run_file_status and checking the static_analyzer.rs note-attachment behavior.
The test should assert whether the include from hint is present for the mixed
case so the current undefined-symbol hint behavior stays pinned when load module
appears anywhere in the file.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 18b22426-c6f9-4950-9394-8c76ba144d32

📥 Commits

Reviewing files that changed from the base of the PR and between cc63b1c and d937f4a.

📒 Files selected for processing (4)

• Docs/04-advanced-features/modules.md
• src/analyzer/mod.rs
• src/analyzer/static_analyzer.rs
• tests/load_module_undefined_hint_test.rs

[logbie]

Thanks @coderabbitai — both nitpicks addressed in d3b4368.

On the coarse-trigger point: the note does fire for any undefined symbol when a file uses load module, and I kept it that way deliberately. Scoping it to "is this name a plausible export of the loaded file?" would require the analyzer to resolve+parse the loaded module (path resolution, cycle guard, etc.) — the exact heavyweight approach this PR intentionally avoids, since load module symbols are genuinely absent at runtime and precision there buys little for a supplementary note.

Instead I reworded the note to be conditionally framed so it can't mislead a plain typo:

If you expected this name to come from a file loaded with load module, note that load module from "..." runs a file in an isolated scope and does not expose its actions, containers, or variables to the caller. To share definitions across files, use include from "..." instead.

For a real module symbol it's the fix; for an unrelated typo the "if you expected…" clause simply doesn't apply, so the primary is not defined error stands on its own. Added load_module_plus_unrelated_typo_is_hinted_but_not_misleading to pin exactly that behavior.

(Note: the copilot-pull-request-reviewer check failed on a Copilot account quota limit (HTTP 402), unrelated to the diff — no code action needed there.)

---

Generated by Claude Code

[coderabbitai]

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
cd /repo 2>/dev/null || true
git log --oneline -5 2>/dev/null
git show d3b4368 --stat 2>/dev/null | head -50

Length of output: 1996

---

@logbie Thanks for the detailed explanation and for the follow-up in d3b4368. Reworking the note into a conditional framing ("If you expected this name to come from a file loaded with load module...") is a good compromise — it keeps the fix simple and avoids the overhead of resolving/parsing the loaded module just to validate the symbol, while ensuring the note doesn't mislead on unrelated typos. The load_module_plus_unrelated_typo_is_hinted_but_not_misleading test is a nice way to pin that behavior for good.

Agreed the Copilot reviewer failure (HTTP 402 quota) is unrelated to this diff — no action needed there.

ᐡ˵•ᴥ•˵ᐡ