feat(trading-signals): runtime input shapes and a generic indicator CLI by bennycode · Pull Request #1319 · bennycode/trading-signals

bennycode · 2026-08-22T13:05:28Z

What

Two connected changes to the trading-signals package:

1. Every indicator declares its input shape at runtime

TypeScript input types are erased at runtime, so a generic consumer (a CLI, the visual strategy builder, docs tooling) cannot know whether an indicator consumes a close series or full candles — and feeding the wrong shape silently produces NaN rather than an error.

Each indicator now carries inputShape, a runtime value from the new IndicatorInputShape constant (value, high-low, high-low-close, high-low-close-volume, open-high-low-close, open-high-low-close-volume). The declaration is compiler-verified: InputShapeOf<Input> maps each class's Input generic to the one correct literal, so declaring a wrong shape (or forgetting one on a new indicator) fails to compile. MovingAverage subclasses inherit value from the family base; 117 classes declare explicitly.

2. A generic `trading-signals` CLI

trading-signals atr 14 --csv candles.csv
cat candles.json | trading-signals rsi 14
trading-signals stochasticoscillator kPeriod=5 dPeriod=3 kSlowingPeriod=3 --csv candles.csv

The indicator is resolved generically from the package exports (lowercased class name), so the CLI never imports individual indicators — new ones are available the moment they're exported. Abstract bases are filtered out by checking that the prototype chain actually carries an update implementation.
Input formats auto-detected: CSV with header, NDJSON, or a JSON array; stdin or --csv. Field aliases o/h/l/c/v accepted; time/date/timestamp passed through.
The declared inputShape decides which candle fields are fed — closes for value-series indicators, the exact candle fields for the rest, with a clear error naming any missing field.
Constructor parameters from the command line: numbers positionally, key=value pairs collected into one config object, covering both constructor styles in the library.
Output streams one JSON line per stable result as candles arrive, so the CLI composes with live candle feeds over a pipe; --last prints only the final value.
Zero new dependencies (node:util + node:readline); the bin is excluded from coverage since it only wires stdin/stdout to the tested runCli core.

Verification

1760 tests (21 new for the CLI, with exact pinned values per input shape), 100% coverage thresholds passing, typecheck and lint clean. The built bin verified against CSV files, piped NDJSON, and piped JSON arrays.

TypeScript input types are erased at runtime, so generic consumers (CLIs, strategy builders, docs) could not know whether an indicator consumes a close series or full candles without hardcoding knowledge per indicator — feeding the wrong shape produces NaN silently instead of an error. Every indicator now declares inputShape, a runtime value from the new IndicatorInputShape constant. The declaration is compiler-verified: InputShapeOf<Input> maps the class's Input generic to the one correct literal, so a mismatching shape fails to compile. MovingAverage subclasses inherit VALUE from the family base.

A trading-signals binary that computes any exported indicator over candle data, resolving the indicator generically from the package exports — new indicators become available without touching the CLI. - Input: CSV with a header row, NDJSON, or a JSON array — auto-detected, from stdin or --csv. Field aliases o/h/l/c/v are accepted and a time/date/timestamp column is passed through to the output. - The indicator's runtime inputShape decides which candle fields are fed, so value-series indicators get closes and candle-based ones get the fields they declare. - Constructor parameters come from the command line: numbers are passed positionally, key=value pairs are collected into one config object (e.g. stochasticoscillator kPeriod=5 dPeriod=3 kSlowingPeriod=3). - Output streams one JSON line per stable result as candles arrive, so the CLI composes with live feeds; --last prints only the final value. - Zero new dependencies: node:util parseArgs and node:readline.

Copilot

🟡 Changes recommended

Unresolved critical CLI source-mapping and dependency-construction defects, along with parsing and configuration issues, block approval.

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

Pull request overview

Adds compiler-verified runtime input-shape metadata and a generic streaming CLI for trading indicators.

Changes:

Adds IndicatorInputShape metadata across indicators.
Supports CSV, NDJSON, and JSON-array CLI inputs with streamed output.
Updates tests, documentation, package metadata, and coverage configuration.

File summaries

