Fix exit codes, error handling, and 21 TestPrograms failures - #556
Merged
Conversation
…r, and stdlib fixes Triage of the failing TestPrograms uncovered a stack of WFL bugs, the biggest being that runtime and parse errors exited with code 0, silently masking dozens of broken programs. Fixing the exit codes exposed a second layer of latent failures, which were triaged and fixed the same way. Interpreter/CLI: - Runtime errors now exit 1 and parse errors exit 2 - catch/when clauses bind error_message alongside the error variable - wait for request refreshes implicit bindings (define_or_replace) instead of failing on redefinition - ActionCall dispatches to native functions - Date/Time/DateTime values support ordering comparisons - Count-loop variable shadows same-named outer variables Analyzer: - Removed the broken `store list as ...` special case - Undefined names inside try bodies downgrade to warnings (documented catchable-at-runtime behavior) - Undefined signal handler is a warning (runtime only records the name) - Nested count loops reusing a loop variable get an explicit error - New newline/tab text constants Parser: - Comparisons now bind tighter than and/or (fixes dropped comparisons after `and` and mis-parsed negative literals) - `count` followed by `with` is concatenation with the loop variable, not a call to the count list builtin - File paths accept `with` concatenation - Documented forms implemented: copy_file/move_file from..to, makedirs, remove_file at, remove_dir at [recursive], bare `when:` - add X to Y always parses as AddToListStatement (runtime decides append vs arithmetic) - change accepts contextual keywords; pattern/contains/output usable as variables in expression position - respond ... and content_type handles merged multi-word identifiers Stdlib: create_datetime, subtract_days, date_part, time_part, utc_now, year/month/day, hour/minute/second, dayofweek, dayofyear, is_leap_year, days_in_month, week_of_year, timestamp, datetime_from_timestamp, time_diff. Test programs were only modified for genuine syntax errors (else:, is before/after, random(), function/end function, reserved keywords as variable names, path exists at, / operator) or obvious authoring bugs (missing init, wrong cleanup filename, infinite loop, wait loop:). Test infrastructure: CI-SKIP first-line directives for tests needing an HTTP/WS client or unimplemented features; run_integration_tests.sh/.ps1 honor CI-SKIP, run describe-block programs with --test, and assert intentional-error programs exit nonzero; ci.yml drops skip entries for all now-fixed programs. Results: TestPrograms 110 passed / 0 failed / 34 skipped (previously 95/21/2, with many "passes" being programs that never parsed); cargo test --all fully green, including three subprocess tests that had been failing invisibly. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017mTdu5SJGa1DJzXHmBq2vE
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Contributor
|
Warning Review limit reached
Next review available in: 31 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 (24)
📝 WalkthroughWalkthroughThis PR applies fixes across the WFL analyzer, interpreter, parser, and stdlib (new time/date functions, comparison support, exit-code handling), updates CI workflow and integration test scripts to support CI-SKIP directives and ChangesLanguage core: analyzer, interpreter, parser, stdlib
Test infrastructure and TestPrograms updates
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant TestRunner as run_integration_tests
participant WflBinary
participant Program as .wfl program
TestRunner->>Program: read first line
alt CI-SKIP directive found
TestRunner-->>TestRunner: mark skipped
else contains describe block
TestRunner->>WflBinary: run with --test flag
WflBinary-->>TestRunner: exit code
TestRunner->>TestRunner: check EXPECTED_FAIL_TESTS
TestRunner-->>TestRunner: report pass/fail
else normal program
TestRunner->>WflBinary: run without --test
WflBinary-->>TestRunner: exit code
TestRunner-->>TestRunner: report pass/fail/timeout
end
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 |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR tightens CLI correctness (nonzero exit codes on errors) and then fixes a set of latent language/runtime issues that were previously masked, bringing the TestPrograms/ suite back to green while improving parsing/analyzer/interpreter behavior around error handling, loops, filesystem/web statements, and time/date utilities.
Changes:
- Fix CLI exit codes for runtime vs parse errors, and update integration/CI runners to correctly handle
--test, expected-failure programs, and// CI-SKIP:directives. - Improve language semantics: refreshed implicit request bindings on repeated waits,
error_messagealias in catch/when blocks, count-loop shadowing, native-function ActionCall dispatch, and date/time comparisons. - Extend parser/analyzer/stdlib to match documented syntax (filesystem statement forms, operator precedence, contextual keywords) and add time/date stdlib functions.
Reviewed changes
Copilot reviewed 65 out of 65 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| TestPrograms/web_server_websocket_test.wfl | Add CI-SKIP directive for unimplemented websocket features. |
| TestPrograms/web_server_session_test.wfl | Add CI-SKIP directive for unimplemented session/cookie/CSRF features. |
| TestPrograms/web_server_request_response_test.wfl | Add CI-SKIP directive (web server requires external HTTP client). |
| TestPrograms/web_server_middleware_test.wfl | Add CI-SKIP directive (web server requires external HTTP client). |
| TestPrograms/web_server_graceful_shutdown_test.wfl | Fix loop label (wait loop: → main loop:). |
| TestPrograms/web_server_example.wfl | Add CI-SKIP and initialize requests_count. |
| TestPrograms/web_server_content_length_test.wfl | Add CI-SKIP directive (web server requires external HTTP client). |
| TestPrograms/web_server_comprehensive_test.wfl | Add CI-SKIP directive (web server requires external HTTP client). |
| TestPrograms/web_route_params_test.wfl | Add CI-SKIP directive (web tests driven by web test script). |
| TestPrograms/time_random_comprehensive.wfl | Update comparison/conditional syntax to supported operators/keywords. |
| TestPrograms/test_web_server_response.wfl | Add CI-SKIP directive (web server requires external HTTP client). |
| TestPrograms/test_string_functions.wfl | Rename variables to avoid reserved keyword conflicts. |
| TestPrograms/test_static_files.wfl | Add CI-SKIP directive (web server requires external HTTP client). |
| TestPrograms/test_simple_static.wfl | Add CI-SKIP directive (web server requires external HTTP client). |
| TestPrograms/test_request_properties.wfl | Add CI-SKIP directive (web server requires external HTTP client). |
| TestPrograms/test_framework_validation.wfl | Rename variables to avoid reserved keyword conflicts and loop-var conflicts. |
| TestPrograms/test_create_list_expression.wfl | Update to current action syntax and nothing literal usage. |
| TestPrograms/test_contextual_keywords.wfl | Use change for contextual-keyword variable names. |
| TestPrograms/stack_overflow_test.wfl | Rename variables to avoid reserved keyword conflicts. |
| TestPrograms/simple_web_test.wfl | Add CI-SKIP directive (web server requires external HTTP client). |
| TestPrograms/simple_web_server.wfl | Add CI-SKIP directive (web server requires external HTTP client). |
| TestPrograms/simple_timeout_test.wfl | Add CI-SKIP directive (web server requires external HTTP client). |
| TestPrograms/patterns_comprehensive.wfl | Update pattern syntax to supported lookaround/capture/alternation forms. |
| TestPrograms/middleware_minimal_test.wfl | Add a break to avoid hanging loop in test program. |
| TestPrograms/lsp_demo.wfl | Replace unsupported / operator with divided by. |
| TestPrograms/header_access_test.wfl | Add CI-SKIP directive (web server requires external HTTP client). |
| TestPrograms/file_io_comprehensive.wfl | Update directory existence check wording to supported form. |
| TestPrograms/error_handling_comprehensive.wfl | Add CI-SKIP directive for unimplemented error-handling features. |
| TestPrograms/docs_examples/keyword_reference/web_network_examples.wfl | Add CI-SKIP directive due to pre-existing parse errors. |
| TestPrograms/docs_examples/keyword_reference/process_examples.wfl | Add CI-SKIP directive due to pre-existing parse errors. |
| TestPrograms/docs_examples/keyword_reference/operations_examples.wfl | Add CI-SKIP directive due to pre-existing parse errors. |
| TestPrograms/docs_examples/keyword_reference/file_io_examples.wfl | Add CI-SKIP directive due to pre-existing parse errors. |
| TestPrograms/docs_examples/keyword_reference/error_handling_examples.wfl | Add CI-SKIP directive due to pre-existing parse errors. |
| TestPrograms/docs_examples/keyword_reference/declaration_examples.wfl | Add CI-SKIP directive due to pre-existing parse errors. |
| TestPrograms/docs_examples/keyword_reference/control_flow_examples.wfl | Add CI-SKIP directive due to pre-existing parse errors. |
| TestPrograms/docs_examples/keyword_reference/contextual_keywords_examples.wfl | Add CI-SKIP directive due to pre-existing parse errors. |
| TestPrograms/docs_examples/keyword_reference/containers_examples.wfl | Add CI-SKIP directive due to pre-existing parse errors. |
| TestPrograms/docs_examples/keyword_reference/comparison_examples.wfl | Add CI-SKIP directive due to pre-existing parse errors. |
| TestPrograms/direct_index_comprehensive.wfl | Add CI-SKIP directive for unimplemented syntax/features. |
| TestPrograms/destructive_operations_test.wfl | Update directory existence check wording to supported form. |
| TestPrograms/debug_random.wfl | Remove unsupported call-parentheses syntax for zero-arg builtins. |
| TestPrograms/count_lines_test.wfl | Fix cleanup deleting wrong filename. |
| TestPrograms/comprehensive_web_server_demo.wfl | Add CI-SKIP directive (web server requires external HTTP client). |
| src/stdlib/time.rs | Add/extend time/date stdlib helpers and new native functions. |
| src/stdlib/core.rs | Add global newline and tab text constants. |
| src/parser/stmt/web.rs | Improve respond ... and content_type ... parsing for merged identifiers. |
| src/parser/stmt/variables.rs | Allow contextual keywords as variable names in change statements. |
| src/parser/stmt/processes.rs | Allow output/contextual keywords as variable names in process-output statements. |
| src/parser/stmt/io.rs | Introduce parse_path_expression to support with concatenation in paths. |
| src/parser/stmt/errors.rs | Allow bare when: shorthand for when error:. |
| src/parser/stmt/collections.rs | Defer add X to Y semantics to runtime (parser no longer guesses by literal type). |
| src/parser/mod.rs | Add parsing for documented filesystem statement forms and their arguments. |
| src/parser/helpers.rs | Extend token-to-text mapping for additional keywords. |
| src/parser/expr/primary.rs | Treat pattern/output/contains as contextual where appropriate. |
| src/parser/expr/binary.rs | Fix precedence ladder and avoid mis-parsing count as legacy builtin call. |
| src/main.rs | Exit nonzero on runtime errors (1) and parse errors (2). |
| src/interpreter/mod.rs | Implement count-loop shadowing, request rebind refresh, native-function ActionCall dispatch, error_message binding, and date/time comparisons. |
| src/interpreter/environment.rs | Add define_or_replace for refreshable implicit bindings. |
| src/builtins.rs | Register new time-related builtins and update arity metadata. |
| src/analyzer/static_analyzer.rs | Improve warning notes for undefined variables/handlers. |
| src/analyzer/mod.rs | Add try-depth and loop-variable tracking; add newline/tab globals; relax undefined-name severity inside try; refresh request binding analysis. |
| scripts/run_integration_tests.sh | Honor CI-SKIP directives, run describe programs under --test, and assert expected-failure programs. |
| scripts/run_integration_tests.ps1 | PowerShell equivalent updates for CI-SKIP, --test, and expected-failure programs. |
| Dev diary/2026-07-03-testprograms-triage-and-fixes.md | Document triage findings, fixes, and follow-ups. |
| .github/workflows/ci.yml | Align CI TestPrograms execution with CI-SKIP handling and --test detection. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+6990
to
+6991
| native_fn(arg_values) | ||
| .map_err(|e| RuntimeError::new(e.to_string(), *line, *column)) |
Comment on lines
+736
to
+739
| // The implicit loop variable may shadow an ordinary variable of | ||
| // the same name from an outer scope, so redefinition is not an | ||
| // error here (nested loops are handled above). | ||
| let _ = self.current_scope.define(count_symbol); |
Comment on lines
+1458
to
+1469
| @@ -1423,11 +1466,10 @@ | |||
| column: *column, | |||
| }; | |||
|
|
|||
| if let Err(error) = self.current_scope.define(request_symbol) { | |||
| self.errors.push(error); | |||
| } | |||
| let _ = self.current_scope.define(request_symbol); | |||
Comment on lines
1481
to
1491
| for (prop_name, prop_type) in request_properties.iter() { | ||
| let prop_symbol = Symbol { | ||
| name: prop_name.to_string(), | ||
| kind: SymbolKind::Variable { mutable: false }, | ||
| symbol_type: Some(prop_type.clone()), | ||
| line: *line, | ||
| column: *column, | ||
| }; | ||
|
|
||
| if let Err(error) = self.current_scope.define(prop_symbol) { | ||
| self.errors.push(error); | ||
| } | ||
| let _ = self.current_scope.define(prop_symbol); | ||
| } |
Comment on lines
+721
to
+725
| "Nested count loops both use the loop variable '{loop_var_name}'. Give the loops distinct names with 'count from X to Y as <name>:'." | ||
| ), | ||
| 0, | ||
| 0, | ||
| )); |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (5)
scripts/run_integration_tests.sh (1)
189-197: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMisleading failure message for timed-out expected-fail tests.
When an expected-fail test times out (
exit_code=124), it falls into theelsebranch and prints"FAIL $test_name (expected a nonzero exit, got $exit_code)"— but 124 is nonzero; the real problem is that it timed out rather than exiting with a genuine error code. This message could mislead someone debugging CI output into thinking the test exited with code 0.✏️ Suggested message fix
if [ $exit_code -ne 0 ] && [ $exit_code -ne 124 ]; then print_success "PASS $test_name (expected failure, exit code: $exit_code)" passed_programs=$((passed_programs + 1)) + elif [ $exit_code -eq 124 ]; then + print_error "TIMEOUT $test_name (exceeded ${TEST_TIMEOUT}s, expected a real failure)" + failed_programs=$((failed_programs + 1)) else print_error "FAIL $test_name (expected a nonzero exit, got $exit_code)" failed_programs=$((failed_programs + 1)) fi🤖 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 `@scripts/run_integration_tests.sh` around lines 189 - 197, The expected-fail handling in is_expected_fail within the integration test runner treats exit_code 124 as a generic nonzero failure, which makes the failure message misleading. Update the branch around print_error/print_success so timed-out expected-fail tests are reported explicitly as a timeout when exit_code is 124, while preserving the current success path for other nonzero exits. Keep the change localized to the expected-fail block in the shell script and adjust the message text to reflect the actual condition.src/builtins.rs (1)
308-340: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDocument
create_datetime's minimum arity
create_datetimeaccepts 3–6 args at runtime, butget_function_arityexposes it as 3 and that value also feeds builtin type inference. The direct-call path usesarguments.len(), so execution isn’t bounded here, but the signature is still misleading; add a short note like thetimestampentry.🤖 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/builtins.rs` around lines 308 - 340, Update get_function_arity in src/builtins.rs to clarify that create_datetime is only documented with a minimum arity of 3, since it can accept 3–6 arguments at runtime. Keep the existing arity value used by builtin type inference and direct-call handling, but add a brief inline note near the create_datetime entry, similar to the timestamp comment, so the signature is not misleading and future readers understand the accepted range.TestPrograms/docs_examples/keyword_reference/contextual_keywords_examples.wfl (1)
1-1: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winTrack the "needs a docs-example fix pass" TODO.
This and the other four
docs_examples/keyword_reference/*.wflfiles are permanently skipped with only a vague inline note, no tracked issue. Per coding guidelines, doc examples must be validated before shipping; parking them indefinitely without a tracked follow-up risks them being forgotten. Consider filing an issue per broken example (or one umbrella issue) referencing the CI-SKIP comments.Want me to open tracking issues for these skipped docs examples?
🤖 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 `@TestPrograms/docs_examples/keyword_reference/contextual_keywords_examples.wfl` at line 1, The docs example is being permanently skipped with only an untracked inline note, which leaves the required follow-up undocumented. Replace the vague CI-SKIP note in contextual_keywords_examples.wfl with a tracked reference to an issue, and do the same for the other docs_examples/keyword_reference/*.wfl files; use the existing CI-SKIP comment as the marker and add either one umbrella issue or per-file issue references so the docs-example fix pass is explicitly tracked.Source: Coding guidelines
TestPrograms/web_route_params_test.wfl (1)
1-2: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRedundant/dead CI-SKIP comment on line 2.
The CI workflow only inspects the first line of the file for the
CI-SKIP:directive. The pre-existing comment on line 2 (// CI-SKIP: starts server and waits for requests) is now dead documentation and could confuse readers into thinking it also triggers skipping.🧹 Proposed cleanup
// CI-SKIP: starts a web server and needs an HTTP client to drive it (covered by run_web_tests) -// CI-SKIP: starts server and waits for requests // Route parameter extraction E2E test, driven by scripts/run_web_tests.sh.🤖 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 `@TestPrograms/web_route_params_test.wfl` around lines 1 - 2, The second CI-SKIP comment is dead documentation because only the first line directive is read by the workflow. Remove the redundant skip comment from web_route_params_test.wfl and keep only the intended top-line CI-SKIP directive so the intent is clear; this cleanup is in the test file’s header comments and does not require changes to the test logic itself.TestPrograms/debug_random.wfl (1)
2-12: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value"With parentheses" and "without parentheses" sections are now identical.
Both blocks now call
random/random_booleanwithout parentheses, so this debug script no longer actually contrasts the two calling conventions it claims to test (labels at Lines 2 and 6, and Lines 10 and 14, describe different scenarios but exercise identical syntax).🤖 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 `@TestPrograms/debug_random.wfl` around lines 2 - 12, The debug script’s “with parentheses” and “without parentheses” checks are identical, so it no longer tests two calling conventions. Update the logic in debug_random.wfl around the random and random_boolean calls so the sections labeled by the display statements actually differ: one should invoke the functions with parentheses and the other without, using the store/display statements for r1, r2, and r3 to verify both forms.
🤖 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 @.github/workflows/ci.yml:
- Around line 206-216: `module_helper.wfl` is missing from the non-CI skip
handling, so local integration runs still try to execute this helper-only
program. Update the skip lists in `scripts/run_integration_tests.sh` and
`scripts/run_integration_tests.ps1` to include `module_helper.wfl`, or add a
file-level `CI-SKIP:` directive so the existing `SKIP_REASONS`-style logic
treats it as intentionally skipped.
In `@src/analyzer/mod.rs`:
- Around line 719-726: The nested count-loop diagnostic in the CountLoop
handling is using placeholder coordinates, causing the error to surface at 0:0.
Update the `CountLoop` match arm in `SemanticAnalyzer` to bind the actual `line`
and `column` fields instead of eliding them, then pass those real coordinates
into `SemanticError::new` for the nested loop variable name check.
In `@src/interpreter/environment.rs`:
- Around line 103-109: The define_or_replace method in Environment currently
blindly inserts into the current scope and can overwrite an existing local
constant while is_constant(name) still says it is constant. Update this path to
preserve constant bindings in the current scope, either by refusing to replace
them or by routing only non-constant runtime-refreshed names through the
overwrite behavior. Keep the fix localized around Environment::define_or_replace
and its constant-checking helpers so names like method, path, body, client_ip,
headers, and error_message remain consistent.
In `@src/parser/expr/binary.rs`:
- Around line 157-172: Update the comparison parsing in binary expression
handling so `is greater than or equal to` uses the same precedence as the rest
of the comparison ladder. In `parse_binary_expression` within
`src/parser/expr/binary.rs`, adjust the `Operator::GreaterThanOrEqual` branches
to return precedence `1` instead of `0`, matching the other comparison operators
and keeping `greater than or equal to` from stopping early in nested/argument
parsing contexts.
In `@src/parser/mod.rs`:
- Around line 549-645: The parser in mod.rs accepts delete_file as an action
call even though it is not implemented in the filesystem runtime wiring. Either
remove delete_file from the parser’s supported statement forms or register a
matching action in stdlib/filesystem.rs and ensure the action name is recognized
consistently with copy_file, move_file, makedirs, remove_file, and remove_dir so
parsing cannot produce an Undefined action error at runtime.
In `@src/stdlib/time.rs`:
- Around line 443-471: The native_timestamp function is treating local naive
datetimes as if they were UTC by passing Local::now().naive_local() and the
bare-Time path through .and_utc(), which skews Unix seconds on non-UTC hosts.
Update native_timestamp to use UTC-based values consistently (and align
datetime_now if it returns a local naive datetime), or otherwise make the
local-wall-clock behavior explicit and documented. Make sure the Date, Time, and
DateTime branches all follow the same timezone contract.
---
Nitpick comments:
In `@scripts/run_integration_tests.sh`:
- Around line 189-197: The expected-fail handling in is_expected_fail within the
integration test runner treats exit_code 124 as a generic nonzero failure, which
makes the failure message misleading. Update the branch around
print_error/print_success so timed-out expected-fail tests are reported
explicitly as a timeout when exit_code is 124, while preserving the current
success path for other nonzero exits. Keep the change localized to the
expected-fail block in the shell script and adjust the message text to reflect
the actual condition.
In `@src/builtins.rs`:
- Around line 308-340: Update get_function_arity in src/builtins.rs to clarify
that create_datetime is only documented with a minimum arity of 3, since it can
accept 3–6 arguments at runtime. Keep the existing arity value used by builtin
type inference and direct-call handling, but add a brief inline note near the
create_datetime entry, similar to the timestamp comment, so the signature is not
misleading and future readers understand the accepted range.
In `@TestPrograms/debug_random.wfl`:
- Around line 2-12: The debug script’s “with parentheses” and “without
parentheses” checks are identical, so it no longer tests two calling
conventions. Update the logic in debug_random.wfl around the random and
random_boolean calls so the sections labeled by the display statements actually
differ: one should invoke the functions with parentheses and the other without,
using the store/display statements for r1, r2, and r3 to verify both forms.
In
`@TestPrograms/docs_examples/keyword_reference/contextual_keywords_examples.wfl`:
- Line 1: The docs example is being permanently skipped with only an untracked
inline note, which leaves the required follow-up undocumented. Replace the vague
CI-SKIP note in contextual_keywords_examples.wfl with a tracked reference to an
issue, and do the same for the other docs_examples/keyword_reference/*.wfl
files; use the existing CI-SKIP comment as the marker and add either one
umbrella issue or per-file issue references so the docs-example fix pass is
explicitly tracked.
In `@TestPrograms/web_route_params_test.wfl`:
- Around line 1-2: The second CI-SKIP comment is dead documentation because only
the first line directive is read by the workflow. Remove the redundant skip
comment from web_route_params_test.wfl and keep only the intended top-line
CI-SKIP directive so the intent is clear; this cleanup is in the test file’s
header comments and does not require changes to the test logic itself.
🪄 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: c2339705-f8df-4f4e-baf4-97a06a0b7984
📒 Files selected for processing (65)
.github/workflows/ci.ymlDev diary/2026-07-03-testprograms-triage-and-fixes.mdTestPrograms/comprehensive_web_server_demo.wflTestPrograms/count_lines_test.wflTestPrograms/debug_random.wflTestPrograms/destructive_operations_test.wflTestPrograms/direct_index_comprehensive.wflTestPrograms/docs_examples/keyword_reference/comparison_examples.wflTestPrograms/docs_examples/keyword_reference/containers_examples.wflTestPrograms/docs_examples/keyword_reference/contextual_keywords_examples.wflTestPrograms/docs_examples/keyword_reference/control_flow_examples.wflTestPrograms/docs_examples/keyword_reference/declaration_examples.wflTestPrograms/docs_examples/keyword_reference/error_handling_examples.wflTestPrograms/docs_examples/keyword_reference/file_io_examples.wflTestPrograms/docs_examples/keyword_reference/operations_examples.wflTestPrograms/docs_examples/keyword_reference/process_examples.wflTestPrograms/docs_examples/keyword_reference/web_network_examples.wflTestPrograms/error_handling_comprehensive.wflTestPrograms/file_io_comprehensive.wflTestPrograms/header_access_test.wflTestPrograms/lsp_demo.wflTestPrograms/middleware_minimal_test.wflTestPrograms/patterns_comprehensive.wflTestPrograms/simple_timeout_test.wflTestPrograms/simple_web_server.wflTestPrograms/simple_web_test.wflTestPrograms/stack_overflow_test.wflTestPrograms/test_contextual_keywords.wflTestPrograms/test_create_list_expression.wflTestPrograms/test_framework_validation.wflTestPrograms/test_request_properties.wflTestPrograms/test_simple_static.wflTestPrograms/test_static_files.wflTestPrograms/test_string_functions.wflTestPrograms/test_web_server_response.wflTestPrograms/time_random_comprehensive.wflTestPrograms/web_route_params_test.wflTestPrograms/web_server_comprehensive_test.wflTestPrograms/web_server_content_length_test.wflTestPrograms/web_server_example.wflTestPrograms/web_server_graceful_shutdown_test.wflTestPrograms/web_server_middleware_test.wflTestPrograms/web_server_request_response_test.wflTestPrograms/web_server_session_test.wflTestPrograms/web_server_websocket_test.wflscripts/run_integration_tests.ps1scripts/run_integration_tests.shsrc/analyzer/mod.rssrc/analyzer/static_analyzer.rssrc/builtins.rssrc/interpreter/environment.rssrc/interpreter/mod.rssrc/main.rssrc/parser/expr/binary.rssrc/parser/expr/primary.rssrc/parser/helpers.rssrc/parser/mod.rssrc/parser/stmt/collections.rssrc/parser/stmt/errors.rssrc/parser/stmt/io.rssrc/parser/stmt/processes.rssrc/parser/stmt/variables.rssrc/parser/stmt/web.rssrc/stdlib/core.rssrc/stdlib/time.rs
Analyzer: - Align static scope shadowing with runtime: count-loop variables and wait-for-request bindings now use Scope::define_or_replace so they shadow outer variables exactly like the interpreter does - Report the nested-count-loop error at the loop's real line/column instead of 0:0 Parser: - Fix two 'greater than or equal to' branches that were left at precedence 0 (cargo fmt had reflowed them past the precedence rebalance), making gte consistent with the comparison ladder Interpreter: - Native ActionCall errors keep their message and kind; only the location is pointed at the call site (no more duplicated "Runtime error at..." prefixes) - Environment::define_or_replace clears a stale constant marker so refreshed implicit bindings can't be left half-constant Stdlib: - Register delete_file as the documented alias of remove_file (it parsed but failed with "Undefined action" at runtime) - timestamp uses UTC consistently (no-arg returns true Unix time; naive arguments are documented as UTC wall-clock, keeping it an exact inverse of datetime_from_timestamp) - Document create_datetime's 3-6 argument range in the arity table Test infrastructure: - module_helper.wfl added to both runner skip lists (helper module) - Expected-fail timeouts are reported as TIMEOUT instead of a misleading "expected a nonzero exit" message - Removed a duplicate CI-SKIP line in web_route_params_test.wfl - CI-SKIP headers for docs examples and unimplemented-feature tests now reference tracking issue #555 - debug_random.wfl now contrasts store-then-display vs direct display auto-calls instead of two identical sections Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017mTdu5SJGa1DJzXHmBq2vE
Integration Tests (ubuntu-latest) failed on wflhash_security_tests:: test_constant_time_measures with a timing coefficient of variation of 150.68% against the 150% threshold. The test itself documents that timing measurements are unreliable on shared CI runners; nothing in this PR touches the crypto code, and the same job passed on the previous commit. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017mTdu5SJGa1DJzXHmBq2vE
This was referenced Jul 9, 2026
This was referenced Jul 18, 2026
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
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.
Summary
This PR fixes a critical bug where
main.rsreported runtime and parse errors but still exited with code 0, masking dozens of broken programs. Fixing exit codes exposed a second layer of latent failures across the analyzer, parser, and interpreter. All 21 failing TestPrograms are now fixed.Key Changes
Interpreter / CLI
src/main.rs(previously both fell through to exit 0)error_messagein catch blocks:catch/when errorclauses now bind the caught error's message toerror_messagein addition to the clause's error variable, in both interpreter and analyzerwait for request: Implicit request bindings (method,path,client_ip,body,headers) are refreshed on every wait via newEnvironment::define_or_replaceinstead of failing with "already defined"ActionCallon native functions: Expression-level action calls now dispatch toValue::NativeFunction(previously only user-defined actions were callable)is less than/is greater thannow compareDate,Time, andDateTimevaluescountloop variable now shadows an outer variable of the same name instead of silently failing to bindAnalyzer
store list as …special case that defined a variable named after the value instead oflisttrybody are now warnings instead of fatal errors (documented behavior: they raise catchable runtime errors)Undefined signal handleris now a warningnewline("\n") andtab("\t")Parser
and/or(ladder:and/or< comparisons <+/-<*///%). Previously multi-token operators were consumed during detection, losing them on precedence breakscountis the loop variable, not a call:display "…" with count with "…"is concatenation with the count-loop variable (documented idiom), not a legacy call to thecountlist builtinwithconcatenation:open file at base with "/index.html" for reading as fcopy_file from A to B,move_file from A to B,makedirs <path>,remove_file at <path>,remove_dir at <path> [recursive <flag>]add X to Ydecided at runtime: Parser no longer guesses list-append vs arithmetic from literal typechangeaccepts contextual keywords as variable names;patternandcontainsfall back to variable references in expression positionrespond … and content_type <variable>: Handles lexer's merged multi-word identifiers so variables work as content-type valueswhen:accepted as shorthand forwhen error:Standard Library
create_datetime,subtract_days,date_part,time_part,utc_now,year,month,day,dayofweek,dayofyear,hour,minute,second,is_leap_year,days_in_month,week_of_year,timestamp,datetime_from_timestamp,time_diffexpect_date_likeandexpect_time_likefor flexible date/time argument handlingTest Infrastructure
// CI-SKIP:directivesEXPECTED_FAIL_TESTSarray for programs that intentionally exit nonzerohttps://claude.ai/code/session_017mTdu5SJGa1DJzXHmBq2vE
Summary by CodeRabbit
New Features
Bug Fixes
Tests & CI