fix: draft() emits ciceromark Formula node without required value field (#146)

[mttrbrts]

Fixes #146.

Root cause

A formula written as a bare expression — e.g. {{% importerLOCAmount / 2.0 %}} without an explicit return — was compiled by writeFunctionToString into a TS function whose body was an expression statement:

export function formula_xxx(data, library, options) : any {
    const importerLOCAmount = data.importerLOCAmount;
    // ...
    importerLOCAmount / 2.0   // expression statement — discarded, returns undefined
}

That function returns undefined. In evaluateUserCode the result was then stored as JSON.stringify(undefined) === undefined. In the visitor that writes value onto the Formula node, neither the result === null branch nor the typeof result === 'string' branch matched, so the fallback context.value = JSON.stringify(result) also produced undefined. context.code was then deleted, leaving the Formula node with name + dependencies but no value. validateCiceroMark then rejected the document with:

ValidationException: The instance "org.accordproject.ciceromark@0.6.0.Formula" is missing the required field "value"

This reproduces with the supply-agreement-loc template in cicero-template-library PR #484 (versions: template-engine 3.0.1, cicero-core 0.26.0, concerto-core 4.1.0).

Fix

• src/utils.ts: writeFunctionToString now wraps the user's code with return when no explicit return keyword is present at the source level. Strings and comments are stripped before the keyword scan to avoid false positives (e.g. a literal 'return foo' inside the user's code). This restores the implicit-expression semantics that Cicero/Ergo formulas have always had — {{% amount / 2.0 %}} again produces a value. Existing formulas that already use return are unchanged.
• src/TemplateMarkInterpreter.ts: as defense in depth, the formula visitor in generateAgreement now throws a clear, named error if the formula's evaluation result is still undefined, instead of producing an invalid ciceromark document and surfacing the confusing missing the required field "value" message from the validator.

Test

Adds test/templates/good/helloformula-implicit-return/ — a minimal template containing the inline-formula repro {{% importerLOCAmount / 2.0 %}}. The matching snapshot asserts the produced org.accordproject.ciceromark@0.6.0.Formula node has "value": "500". Sandbox (child-process) evaluation is also covered by the existing harness, so both code paths are exercised.

The formula-invalid snapshot was regenerated: the test still rejects garbage input — only the reported character offsets shift by the length of the prepended return token. No behavior change.

Scope

Limited to accordproject/template-engine. No changes to cicero-core, ciceromark model, concerto, or other dependencies.

Test plan

• npx jest TemplateMarkInterpreter -t formula — all 6 formula tests pass (helloformula, helloformula-implicit-return, formula-now, formula-invalid, formula-no-method, formula-optional-noguard)
• npm run lint clean (no new warnings)
• npm run licchk clean
• Full npm test — remaining 7 failures are pre-existing 403 errors fetching external models (network sandbox) and a pre-existing dynamic-import-callback issue in TemplateArchiveProcessor.test; all confirmed unchanged vs. main baseline.

---

Generated by Claude Code

[coveralls]

Coverage Report for CI Build 25955862016

Coverage increased (+0.4%) to 64.606%

Details

• Coverage increased (+0.4%) from the base build.
• Patch coverage: 2 uncovered changes across 2 files (21 of 23 lines covered, 91.3%).
• 1 coverage regression across 1 file.

Uncovered Changes

File	Changed	Covered	%
src/TemplateMarkInterpreter.ts	3	2	66.67%
src/utils.ts	20	19	95.0%

Coverage Regressions

1 previously-covered line in 1 file lost coverage.

File	Lines Losing Coverage	Coverage
src/TemplateMarkInterpreter.ts	1	84.26%

---

Coverage Stats

Relevant Lines:	1153
Covered Lines:	742
Line Coverage:	64.35%
Relevant Branches:	562
Covered Branches:	366
Branch Coverage:	65.12%
Branches in Coverage %:	Yes
Coverage Strength:	5686.03 hits per line

---

💛 - Coveralls

[mttrbrts]

Follow-up push: swapped the regex-on-source detection for an AST-based check.

What changed

• wrapExpressionWithReturn now parses the user code with ts.createSourceFile(...) and inspects only sourceFile.statements (top-level only).
  • Any top-level ReturnStatement → code is kept verbatim.
  • Last top-level statement is an ExpressionStatement → emit preceding statements verbatim followed by return <expr>;.
  • Otherwise → leave unchanged; the existing "did not return a value" runtime error in TemplateMarkInterpreter.ts fires as before.
• Deleted stripStringsAndComments and RETURN_KEYWORD_RE. typescript is already a direct dependency, so no package.json changes were needed.

Why
The previous regex matched any return keyword anywhere in the source (after stripping strings/comments). That triggered a false positive when a nested function or arrow contained its own return — e.g. [1,2,3].map(x => { return x*2 })[0]. The outer expression was treated as "already returns" and never wrapped, so the resulting Formula node was missing its required value field — the same shape of bug this PR was originally fixing, just via a different route. Walking only top-level statements via the TS AST eliminates that class of false positive.

Tests added (test/utils.test.ts)

• bare expression → wrapped
• explicit top-level return → unchanged
• nested arrow with return, outer is an expression → wrapped (the regression case)
• multi-statement body with trailing expression → wraps only the trailing expression
• statements-only body (no trailing expression) → unchanged
• empty / whitespace-only → unchanged

Snapshot update
The formula-invalid snapshot was regenerated. The diff is purely positional for the three semantic Cannot find name 'THIS'/'IS'/'GARBAGE' errors, plus the disappearance of a spurious ';' expected. syntax error — the new wrapping shape (THIS\nIS\nreturn GARBAGE;) is itself syntactically valid, so the TS compiler no longer reports a syntax error on top of the identifier errors. No semantic change in the failure mode.

---

Generated by Claude Code

[Copilot]

Pull request overview

This PR fixes implicit formula expressions so inline {{% expr %}} formulas produce a ciceromark Formula.value instead of undefined, while adding a clearer guard for formulas that still do not return a value.

Changes:

• Adds wrapExpressionWithReturn and applies it when generating user-code functions.
• Adds an undefined-result guard for formula evaluation.
• Adds a regression template/snapshot and unit coverage for expression wrapping.

Reviewed changes

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

Show a summary per file

File	Description
src/utils.ts	Adds formula/condition wrapping logic before generated function output.
src/TemplateMarkInterpreter.ts	Adds explicit error handling for undefined formula results.
test/utils.test.ts	Adds unit tests for expression wrapping behavior.
test/templates/good/helloformula-implicit-return/template.md	Adds regression fixture for implicit formula return.
test/templates/good/helloformula-implicit-return/model.cto	Adds model for the regression fixture.
test/templates/good/helloformula-implicit-return/data.json	Adds data for the regression fixture.
test/__snapshots__/TemplateMarkInterpreter.test.ts.snap	Updates formula snapshots, including the new implicit-return fixture.

---

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