File	Reviewed change
`packages/trading-signals/vitest.config.ts`	Excludes the CLI wrapper from coverage.
`packages/trading-signals/src/volume/WAD/WAD.ts`	Adds runtime input-shape metadata.
`packages/trading-signals/src/volume/VWMA/VWMA.ts`	Adds runtime input-shape metadata.
`packages/trading-signals/src/volume/VROC/VROC.ts`	Adds runtime input-shape metadata.
`packages/trading-signals/src/volume/RVOL/RVOL.ts`	Adds runtime input-shape metadata.
`packages/trading-signals/src/volume/PVT/PVT.ts`	Adds runtime input-shape metadata.
`packages/trading-signals/src/volume/PVO/PVO.ts`	Adds runtime input-shape metadata.
`packages/trading-signals/src/volume/PVI/PVI.ts`	Adds runtime input-shape metadata.
`packages/trading-signals/src/volume/NVI/NVI.ts`	Adds runtime input-shape metadata.
`packages/trading-signals/src/volume/MARKETFI/MarketFacilitationIndex.ts`	Adds runtime input-shape metadata.
`packages/trading-signals/src/volume/KVO/KVO.ts`	Adds runtime input-shape metadata.
`packages/trading-signals/src/volume/FI/ForceIndex.ts`	Adds runtime input-shape metadata.
`packages/trading-signals/src/volume/EMV/EMV.ts`	Adds runtime input-shape metadata.
`packages/trading-signals/src/volume/CMF/CMF.ts`	Adds runtime input-shape metadata.
`packages/trading-signals/src/volume/ADOSC/ADOSC.ts`	Adds runtime input-shape metadata.
`packages/trading-signals/src/volume/AD/AD.ts`	Adds runtime input-shape metadata.
`packages/trading-signals/src/volatility/UI/UlcerIndex.ts`	Adds runtime input-shape metadata.
`packages/trading-signals/src/volatility/TR/TR.ts`	Adds runtime input-shape metadata.
`packages/trading-signals/src/volatility/SQUEEZE/TTMSqueeze.ts`	Adds runtime input-shape metadata.
`packages/trading-signals/src/volatility/RVI/RelativeVolatilityIndex.ts`	Adds runtime input-shape metadata.
`packages/trading-signals/src/volatility/RSV/RogersSatchellVolatility.ts`	Adds runtime input-shape metadata.
`packages/trading-signals/src/volatility/PO/ProjectionOscillator.ts`	Adds runtime input-shape metadata.
`packages/trading-signals/src/volatility/PB/PercentB.ts`	Adds runtime input-shape metadata.
`packages/trading-signals/src/volatility/NATR/NATR.ts`	Adds runtime input-shape metadata.
`packages/trading-signals/src/volatility/MI/MassIndex.ts`	Adds runtime input-shape metadata.
`packages/trading-signals/src/volatility/MAD/MAD.ts`	Adds runtime input-shape metadata.
`packages/trading-signals/src/volatility/KC/KeltnerChannels.ts`	Adds runtime input-shape metadata.
`packages/trading-signals/src/volatility/IQR/IQR.ts`	Adds runtime input-shape metadata.
`packages/trading-signals/src/volatility/GAPO/GAPO.ts`	Adds runtime input-shape metadata.
`packages/trading-signals/src/volatility/DC/DonchianChannels.ts`	Adds runtime input-shape metadata.
`packages/trading-signals/src/volatility/CVI/CVI.ts`	Adds runtime input-shape metadata.
`packages/trading-signals/src/volatility/CHOP/CHOP.ts`	Adds runtime input-shape metadata.
`packages/trading-signals/src/volatility/BBW/BollingerBandsWidth.ts`	Adds runtime input-shape metadata.
`packages/trading-signals/src/volatility/BBANDS/BollingerBands.ts`	Adds runtime input-shape metadata.
`packages/trading-signals/src/volatility/ATR/ATR.ts`	Adds runtime input-shape metadata.
`packages/trading-signals/src/volatility/ABANDS/AccelerationBands.ts`	Adds runtime input-shape metadata.
`packages/trading-signals/src/trend/ZIGZAG/ZigZag.ts`	Adds runtime input-shape metadata.
`packages/trading-signals/src/trend/WSMA/WSMA.ts`	Adds runtime input-shape metadata.
`packages/trading-signals/src/trend/VWAP/VWAP.ts`	Adds runtime input-shape metadata.
`packages/trading-signals/src/trend/VSTOP/VolatilityStop.ts`	Adds runtime input-shape metadata.
`packages/trading-signals/src/trend/VI/VortexIndicator.ts`	Adds runtime input-shape metadata.
`packages/trading-signals/src/trend/VHF/VHF.ts`	Adds runtime input-shape metadata.
`packages/trading-signals/src/trend/TEMA/TEMA.ts`	Adds runtime input-shape metadata.
`packages/trading-signals/src/trend/SWING_LOW/SwingLow.ts`	Adds runtime input-shape metadata.
`packages/trading-signals/src/trend/SWING_HIGH/SwingHigh.ts`	Adds runtime input-shape metadata.
`packages/trading-signals/src/trend/SUPERTREND/SuperTrend.ts`	Adds runtime input-shape metadata.
`packages/trading-signals/src/trend/RWI/RandomWalkIndex.ts`	Adds runtime input-shape metadata.
`packages/trading-signals/src/trend/PSAR/PSAR.ts`	Adds runtime input-shape metadata.
`packages/trading-signals/src/trend/MAMA/MAMA.ts`	Adds runtime input-shape metadata.
`packages/trading-signals/src/trend/MA/MovingAverage.ts`	Adds inherited input-shape metadata.
`packages/trading-signals/src/trend/LINREG/LinearRegression.ts`	Adds runtime input-shape metadata.
`packages/trading-signals/src/trend/ICHIMOKU/IchimokuCloud.ts`	Adds runtime input-shape metadata.
`packages/trading-signals/src/trend/HT/HTTrendline.ts`	Adds runtime input-shape metadata.
`packages/trading-signals/src/trend/HILO/GannHiLo.ts`	Adds runtime input-shape metadata.
`packages/trading-signals/src/trend/HIGHER_LOW_TRAIL/HigherLowTrail.ts`	Adds runtime input-shape metadata.
`packages/trading-signals/src/trend/DX/DX.ts`	Adds runtime input-shape metadata.
`packages/trading-signals/src/trend/DMA/DMA.ts`	Adds runtime input-shape metadata.
`packages/trading-signals/src/trend/DEMA/DEMA.ts`	Adds runtime input-shape metadata.
`packages/trading-signals/src/trend/CKS/ChandeKrollStop.ts`	Adds runtime input-shape metadata.
`packages/trading-signals/src/trend/CE/ChandelierExit.ts`	Adds runtime input-shape metadata.
`packages/trading-signals/src/trend/BREAKOUT_BAR_LOW/BreakoutBarLow.ts`	Adds runtime input-shape metadata.
`packages/trading-signals/src/trend/AROON/Aroon.ts`	Adds runtime input-shape metadata.
`packages/trading-signals/src/trend/ALLIGATOR/GatorOscillator.ts`	Adds runtime input-shape metadata.
`packages/trading-signals/src/trend/ALLIGATOR/Alligator.ts`	Adds runtime input-shape metadata.
`packages/trading-signals/src/trend/ADXR/ADXR.ts`	Adds runtime input-shape metadata.
`packages/trading-signals/src/trend/ADX/ADX.ts`	Adds runtime input-shape metadata.
`packages/trading-signals/src/momentum/WT/WaveTrend.ts`	Adds runtime input-shape metadata.
`packages/trading-signals/src/momentum/WILLR/WilliamsR.ts`	Adds runtime input-shape metadata.
`packages/trading-signals/src/momentum/WAE/WaddahAttarExplosion.ts`	Adds runtime input-shape metadata.
`packages/trading-signals/src/momentum/ULTOSC/UltimateOscillator.ts`	Adds runtime input-shape metadata.
`packages/trading-signals/src/momentum/TSI/TSI.ts`	Adds runtime input-shape metadata.
`packages/trading-signals/src/momentum/TRIX/TRIX.ts`	Adds runtime input-shape metadata.
`packages/trading-signals/src/momentum/TDS/TDS.ts`	Adds runtime input-shape metadata.
`packages/trading-signals/src/momentum/STOCHRSI/StochasticRSI.ts`	Adds runtime input-shape metadata.
`packages/trading-signals/src/momentum/STOCH/StochasticOscillator.ts`	Adds runtime input-shape metadata.
`packages/trading-signals/src/momentum/STC/STC.ts`	Adds runtime input-shape metadata.
`packages/trading-signals/src/momentum/SMI/SMI.ts`	Adds runtime input-shape metadata.
`packages/trading-signals/src/momentum/SI/SwingIndex.ts`	Adds runtime input-shape metadata.
`packages/trading-signals/src/momentum/SI/AccumulativeSwingIndex.ts`	Adds runtime input-shape metadata.
`packages/trading-signals/src/momentum/RVGI/RelativeVigorIndex.ts`	Adds runtime input-shape metadata.
`packages/trading-signals/src/momentum/RSI/RSI.ts`	Adds runtime input-shape metadata.
`packages/trading-signals/src/momentum/ROC/ROC.ts`	Adds runtime input-shape metadata.
`packages/trading-signals/src/momentum/RMI/RMI.ts`	Adds runtime input-shape metadata.
`packages/trading-signals/src/momentum/REI/REI.ts`	Adds runtime input-shape metadata.
`packages/trading-signals/src/momentum/RCI/RCI.ts`	Adds runtime input-shape metadata.
`packages/trading-signals/src/momentum/QSTICK/Qstick.ts`	Adds runtime input-shape metadata.
`packages/trading-signals/src/momentum/QQE/QQE.ts`	Adds runtime input-shape metadata.
`packages/trading-signals/src/momentum/PSO/PremierStochastic.ts`	Adds runtime input-shape metadata.
`packages/trading-signals/src/momentum/PSL/PSL.ts`	Adds runtime input-shape metadata.
`packages/trading-signals/src/momentum/PPO/PPO.ts`	Adds runtime input-shape metadata.
`packages/trading-signals/src/momentum/PMO/PMO.ts`	Adds runtime input-shape metadata.
`packages/trading-signals/src/momentum/PGO/PGO.ts`	Adds runtime input-shape metadata.
`packages/trading-signals/src/momentum/OBV/OBV.ts`	Adds runtime input-shape metadata.
`packages/trading-signals/src/momentum/MOM/MOM.ts`	Adds runtime input-shape metadata.
`packages/trading-signals/src/momentum/MFI/MFI.ts`	Adds runtime input-shape metadata.
`packages/trading-signals/src/momentum/MACD/MACD.ts`	Adds runtime input-shape metadata.
`packages/trading-signals/src/momentum/LRSI/LaguerreRSI.ts`	Adds runtime input-shape metadata.
`packages/trading-signals/src/momentum/KST/KST.ts`	Adds runtime input-shape metadata.
`packages/trading-signals/src/momentum/IMI/IMI.ts`	Adds runtime input-shape metadata.
`packages/trading-signals/src/momentum/IBS/IBS.ts`	Adds runtime input-shape metadata.
`packages/trading-signals/src/momentum/FISHER/FisherTransform.ts`	Adds runtime input-shape metadata.
`packages/trading-signals/src/momentum/ERI/ElderRay.ts`	Adds runtime input-shape metadata.
`packages/trading-signals/src/momentum/ER/ER.ts`	Adds runtime input-shape metadata.
`packages/trading-signals/src/momentum/DPO/DPO.ts`	Adds runtime input-shape metadata.
`packages/trading-signals/src/momentum/DOSC/DerivativeOscillator.ts`	Adds runtime input-shape metadata.
`packages/trading-signals/src/momentum/DI/DisparityIndex.ts`	Adds runtime input-shape metadata.
`packages/trading-signals/src/momentum/DEM/DeMarker.ts`	Adds runtime input-shape metadata.
`packages/trading-signals/src/momentum/CRSI/ConnorsRSI.ts`	Adds runtime input-shape metadata.
`packages/trading-signals/src/momentum/COPPOCK/CoppockCurve.ts`	Adds runtime input-shape metadata.
`packages/trading-signals/src/momentum/CMO/CMO.ts`	Adds runtime input-shape metadata.
`packages/trading-signals/src/momentum/CG/CG.ts`	Adds runtime input-shape metadata.
`packages/trading-signals/src/momentum/CFO/CFO.ts`	Adds runtime input-shape metadata.
`packages/trading-signals/src/momentum/CCI/CCI.ts`	Adds runtime input-shape metadata.
`packages/trading-signals/src/momentum/BOP/BOP.ts`	Adds runtime input-shape metadata.
`packages/trading-signals/src/momentum/APO/APO.ts`	Adds runtime input-shape metadata.
`packages/trading-signals/src/momentum/AO/AO.ts`	Adds runtime input-shape metadata.
`packages/trading-signals/src/momentum/AC/AC.ts`	Adds runtime input-shape metadata.
`packages/trading-signals/src/cli/runCli.ts`	Implements CLI parsing, input mapping, and execution.
`packages/trading-signals/src/cli/runCli.test.ts`	Adds CLI behavior and error tests.
`packages/trading-signals/src/cli/cli.ts`	Adds the executable CLI wrapper.
`packages/trading-signals/src/base/Period.ts`	Adds scalar input metadata.
`packages/trading-signals/src/base/Indicator.ts`	Defines runtime shapes and compiler mappings.
`packages/trading-signals/src/base/Indicator.test.ts`	Updates the indicator test fixture.
`packages/trading-signals/README.md`	Documents CLI usage.
`packages/trading-signals/package.json`	Registers the CLI binary.
`packages/trading-signals/CLAUDE.md`	Documents the input-shape contract.

