fix: reject parse errors before execution

[flemzord]

Summary

• reject public ParseResult.Run calls when parsing produced errors
• return MCP evaluate parse errors immediately instead of continuing into execution
• add regression coverage for public Run and MCP evaluate

TDD evidence

RED before fix:

• go test . -run TestRunRejectsParseErrors -count=1 -v panicked through batch_balances_query.go on an AST with nil source
• go test ./internal/mcp_impl -run TestHandleEvalToolRejectsParseErrors -count=1 -v failed before the handler seam/fix existed

GREEN after fix:

• go test . -run TestRunRejectsParseErrors -count=1 -v
• go test ./internal/mcp_impl -run TestHandleEvalToolRejectsParseErrors -count=1 -v
• go test ./...

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 6643aedd-7eb2-4520-888e-e429e5d84ca5

📥 Commits

Reviewing files that changed from the base of the PR and between 5cc059f and 373e09f.

📒 Files selected for processing (4)

• internal/mcp_impl/handlers.go
• internal/mcp_impl/handlers_test.go
• numscript.go
• numscript_test.go

---

Walkthrough

This PR improves parse error handling in numscript by adding early validation in the core execution path and refactoring the MCP tool handler to properly return parse errors instead of silently continuing or constructing errors without returning them.

Changes

Parse Error Handling Validation

Layer / File(s)	Summary
Core parse error validation numscript.go, numscript_test.go	RunWithFeatureFlags adds an early guard that returns the first parse error immediately when parseResult.Errors is non-empty, preventing invalid scripts from proceeding to interpreter execution. Test verifies error message matches the first parse error.
MCP handler refactor with proper parse error return internal/mcp_impl/handlers.go, internal/mcp_impl/handlers_test.go	handleEvalTool is extracted from an inline closure to a dedicated function that properly returns a tool error result when the script fails to parse (fixing the prior path where the error was constructed but not returned). Tool registration delegates to the new function, and test verifies parse errors return an error-marked result with the parse diagnostic message.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Poem

🐰 Parse errors caught before they spread,
No silent failures, no paths misdirected,
Early returns guard each invalid thread,
Now MCP and core both properly protected!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title 'fix: reject parse errors before execution' accurately and concisely describes the main change—preventing execution when parse errors are present.
Description check	✅ Passed	The description clearly explains the changes: rejecting parse errors in public Run calls and MCP handlers, plus adding regression tests with TDD evidence.
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.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch feat/reject-parse-errors-before-run

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 golangci-lint (2.12.2)

level=error msg="[linters_context] typechecking error: pattern ./...: directory prefix . does not contain main module or its selected dependencies"

---

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 and usage tips.}

[codecov]

Codecov Report

❌ Patch coverage is 32.25806% with 21 lines in your changes missing coverage. Please review.
✅ Project coverage is 67.16%. Comparing base (5cc059f) to head (373e09f).

Files with missing lines	Patch %	Lines
internal/mcp_impl/handlers.go	27.58%	20 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #144      +/-   ##
==========================================
+ Coverage   66.96%   67.16%   +0.19%     
==========================================
  Files          47       47              
  Lines        5068     5071       +3     
==========================================
+ Hits         3394     3406      +12     
+ Misses       1477     1468       -9     
  Partials      197      197              

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[ascandone]

Thanks!

[ascandone]

I'm merging that @flemzord