fix: reject duplicate remaining allotments

[flemzord]

Summary

• reject allotment expressions with more than one remaining clause at runtime
• add public API regression coverage through Parse(...).Run(...)
• preserve existing supported runtime behavior where a single remaining can appear before a variable portion, as covered by metadata.num

TDD evidence

RED before fix:

• go test . -run TestRunRejectsDuplicateRemainingAllotments -count=1 -v returned nil error for a source allotment with two remaining clauses

GREEN after fix:

• go test . -run TestRunRejectsDuplicateRemainingAllotments -count=1 -v
• go test ./...

Notes

An initial broader runtime check for non-final remaining broke the existing metadata.num spec, which uses remaining before a variable commission portion. This PR therefore fixes the duplicate-remaining corruption path without narrowing the current runtime language semantics.

[coderabbitai]

Warning

Review limit reached

@flemzord, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 17 minutes and 57 seconds. Learn how PR review limits work.

Your organization has run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

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.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 838bddc5-3163-4a7d-bccf-054b1a7ab7e5

📥 Commits

Reviewing files that changed from the base of the PR and between f7bc53c and 603cbd6.

📒 Files selected for processing (3)

• internal/interpreter/interpreter.go
• internal/interpreter/interpreter_error.go
• internal/interpreter/interpreter_test.go

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch feat/reject-static-analysis-errors

---

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 60.00000% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 67.11%. Comparing base (f7bc53c) to head (603cbd6).

Files with missing lines	Patch %	Lines
internal/interpreter/interpreter_error.go	0.00%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #145      +/-   ##
==========================================
- Coverage   67.12%   67.11%   -0.01%     
==========================================
  Files          47       47              
  Lines        5071     5076       +5     
==========================================
+ Hits         3404     3407       +3     
- Misses       1469     1471       +2     
  Partials      198      198              

☔ 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]

The general philosophy is that, with the new rewrite (well, not so "new" anymore), we separate static analysis and runtime errors, and reduce runtime errors to be more permissive and only block computations we can't possibly interpreter or succeed (unbound var, missing funds, etc). However, it's true that this one's actually confusing/misleading to allow, so it's probably a good idea to merge that.

Since we're here, I'd suggest to

1. emit a more precise error on the last (redudant) clause, not on the whole allotment, which is a bit noisy on e.g. editors (however that's debatable, one may also argue that error is best on the whole source). This one's optional
2. Avoiding writing the test using the flaky error message, but on the exact emitted struct. Also note most of the tests are on interpreter_test instead, whereas this test suite focuses more on the external contract. This one's more strongly suggested