Review details

Suppressed comments (8)

packages/trading-signals/src/cli/runCli.test.ts:124

The repository testing rule requires explanations for assertions to be passed as the expect message rather than a preceding comment, so this formula rationale is not shown alongside the failure output. Move the expectation into the assertion message.

    // (close 100 - open 98) / (high 105 - low 95) = 0.2 on every candle.
    expect(lines).toEqual([JSON.stringify({value: 0.2})]);

packages/trading-signals/src/cli/runCli.ts:103

An empty positional argument is not a number, but Number(param) converts it (and whitespace-only arguments) to 0, so malformed constructor input is silently accepted and can create an unusable zero-period indicator. Validate the raw argument before accepting the conversion.

      const value = Number(param);

      if (Number.isNaN(value)) {
        throw new Error(`Invalid indicator parameter "${param}". Use a number or key=value.`);

packages/trading-signals/src/cli/runCli.ts:111

The config-value branch has the same coercion issue: interval= and whitespace values become 0 instead of producing the documented invalid-parameter error. Validate the unconverted value and require a finite number here as well.

      const value = Number(param.slice(separator + 1));

      if (Number.isNaN(value)) {
        throw new Error(`Invalid indicator parameter "${param}". The value has to be a number.`);

packages/trading-signals/src/cli/runCli.ts:178

The OBV fixture includes open, but OBV's calculation only reads close and volume, so a mutation that removes open: requireField(row, 'open') from this OPEN_HIGH_LOW_CLOSE_VOLUME branch would still pass the current tests. Add a missing-open case or exercise an OHLCV indicator whose result depends on open so the full shape contract is actually guarded.

    case IndicatorInputShape.OPEN_HIGH_LOW_CLOSE_VOLUME:
      return {
        close: requireField(row, 'close'),
        high: requireField(row, 'high'),
        low: requireField(row, 'low'),
        open: requireField(row, 'open'),
        volume: requireField(row, 'volume'),

packages/trading-signals/src/cli/runCli.ts:163

The volume fixture is constant: MFI returns its neutral 50 when there is no flow, and OBV returns 0 when closes are unchanged. Consequently, removing any of the HIGH_LOW_CLOSE_VOLUME field mappings (including volume) would still pass the current assertions, so this test does not guard the volume-aware input contract. Use changing prices/volumes or explicit missing-field cases.

    case IndicatorInputShape.HIGH_LOW_CLOSE_VOLUME:
      return {
        close: requireField(row, 'close'),
        high: requireField(row, 'high'),
        low: requireField(row, 'low'),
        volume: requireField(row, 'volume'),

packages/trading-signals/src/cli/runCli.ts:157

The ATR fixture has identical high/low values, and getTrueRange() only uses the current candle's high/low plus the previous candle's close. A mutation that removes close: requireField(row, 'close') from this HIGH_LOW_CLOSE mapping would therefore still produce the asserted 10. Add an explicit missing-close case or use a gap-sensitive input so the required close field is protected.

    case IndicatorInputShape.HIGH_LOW_CLOSE:
      return {close: requireField(row, 'close'), high: requireField(row, 'high'), low: requireField(row, 'low')};

packages/trading-signals/src/cli/runCli.ts:230

The JSON-array branch buffers every raw line and only parses/yields after EOF. This supported format therefore does not emit results as candles arrive and retains both the raw input and parsed array, creating an avoidable memory spike for large files or pipes. Parse arrays incrementally or document JSON arrays as batch-only.

    if (format === 'json-array') {
      buffered.push(line);
      continue;

packages/trading-signals/vitest.config.ts:10

cli.ts is excluded as if it only wires streams, but lines 15-16 contain behavior that selects the CSV source and is not exercised by the runCli tests. This lets the valid --csv=... path regress while the package still reports 100% coverage; cover the entrypoint's option handling or move all option parsing into the tested core.

Files reviewed: 126/126 changed files
Comments generated: 10
Review effort level: Lite

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

+const csvFlagIndex = process.argv.indexOf('--csv');
+const csvPath = csvFlagIndex === -1 ? undefined : process.argv[csvFlagIndex + 1];


+    // A close in the middle of a constant range pins every stochastic line at 50.
+    expect(lines.at(-1)).toBe(JSON.stringify({value: {stochD: 50, stochJ: 50, stochK: 50}}));


+}
+
+function parseCsvLine(line: string): string[] {
+  return line.split(',').map(cell => cell.trim());


+    case IndicatorInputShape.VALUE:
+      return requireField(row, 'close');


+  const value = Number(row[field]);
+
+  if (row[field] === undefined || Number.isNaN(value)) {
+    throw new Error(`Candle is missing a numeric "${field}" field: ${JSON.stringify(row)}`);
+  }


+  const constructorArgs = parseConstructorArgs(params);
+  const indicator = new IndicatorClass(...constructorArgs);


+      if (Number.isNaN(value)) {
+        throw new Error(`Invalid indicator parameter "${param}". The value has to be a number.`);
+      }
+      config[key] = value;


 * @see https://www.investopedia.com/terms/p/ppo.asp
 */
 export class PVO extends ZeroCrossSeries {
+  override readonly inputShape = IndicatorInputShape.VALUE;


 * @see https://arongroups.co/technical-analyze/relative-volume-indicator/
 */
 export class RVOL extends IndicatorSeries {
+  override readonly inputShape = IndicatorInputShape.VALUE;


 * @see https://www.investopedia.com/terms/v/volumerateofchange.asp
 */
 export class VROC extends ZeroCrossSeries {
+  override readonly inputShape = IndicatorInputShape.VALUE;


- New VOLUME input shape: VROC, PVO, and RVOL consume a volume series but their scalar input made the CLI feed closes, silently computing price-based results. The number branch of InputShapeOf now allows VALUE or VOLUME (the type system cannot tell the two series apart, so that distinction is the one part a human declares), and the CLI feeds the volume field accordingly. - Factories for dependency-injected indicators: MACD and BollingerBandsWidth take indicator instances, so generic construction from numbers crashed at first use. Flat CLI parameters are now wired into the dependencies; missing parameters fail with usage messages. - Dotted config keys nest (signalThresholds.overbought=4), so config indicators like PGO and STC no longer silently drop thresholds. - Quote-aware CSV parsing: quoted cells, commas inside quotes, and "" escapes. - Empty CSV cells are rejected instead of passing as 0 (Number('')). - The bin accepts both --csv path and --csv=path spellings. - Assertion rationales moved from comments into expect messages.

bennycode · 2026-08-23T08:38:10Z

Addressed the review in 6a93453:

New VOLUME input shape: VROC, PVO, and RVOL declare it, and the CLI feeds the volume field instead of the close. The scalar branch of InputShapeOf allows VALUE or VOLUME since the type system cannot tell a price series from a volume series — that distinction is the one part a human declares. Regression: constant closes with quadrupling volume must register as +300% VROC.
MACD and BollingerBandsWidth construct through factories that wire their indicator dependencies from flat numeric parameters (macd 12 26 9); missing parameters fail with a usage message instead of crashing at first update.
Dotted config keys nest, so pgo interval=14 signalThresholds.overbought=4 reaches the nested config instead of being silently dropped.
CSV parsing is quote-aware (commas inside quotes, "" escapes) with a regression test.
Empty CSV cells are rejected instead of passing as 0 via Number('').
The bin accepts both --csv path and --csv=path.
Assertion rationales moved from comments into expect messages per the repo testing rule.

1766 tests, 100% coverage thresholds passing, lint and typecheck clean.

Copilot

🟡 Changes recommended

Unresolved critical and moderate findings remain in the base API contract and CLI handling.

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

Review details

Suppressed comments (8)

Previously missed (3) — in code that hasn't changed since the last review.

packages/trading-signals/src/cli/runCli.ts:158

Number.isNaN accepts Infinity and -Infinity, so sma Infinity is treated as valid and can leave the indicator permanently unstable (or propagate a non-finite setting). Use Number.isFinite for positional numeric parameters and apply the same guard to the key=value branch.

      const value = Number(param);

      if (Number.isNaN(value)) {

packages/trading-signals/src/cli/runCli.ts:165

Number('') is 0, so a command such as sma interval= is accepted and creates a zero-length SMA that emits 0 on the first candle instead of rejecting the malformed parameter. Check the raw value for blank text before numeric coercion, as requireField already does for CSV cells.

      const key = param.slice(0, separator);
      const value = Number(param.slice(separator + 1));

      if (Number.isNaN(value)) {

packages/trading-signals/src/cli/runCli.ts:319

Selecting json-array appends every line to buffered until EOF, then JSON.parse materializes the whole array before yielding any candle. Large arrays therefore require full-input buffering (plus the parsed array) and live array feeds cannot emit incrementally, despite the CLI's streaming guarantee. Implement an incremental array parser or document/limit JSON arrays to batch input.

    if (format === 'json-array') {
      buffered.push(line);
      continue;

packages/trading-signals/src/base/Indicator.test.ts:6

This line only supplies the new abstract member to the test double; no expectation checks its runtime value. Removing the inputShape contract and this declaration leaves all existing assertions green, so the public metadata behavior is not directly covered. Add an assertion that a concrete indicator exposes its declared literal, with a separate candle/volume case if needed.

    override readonly inputShape = IndicatorInputShape.VALUE;

packages/trading-signals/src/cli/runCli.test.ts:167

The escaped-quote case is only in the first (warm-up) CSV row, while the assertion checks the result from the second row, so a parser mutation that mishandles "" still passes. Make the escaped-quote timestamp occur on an emitted row or assert the parsed time directly.

  it('parses quoted CSV cells including commas and escaped quotes', async () => {
    const csv = ['"time","close"', '"day ""one""","10"', '"Jan 02, 2026","20"'].join('\n');

    const lines = await run(['sma', '2'], csv);

    expect(lines).toEqual([JSON.stringify({time: 'Jan 02, 2026', value: 15})]);

packages/trading-signals/src/cli/runCli.test.ts:154

This assertion cannot distinguish a working deviationMultiplier from a factory that ignores it: with constant closes the Bollinger bandwidth is 0 for every multiplier. Use a non-flat candle series and assert the pinned result so the factory wiring is actually covered.

    const bbw = await run(['bollingerbandswidth', '2', '--last'], candles);
    const bbwWithMultiplier = await run(['bollingerbandswidth', '2', '3', '--last'], candles);

    expect(macd, 'constant closes keep every MACD line at zero').toEqual([
      JSON.stringify({value: {histogram: 0, macd: 0, signal: 0}}),
    ]);
    expect(bbw, 'zero deviation collapses the bandwidth to zero').toEqual([JSON.stringify({value: 0})]);
    expect(bbwWithMultiplier).toEqual([JSON.stringify({value: 0})]);

packages/trading-signals/src/cli/runCli.ts:383

The generic construction path does not validate missing required arguments at runtime. For example, sma creates SMA with an undefined interval (TypeScript parameter requirements are erased), then never stabilizes and --last reports needs undefined inputs instead of rejecting the invocation. Add runtime constructor-argument validation or indicator metadata for required parameters.

  const indicator = factory
    ? factory(numbers)
    : new IndicatorClass(...(config === undefined ? numbers : [...numbers, config]));

packages/trading-signals/src/cli/runCli.ts:167

The named-value parser only accepts numbers, so the generic CLI cannot pass the boolean monotonic=false supported by HigherLowTrailConfig; it also accepts non-finite numeric values in this branch. Parse supported typed literals (and reject non-finite numbers), or explicitly reject/document config fields the CLI cannot represent.

      const value = Number(param.slice(separator + 1));

      if (Number.isNaN(value)) {
        throw new Error(`Invalid indicator parameter "${param}". The value has to be a number.`);

Files reviewed: 126/126 changed files
Comments generated: 5
Review effort level: Lite

+  : Input extends number
+    ? typeof IndicatorInputShape.VALUE | typeof IndicatorInputShape.VOLUME


+   * The candle fields this indicator consumes, as a runtime value. The type ties the
+   * declaration to the `Input` generic, so a mismatching shape fails to compile.
+   */
+  abstract readonly inputShape: InputShapeOf<Input>;


+  if (isRecord(nested)) {
+    nested[inner] = value;


+  const indicator = factory
+    ? factory(numbers)
+    : new IndicatorClass(...(config === undefined ? numbers : [...numbers, config]));


    test: {
      coverage: {
+        // The bin entry only wires stdin/stdout to runCli(), which carries the tested logic.
+        exclude: ['src/cli/cli.ts'],


bennycode added 2 commits August 22, 2026 15:04

bennycode requested a lite review from Copilot August 22, 2026 13:05

github-actions Bot added the type: feature ✨ label Aug 22, 2026

Copilot started reviewing on behalf of bennycode August 22, 2026 13:06 View session

Copilot AI reviewed Aug 22, 2026

View reviewed changes

bennycode requested a lite review from Copilot August 23, 2026 08:38

Copilot started reviewing on behalf of bennycode August 23, 2026 08:38 View session

Copilot AI reviewed Aug 23, 2026

View reviewed changes

Navigation Menu

Uh oh!

feat(trading-signals): runtime input shapes and a generic indicator CLI - #1319

feat(trading-signals): runtime input shapes and a generic indicator CLI#1319
bennycode wants to merge 3 commits into
mainfrom
trading-signals-cli

bennycode commented Aug 22, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

bennycode commented Aug 23, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		const csvFlagIndex = process.argv.indexOf('--csv');
		const csvPath = csvFlagIndex === -1 ? undefined : process.argv[csvFlagIndex + 1];

		// A close in the middle of a constant range pins every stochastic line at 50.
		expect(lines.at(-1)).toBe(JSON.stringify({value: {stochD: 50, stochJ: 50, stochK: 50}}));

		case IndicatorInputShape.VALUE:
		return requireField(row, 'close');

		const constructorArgs = parseConstructorArgs(params);
		const indicator = new IndicatorClass(...constructorArgs);

		: Input extends number
		? typeof IndicatorInputShape.VALUE \| typeof IndicatorInputShape.VOLUME

Uh oh!

Conversation

bennycode commented Aug 22, 2026

What

1. Every indicator declares its input shape at runtime

2. A generic trading-signals CLI

Verification

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Suppressed comments (8)

Uh oh!

bennycode commented Aug 23, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Suppressed comments (8)

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

2. A generic `trading-signals` CLI