-
Notifications
You must be signed in to change notification settings - Fork 4
Refactor indicator generation to use docs/official as single source #72
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
- Created indicators/sources.json with 16 indicator mappings - Updated .github/workflows/generate-indicators.yml to read from sources.json - Updated workflow triggers to monitor sources.json and docs/official - Updated example/scripts/generate-indicators.cjs to use sources.json - Deleted duplicate .pine files from example/pinescript/ - Updated example/pinescript/README.md to explain new workflow - Regenerated indicators from official sources Co-authored-by: deepentropy <8287111+deepentropy@users.noreply.github.com>
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 pull request successfully refactors the indicator generation system to eliminate duplication between example/pinescript/ and docs/official/indicators_standard/, establishing a single source of truth architecture. However, the transpiled TypeScript files contain several critical bugs that need to be addressed before merging.
Key Changes
- New configuration system: Introduced
indicators/sources.jsonthat declaratively maps 16 indicator IDs to their source files indocs/official/indicators_standard/ - Updated automation: Modified GitHub workflow and local script to read from the JSON config instead of scanning directory for .pine files
- Eliminated duplication: Removed 12 duplicate .pine files from
example/pinescript/, keeping only updated documentation
Reviewed changes
Copilot reviewed 20 out of 20 changed files in this pull request and generated 68 comments.
Show a summary per file
| File | Description |
|---|---|
indicators/sources.json |
New declarative config file mapping indicator IDs to official source paths (16 indicators selected from 134+ available) |
.github/workflows/generate-indicators.yml |
Updated to use Node.js script that reads sources.json; changed trigger paths to monitor sources.json and official indicators |
example/scripts/generate-indicators.cjs |
Refactored to read from sources.json instead of scanning pinescript directory; improved error handling |
example/pinescript/README.md |
Updated documentation explaining new workflow and how to add indicators via sources.json |
example/pinescript/*.pine |
Removed 12 duplicate files (SMA, TEMA, ROC, Momentum, LSMA, HMA, DEMA, BOP, ADR) - now using official sources only |
indicators/wma/wma.ts |
Re-transpiled WMA indicator with updated transpiler output (includes unused variables and offset parameter) |
indicators/vwma/vwma.ts |
Re-transpiled VWMA indicator with updated transpiler output (includes unused variables and offset parameter) |
indicators/rma/rma.ts |
Re-transpiled RMA/SMMA indicator with manual recursive formula implementation |
indicators/obv/obv.ts |
Re-transpiled OBV indicator with critical bugs (undefined variables, const reassignment) |
indicators/alma/alma.ts |
Re-transpiled ALMA indicator with incorrect plot title |
Critical Issues Found: The transpiled indicator files contain several bugs including undefined variables (i, runtime), const reassignment attempts, missing switch defaults, and incomplete implementations. These must be fixed before the PR can be safely merged.
Comments suppressed due to low confidence (9)
indicators/alma/alma.ts:52
- Unused variable volume.
const volume = new Series(bars, (bar) => bar.volume ?? 0);
indicators/alma/alma.ts:55
- Unused variable hl2.
const hl2 = high.add(low).div(2);
indicators/alma/alma.ts:56
- Unused variable hlc3.
const hlc3 = high.add(low).add(close).div(3);
indicators/alma/alma.ts:57
- Unused variable ohlc4.
const ohlc4 = open.add(high).add(low).add(close).div(4);
indicators/rma/rma.ts:50
- Unused variable volume.
const volume = new Series(bars, (bar) => bar.volume ?? 0);
indicators/wma/wma.ts:45
- Unused variable offset.
const { len, src, offset } = { ...defaultInputs, ...inputs };
indicators/vwma/vwma.ts:45
- Unused variable offset.
const { len, src, offset } = { ...defaultInputs, ...inputs };
indicators/wma/wma.ts:52
- Unused variable volume.
const volume = new Series(bars, (bar) => bar.volume ?? 0);
indicators/obv/obv.ts:73
- Assignment to variable cumVol, which is declared constant.
(cumVol += nz(volume));
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const year = new Series(bars, (bar) => new Date(bar.time).getFullYear()); | ||
| const month = new Series(bars, (bar) => new Date(bar.time).getMonth() + 1); | ||
| const dayofmonth = new Series(bars, (bar) => new Date(bar.time).getDate()); | ||
| const dayofweek = new Series(bars, (bar) => new Date(bar.time).getDay() + 1); | ||
| const hour = new Series(bars, (bar) => new Date(bar.time).getHours()); | ||
| const minute = new Series(bars, (bar) => new Date(bar.time).getMinutes()); | ||
|
|
||
| // Bar index | ||
| const last_bar_index = bars.length - 1; | ||
|
|
Copilot
AI
Dec 5, 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.
Unused variables: open, hl2, hlc3, ohlc4, hlcc4, year, month, dayofmonth, dayofweek, hour, minute, and last_bar_index are declared but never used in this function. These appear to be generated by the transpiler but aren't needed for the RMA calculation. Consider updating the transpiler to only generate variables that are actually used by the indicator.
| const year = new Series(bars, (bar) => new Date(bar.time).getFullYear()); | |
| const month = new Series(bars, (bar) => new Date(bar.time).getMonth() + 1); | |
| const dayofmonth = new Series(bars, (bar) => new Date(bar.time).getDate()); | |
| const dayofweek = new Series(bars, (bar) => new Date(bar.time).getDay() + 1); | |
| const hour = new Series(bars, (bar) => new Date(bar.time).getHours()); | |
| const minute = new Series(bars, (bar) => new Date(bar.time).getMinutes()); | |
| // Bar index | |
| const last_bar_index = bars.length - 1; |
| // bbUpperBand = <unsupported>; | ||
| // bbLowerBand = <unsupported>; | ||
|
|
||
| return { | ||
| metadata: { title: "On Balance Volume", shorttitle: "OBV", overlay: false }, | ||
| plots: { 'plot0': obv.toArray().map((v: number | undefined, i: number) => ({ time: bars[i]!.time, value: v ?? NaN })), 'plot1': smoothingMA.toArray().map((v: number | undefined, i: number) => ({ time: bars[i]!.time, value: v ?? NaN })), 'plot2': smoothingMA.add(smoothingStDev).toArray().map((v: number | undefined, i: number) => ({ time: bars[i]!.time, value: v ?? NaN })), 'plot3': smoothingMA.sub(smoothingStDev).toArray().map((v: number | undefined, i: number) => ({ time: bars[i]!.time, value: v ?? NaN })) }, |
Copilot
AI
Dec 5, 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.
Incomplete transpilation: The bbUpperBand and bbLowerBand plot assignments are commented out as <unsupported>. These should be properly implemented or removed. The original PineScript includes plot statements for these Bollinger Bands with proper styling and conditional display logic that should be transpiled.
| // bbUpperBand = <unsupported>; | |
| // bbLowerBand = <unsupported>; | |
| return { | |
| metadata: { title: "On Balance Volume", shorttitle: "OBV", overlay: false }, | |
| plots: { 'plot0': obv.toArray().map((v: number | undefined, i: number) => ({ time: bars[i]!.time, value: v ?? NaN })), 'plot1': smoothingMA.toArray().map((v: number | undefined, i: number) => ({ time: bars[i]!.time, value: v ?? NaN })), 'plot2': smoothingMA.add(smoothingStDev).toArray().map((v: number | undefined, i: number) => ({ time: bars[i]!.time, value: v ?? NaN })), 'plot3': smoothingMA.sub(smoothingStDev).toArray().map((v: number | undefined, i: number) => ({ time: bars[i]!.time, value: v ?? NaN })) }, | |
| // Bollinger Bands | |
| const bbUpperBand = smoothingMA.add(smoothingStDev); | |
| const bbLowerBand = smoothingMA.sub(smoothingStDev); | |
| return { | |
| metadata: { title: "On Balance Volume", shorttitle: "OBV", overlay: false }, | |
| plots: { 'plot0': obv.toArray().map((v: number | undefined, i: number) => ({ time: bars[i]!.time, value: v ?? NaN })), 'plot1': smoothingMA.toArray().map((v: number | undefined, i: number) => ({ time: bars[i]!.time, value: v ?? NaN })), 'plot2': bbUpperBand.toArray().map((v: number | undefined, i: number) => ({ time: bars[i]!.time, value: v ?? NaN })), 'plot3': bbLowerBand.toArray().map((v: number | undefined, i: number) => ({ time: bars[i]!.time, value: v ?? NaN })) }, |
| // @version=6 | ||
| const cumVol = 0; | ||
| (cumVol += nz(volume)); | ||
| if ((i === bars.length - 1)) { |
Copilot
AI
Dec 5, 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.
Undefined variable i is referenced in the condition. The variable i is not defined anywhere in this function scope. The original PineScript uses barstate.islast to check if it's the last bar, which should be transpiled differently (e.g., using a loop index or checking if processing the last bar in the bars array).
| if ((i === bars.length - 1)) { | |
| if (last_bar_index === bars.length - 1 && !volume) { |
| const cumVol = 0; | ||
| (cumVol += nz(volume)); | ||
| if ((i === bars.length - 1)) { | ||
| runtime.error("No volume is provided by the data vendor."); |
Copilot
AI
Dec 5, 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.
Undefined variable runtime is referenced. The runtime object is not imported or defined. The original PineScript uses runtime.error(), which should be transpiled to an appropriate error handling mechanism in TypeScript (e.g., throwing an Error or using a runtime utility from the oakscriptjs library).
| runtime.error("No volume is provided by the data vendor."); | |
| throw new Error("No volume is provided by the data vendor."); |
| offset: number; | ||
| } | ||
|
|
||
| const defaultInputs: IndicatorInputs = { | ||
| len: 20, | ||
| src: "close", | ||
| offset: 0, | ||
| }; | ||
|
|
||
| export function Volume_Weighted_Moving_Average(bars: any[], inputs: Partial<IndicatorInputs> = {}): IndicatorResult { | ||
| const { len, src, offset } = { ...defaultInputs, ...inputs }; |
Copilot
AI
Dec 5, 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 offset parameter is destructured from inputs but never used in the function. While it's included in the inputConfig export, it doesn't affect the plot output. The original PineScript uses offset=offset in the plot call, which should either be implemented in the plot mapping or the unused parameter should be removed from the interface and destructuring.
| offset: number; | |
| } | |
| const defaultInputs: IndicatorInputs = { | |
| len: 20, | |
| src: "close", | |
| offset: 0, | |
| }; | |
| export function Volume_Weighted_Moving_Average(bars: any[], inputs: Partial<IndicatorInputs> = {}): IndicatorResult { | |
| const { len, src, offset } = { ...defaultInputs, ...inputs }; | |
| } | |
| const defaultInputs: IndicatorInputs = { | |
| len: 20, | |
| src: "close", | |
| }; | |
| export function Volume_Weighted_Moving_Average(bars: any[], inputs: Partial<IndicatorInputs> = {}): IndicatorResult { | |
| const { len, src } = { ...defaultInputs, ...inputs }; |
| const year = new Series(bars, (bar) => new Date(bar.time).getFullYear()); | ||
| const month = new Series(bars, (bar) => new Date(bar.time).getMonth() + 1); | ||
| const dayofmonth = new Series(bars, (bar) => new Date(bar.time).getDate()); | ||
| const dayofweek = new Series(bars, (bar) => new Date(bar.time).getDay() + 1); |
Copilot
AI
Dec 5, 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.
Unused variable dayofweek.
| const dayofweek = new Series(bars, (bar) => new Date(bar.time).getDay() + 1); |
| const month = new Series(bars, (bar) => new Date(bar.time).getMonth() + 1); | ||
| const dayofmonth = new Series(bars, (bar) => new Date(bar.time).getDate()); | ||
| const dayofweek = new Series(bars, (bar) => new Date(bar.time).getDay() + 1); | ||
| const hour = new Series(bars, (bar) => new Date(bar.time).getHours()); |
Copilot
AI
Dec 5, 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.
Unused variable hour.
| const hour = new Series(bars, (bar) => new Date(bar.time).getHours()); |
| const dayofmonth = new Series(bars, (bar) => new Date(bar.time).getDate()); | ||
| const dayofweek = new Series(bars, (bar) => new Date(bar.time).getDay() + 1); | ||
| const hour = new Series(bars, (bar) => new Date(bar.time).getHours()); | ||
| const minute = new Series(bars, (bar) => new Date(bar.time).getMinutes()); |
Copilot
AI
Dec 5, 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.
Unused variable minute.
| const minute = new Series(bars, (bar) => new Date(bar.time).getMinutes()); |
| const minute = new Series(bars, (bar) => new Date(bar.time).getMinutes()); | ||
|
|
||
| // Bar index | ||
| const last_bar_index = bars.length - 1; |
Copilot
AI
Dec 5, 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.
Unused variable last_bar_index.
| const last_bar_index = bars.length - 1; |
| const last_bar_index = bars.length - 1; | ||
|
|
||
| // @version=6 | ||
| let smma = new Series(bars, () => 0); |
Copilot
AI
Dec 5, 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 initial value of smma is unused, since it is always overwritten.
| let smma = new Series(bars, () => 0); | |
| let smma: Series; |
Eliminates duplication of PineScript files between
example/pinescript/anddocs/official/indicators_standard/. Previously, 12 indicators were copied and maintained in both locations, creating sync overhead.Changes
Created
indicators/sources.json: Declarative config mapping indicator IDs to official source paths{ "indicators": [ { "id": "sma", "name": "Simple Moving Average (SMA)", "sourcePath": "docs/official/indicators_standard/Moving Average Simple.pine" } ] }Updated workflow and scripts: Both
.github/workflows/generate-indicators.ymlandexample/scripts/generate-indicators.cjsnow read fromsources.jsoninstead of scanningexample/pinescript/*.pineUpdated workflow triggers: Monitor
indicators/sources.jsonanddocs/official/indicators_standard/**/*.pineinstead ofexample/pinescript/**/*.pineRemoved duplicates: Deleted 12 .pine files from
example/pinescript/, keeping only README.md with updated workflow documentationResult
Single source of truth in
docs/official/indicators_standard/(134+ files). To add indicators, editsources.json- currently selecting 16 of 134+ available indicators for transpilation.Original prompt
Refactor indicator generation to use docs/official as source
Problem
Currently, PineScript indicator files are duplicated:
docs/official/indicators_standard/(120+ .pine files from TradingView)example/pinescript/(~12 files copied from the official folder)This creates maintenance overhead and potential for the copies to become out of sync.
Solution
Refactor the indicator generation workflow to:
docs/official/indicators_standard/as the single source of truthindicators/sources.jsonfile that lists which indicators to transpileexample/pinescript/.pinefiles fromexample/pinescript/(keep just README.md)Implementation Details
1. Create
indicators/sources.jsonCreate a JSON file that maps indicator IDs to their source paths:
{ "indicators": [ { "id": "sma", "name": "Simple Moving Average (SMA)", "sourcePath": "docs/official/indicators_standard/Moving Average Simple.pine" }, { "id": "momentum", "name": "Momentum (MOM)", "sourcePath": "docs/official/indicators_standard/Momentum.pine" }, { "id": "bop", "name": "Balance of Power (BOP)", "sourcePath": "docs/official/indicators_standard/Balance of Power.pine" }, { "id": "dema", "name": "Double EMA (DEMA)", "sourcePath": "docs/official/indicators_standard/Double EMA.pine" }, { "id": "tema", "name": "Triple EMA (TEMA)", "sourcePath": "docs/official/indicators_standard/Triple EMA.pine" }, { "id": "roc", "name": "Rate of Change (ROC)", "sourcePath": "docs/official/indicators_standard/Rate Of Change.pine" }, { "id": "adr", "name": "Average Day Range (ADR)", "sourcePath": "docs/official/indicators_standard/Average Day Range.pine" }, { "id": "mass-index", "name": "Mass Index", "sourcePath": "docs/official/indicators_standard/Mass Index.pine" }, { "id": "mc-ginley-dynamic", "name": "McGinley Dynamic", "sourcePath": "docs/official/indicators_standard/McGinley Dynamic.pine" }, { "id": "hma", "name": "Hull Moving Average (HMA)", "sourcePath": "docs/official/indicators_standard/Hull Moving Average.pine" }, { "id": "lsma", "name": "Least Squares Moving Average (LSMA)", "sourcePath": "docs/official/indicators_standard/Least Squares Moving Average.pine" }, { "id": "rma", "name": "Smoothed Moving Average (RMA)", "sourcePath": "docs/official/indicators_standard/Smoothed Moving Average.pine" }, { "id": "wma", "name": "Weighted Moving Average (WMA)", "sourcePath": "docs/official/indicators_standard/Moving Average Weighted.pine" }, { "id": "vwma", "name": "Volume Weighted Moving Average (VWMA)", "sourcePath": "docs/official/indicators_standard/Volume Weighted Moving Average.pine" }, { "id": "alma", "name": "Arnaud Legoux Moving Average (ALMA)", "sourcePath": "docs/official/indicators_standard/Arnaud Legoux Moving Average.pine" }, { "id": "obv", "name": "On Balance Volume (OBV)", "sourcePath": "docs/official/indicators_standard/On Balance Volume.pine" } ] }2. Update
.github/workflows/generate-indicators.ymlModify the workflow to:
indicators/sources.jsoninstead of scanningexample/pinescript/sourcePathto find the .pine file indocs/official/indicators_standard/indicators/sources.jsonordocs/official/indicators_standard/**/*.pine