-
Notifications
You must be signed in to change notification settings - Fork 4
Transpiler Phase 2: Input System & Built-in Variables #16
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Co-authored-by: deepentropy <8287111+deepentropy@users.noreply.github.com>
Co-authored-by: deepentropy <8287111+deepentropy@users.noreply.github.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR implements Phase 2 of the PineScript transpiler, adding support for input functions and built-in variables to enable user-configurable indicator parameters. The implementation adds input system handling, calculated price sources, time series, bar state variables, and helper functions for null handling.
Key Changes:
- Input function parsing (
input.int,input.float,input.bool,input.string,input.color,input.source) with TypeScript interface generation - Built-in variables for bar state (
bar_index,barstate.*), time series (year,month, etc.), and metadata (syminfo.*,timeframe.*) - Helper functions
na()andnz()for null/undefined handling - Array literal parsing support in the parser
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| packages/oakscript-engine/src/transpiler/types.ts | Added type definitions for InputDefinition, SymbolInfo, TimeframeInfo, and SourceType |
| packages/oakscript-engine/src/transpiler/PineParser.ts | Added parseArrayLiteral() method to parse array syntax for input options parameter |
| packages/oakscript-engine/src/transpiler/PineToTS.ts | Core implementation: input parsing, interface generation, built-in variable translation, and helper function generation |
| packages/oakscript-engine/tests/transpiler-phase2.test.ts | Comprehensive test suite covering all Phase 2 features including inputs, built-ins, and integration tests |
Comments suppressed due to low confidence (2)
packages/oakscript-engine/src/transpiler/PineToTS.ts:728
- [nitpick] The
generateAssignmentfunction returnsvoidbut can be called fromgenerateExpression(line 1000-1006) which expects a string return value. When an Assignment expression with an input.* function returns early (line 711), the caller in generateExpression will return an empty string, which is correct. However, when called directly as a statement (lines 696-728), the void return is appropriate. This dual usage pattern is correct, but consider adding a return type annotation to make the void return explicit:private generateAssignment(node: ASTNode): void.
private generateAssignment(node: ASTNode): void {
if (!node.children || node.children.length < 2) return;
const left = node.children[0]!;
const right = node.children[1]!;
// Skip input.* function assignments - they're handled as function parameters
if (right.type === 'FunctionCall') {
const funcName = String(right.value || '');
if (funcName.startsWith('input.')) {
// Mark this variable as defined (it's a function parameter)
if (left.type === 'Identifier') {
const name = String(left.value || 'unknown');
this.variables.set(name, name);
}
return;
}
}
if (left.type === 'Identifier') {
const name = String(left.value || 'unknown');
const tsName = this.sanitizeIdentifier(name);
if (!this.variables.has(name)) {
this.variables.set(name, tsName);
const rightExpr = this.generateExpression(right);
this.emit(`const ${tsName} = ${rightExpr};`);
} else {
const rightExpr = this.generateExpression(right);
this.emit(`${tsName} = ${rightExpr};`);
}
}
}
packages/oakscript-engine/src/transpiler/PineParser.ts:803
- Unused variable startPos.
const startPos = this.position;
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.
| // Generate calculated price sources | ||
| this.emit('// Calculated price sources'); | ||
| this.emit('const hl2 = high.add(low).div(2);'); | ||
| this.emit('const hlc3 = high.add(low).add(close).div(3);'); | ||
| this.emit('const ohlc4 = open.add(high).add(low).add(close).div(4);'); | ||
| this.emit('const hlcc4 = high.add(low).add(close).add(close).div(4);'); | ||
| this.emit(''); | ||
|
|
||
| // Generate time series | ||
| this.emit('// Time series'); | ||
| this.emit('const year = new Series(bars, (bar) => new Date(bar.time).getFullYear());'); | ||
| this.emit('const month = new Series(bars, (bar) => new Date(bar.time).getMonth() + 1);'); | ||
| this.emit('const dayofmonth = new Series(bars, (bar) => new Date(bar.time).getDate());'); | ||
| this.emit('const dayofweek = new Series(bars, (bar) => new Date(bar.time).getDay() + 1);'); | ||
| this.emit('const hour = new Series(bars, (bar) => new Date(bar.time).getHours());'); | ||
| this.emit('const minute = new Series(bars, (bar) => new Date(bar.time).getMinutes());'); | ||
| this.emit(''); |
Copilot
AI
Nov 30, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All calculated price sources and time series are always generated regardless of whether they're used in the indicator code. This could lead to unnecessary computation overhead. Consider only generating these variables when they are actually referenced in the code by tracking their usage during the collectInfo phase, similar to how usesSyminfo and usesTimeframe are tracked.
| 'na': 'NaN', | ||
| 'true': 'true', | ||
| 'false': 'false', | ||
| 'bar_index': 'i', |
Copilot
AI
Nov 30, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The bar_index variable is mapped to i (line 1208), but the generated code doesn't create a loop with variable i. The transpiler generates code using Series objects without an explicit loop. This creates a mismatch: when users reference bar_index, it translates to undefined variable i.
Consider one of these approaches:
- If indicators should run in a loop context, wrap the body in
for (let i = 0; i < bars.length; i++) - If using Series-based approach,
bar_indexshould be handled differently (e.g., as a parameter to Series callbacks or removed as a standalone variable)
| 'bar_index': 'i', | |
| // 'bar_index' is not supported in Series context |
| // Handle barstate variables | ||
| const barstateMap: Record<string, string> = { | ||
| 'barstate.isfirst': '(i === 0)', | ||
| 'barstate.islast': '(i === bars.length - 1)', | ||
| 'barstate.isconfirmed': 'true', | ||
| 'barstate.islastconfirmedhistory': '(i === bars.length - 1)', | ||
| 'barstate.isrealtime': 'false', | ||
| 'barstate.isnew': 'true', | ||
| }; | ||
|
|
||
| if (barstateMap[name]) { | ||
| return barstateMap[name]; | ||
| } |
Copilot
AI
Nov 30, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The barstate.isfirst and barstate.islast translations reference variable i which doesn't exist in the generated code. Similar to the bar_index issue, the transpiler uses a Series-based approach without an explicit loop, making these translations invalid. These should either be implemented within a loop context or handled differently (e.g., using Series methods or removing support for these variables).
| this.emit('export interface SymbolInfo {'); | ||
| this.indent++; | ||
| this.emit('ticker: string;'); | ||
| this.emit('tickerid: string;'); | ||
| this.emit('currency: string;'); | ||
| this.emit('mintick: number;'); | ||
| this.emit('pointvalue: number;'); | ||
| this.emit('type: string;'); | ||
| this.indent--; | ||
| this.emit('}'); |
Copilot
AI
Nov 30, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The exported interface SymbolInfo may conflict with the same-named interface defined in types.ts. Since both are exported, this could cause TypeScript compilation errors if both are imported in the same file. Consider renaming the generated interface to something like GeneratedSymbolInfo or use a type re-export approach to avoid conflicts.
| this.emit('export interface TimeframeInfo {'); | ||
| this.indent++; | ||
| this.emit('period: string;'); | ||
| this.emit('multiplier: number;'); | ||
| this.emit('isintraday: boolean;'); | ||
| this.emit('isdaily: boolean;'); | ||
| this.emit('isweekly: boolean;'); | ||
| this.emit('ismonthly: boolean;'); | ||
| this.indent--; | ||
| this.emit('}'); |
Copilot
AI
Nov 30, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The exported interface TimeframeInfo may conflict with the same-named interface defined in types.ts. Since both are exported, this could cause TypeScript compilation errors if both are imported in the same file. Consider renaming the generated interface to something like GeneratedTimeframeInfo or use a type re-export approach to avoid conflicts.
| 'color.gray': '#808080', | ||
| 'color.silver': '#C0C0C0', | ||
| 'color.aqua': '#00FFFF', | ||
| 'color.lime': '#00FF00', |
Copilot
AI
Nov 30, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Duplicate color mapping: both color.green (line 359) and color.lime map to #00FF00. In standard web colors, lime is typically #00FF00 while green is #008000. Consider updating color.green to #008000 or documenting if this is intentional for PineScript compatibility.
Adds PineScript v6 input functions and built-in variables to the oakscript-engine transpiler, enabling customizable indicators with user-configurable parameters.
Input Functions
input.int(),input.float(),input.bool(),input.string(),input.color(),input.source()IndicatorInputsinterface with proper typesoptionsarrays ininput.string()Partial<IndicatorInputs>Built-in Variables
hl2,hlc3,ohlc4,hlcc4as Series calculationsyear,month,dayofmonth,dayofweek,hour,minutebarstate.isfirst→(i === 0),barstate.islast→(i === bars.length - 1)syminfo.ticker,syminfo.mintick, etc. (generates interface when used)timeframe.period,timeframe.isintraday, etc.Helper Functions
na()andnz()generated as local helpers in outputParser
ArrayLiteralnode type for parsing[...]in options parameterOriginal prompt
Transpiler Phase 2: Input System & Built-in Variables
Objective
Enhance the oakscript-engine transpiler to support PineScript v6 input functions and built-in variables, enabling customizable indicators with user-configurable parameters.
Context
Phase 1 has been merged and the transpiler now supports:
var,:=reassignment)if/elseblocks,for/whileloops,switchexpressions[n], tuple destructuring,break/continueFeatures to Implement
1. Input Functions (Critical)
input.int()input.float()input.bool()input.string()input.color()input.source()TypeScript output - generate interface:
2. Calculated Price Sources
TypeScript output:
3.
bar_indexandbarstate.*VariablesTypeScript output:
4. Time Variables
TypeScript output:
5. Symbol Info Variables
TypeScript output:
6. Timeframe Variables
TypeScript output:
7.
na()andnz()Functions (Critical)TypeScript output:
Test Cases
test-inputs.pine