Skip to content

fix(pine): interpolate pine_check error messages and annotate the source line - #505

Open
Bern-code wants to merge 2 commits into
tradesdontlie:mainfrom
Bern-code:pine-check-annotated-errors
Open

Bern-code wants to merge 2 commits into
tradesdontlie:mainfrom
Bern-code:pine-check-annotated-errors

Conversation

@Bern-code

Copy link
Copy Markdown

The bug

pine_check returns TradingView's message templates verbatim, so a real compile error reads:

Undeclared identifier "{identifier}"

pine-facade doesn't interpolate its own messages. It sends the values separately in a ctx map next to the template, and check() in src/core/pine.js was reading e.message while dropping e.ctx:

{"code": "CE10272",
 "ctx": {"identifier": "risk"},
 "message": "Undeclared identifier \"{identifier}\"",
 "start": {"line": 139, "column": 5}}

So the one field the caller actually needs — which identifier — was being thrown away on every error.

The fix

Fill the placeholders from ctx, pass the error code through, and add an annotated view showing the offending line with a caret under the reported column:

 139 |     risk := atrVal * slMult
     |     ^-- Undeclared identifier "risk"

A placeholder with no matching ctx key is left as-is rather than blanked, so an unrecognised template degrades to today's behaviour instead of losing text. Warnings get the same treatment. annotated is omitted entirely when the script is clean, so nothing changes for a passing check.

This matters most on builds where the Pine editor has no React fiber — pine_set_source / pine_smart_compile / pine_get_errors all fail there (several open PRs are chasing variants of this), and pine_check becomes the only compile path left. Its output is then the entire error report.

What I ran

npm run lint       # 0 errors (4 pre-existing warnings, untouched)
npm run test:unit  # 160/160 pass

Against the live pine-facade endpoint, 15 real third-party Pine scripts (v5 and v6, 2.5KB–20KB) were checked end to end: 13 compiled clean, 2 had genuine errors. Both error cases printed raw {identifier} / {funId} templates before this change and read correctly after:

 139 |     risk := atrVal * slMult
     |     ^-- Undeclared identifier "risk"

  87 |     plot(smaFast * (1 - t) + smaSlow * t, title="Band " + str.tostring(i),
     |                                                 ^-- Cannot call "plot" with argument "title"="call "operator +" (series string)". An argument of "series string" type was used but a "const string"  is expected.

The second one is the case that convinced me this was worth filing — untouched, it names neither the function nor the argument.

npm run test:e2e was not run: no TradingView Desktop on this machine, and this path never opens a CDP connection. check() posts to pine-facade from the server process, so there is nothing browser-side for the change to affect.

Tests

tests/pine_annotate.test.js, 8 tests, fetch stubbed — no network, no TradingView. Covers caret column alignment, out-of-range and missing-column input, multiple issues, ctx interpolation for errors and warnings, an unknown placeholder, and annotated being absent on a clean compile.

🤖 Generated with Claude Code

https://claude.ai/code/session_01DsdBoQLyPJihjuyYbPkda6

…rce line

pine_check printed TradingView's raw message templates, so a real compile error
came back as:

    Undeclared identifier "{identifier}"

pine-facade does not interpolate its messages — it sends the values separately
in a `ctx` map alongside the template, and `check()` was dropping that field:

    {"code": "CE10272",
     "ctx": {"identifier": "risk"},
     "message": "Undeclared identifier \"{identifier}\"",
     "start": {"line": 139, "column": 5}}

Fill the placeholders from `ctx`, pass the error `code` through, and add an
`annotated` view that shows the offending line with a caret under the reported
column:

     139 |     risk := atrVal * slMult
         |     ^-- Undeclared identifier "risk"

Placeholders with no matching `ctx` key are left alone rather than blanked, so
a template this code has not seen degrades to today's behaviour instead of
losing text. Warnings get the same treatment.

This matters most on builds where the Pine editor has no React fiber and every
pine_* editor tool fails: pine_check is then the only compile path left, and
its output is the entire error report.

Adds tests/pine_annotate.test.js — 8 tests, fetch stubbed, no network or
TradingView needed.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DsdBoQLyPJihjuyYbPkda6
Copilot AI lite review requested due to automatic review settings September 4, 2026 12:57

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

annotate() should defensively handle CRLF input and clamp untrusted column values to avoid misrendered output and potential large-string allocations.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Improves pine_check output by interpolating pine-facade error/warning message templates using the provided ctx map, and by adding an annotated string that shows the offending source line with a caret at the reported column to make compiler feedback actionable without opening TradingView.

Changes:

  • Add {placeholder} interpolation (and pass through code) for errors and warnings returned by pine-facade.
  • Add annotated output via a new annotate() helper, included only when there are warnings/errors.
  • Add unit tests and update documentation/tool descriptions to reflect the enhanced pine_check output.
File summaries
File Description
tests/pine_annotate.test.js Adds unit coverage for interpolation and annotated caret rendering.
src/tools/pine.js Updates pine_check tool description to mention annotated.
src/core/pine.js Implements message interpolation, surfaces code, and adds annotated generation.
README.md Documents pine_check’s new annotated output with an example.
package.json Ensures the new unit test runs under test:unit / test:all.
CLAUDE.md Updates the Pine workflow docs to recommend starting with pine_check and describes annotated.
Review details
  • Files reviewed: 6/6 changed files
  • Comments generated: 2
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/core/pine.js Outdated
// Render the offending source lines with the compiler's complaint attached, so
// the caller can fix an error without counting columns by hand.
export function annotate(source, issues) {
const lines = source.split('\n');
Comment thread src/core/pine.js Outdated
Comment on lines +207 to +209
const src = lines[ln - 1];
const gutter = String(ln).padStart(4, ' ');
const pad = ' '.repeat(5) + '|' + ' '.repeat(1 + Math.max(0, (issue.column ?? 1) - 1));
Both from Copilot's review:

- split on /\r?\n/ so a CRLF script doesn't leave a stray CR in the rendered
  line, which pushed the caret out of alignment with the text above it
- clamp the column into [1, line length + 1] and require an integer, so a
  malformed column can't allocate a huge pad string

The upper bound is length + 1, not length: "Syntax error at input 'end of line
without line continuation'" legitimately points one past the last character,
and clamping to the text would have moved that caret onto it.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DsdBoQLyPJihjuyYbPkda6
@Bern-code

Bern-code commented Sep 4, 2026

Copy link
Copy Markdown
Author

Both Copilot points addressed in 8e17be2:

  • CRLFannotate() now splits on /\r?\n/. A CRLF script was leaving a stray \r in the rendered line, which pushed the caret out of alignment with the text above it.
  • Column clamp — the column is now required to be an integer and clamped before it reaches repeat().

One nuance on the clamp: the upper bound is line.length + 1, not line.length. Syntax error at input "end of line without line continuation" legitimately points one character past the end, and clamping to the text would have moved that caret onto the last character and made it read as if the problem were there. So the clamp kills the huge-allocation case without changing any real compiler output.

Three tests added for it — CRLF alignment, a column: 9_999_999 that stays under 200 chars, and the end-of-line caret still landing at src.length. 163/163 unit tests pass, lint clean.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants