Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions openspec/changes/fix-jessx-deal-timestamps/.openspec.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
schema: spec-driven
created: 2026-08-07
54 changes: 54 additions & 0 deletions openspec/changes/fix-jessx-deal-timestamps/design.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
## Context

- See `proposal.md` for motivation and `specs/jessx-time-handling/spec.md` for the behavioral requirements.
- JESSX sends deal/order timestamps as *elapsed milliseconds within the current period* (`ExperimentManager.getTimeInPeriod()` = `now - periodBeginning`). EclipseTrader currently feeds them straight into `new Date(...)`, producing 1970 dates.
- Two consumption points in `org.eclipsetrader.jessx`:
1. `BrokerConnector` — streaming `Deal` → `Trade` + OHLC history (this is what the closed PR #18 patched inline with a sub-2000 heuristic).
2. `JessxTradeHistory` — persisted holding `PURCHASE_DATE` (still broken, unchanged by PR #18).

## Goals / Non-Goals

**Goals:**
- Absolute, current-era dates for JESSX deals/orders at both consumption points.
- One shared conversion so streaming charts and persisted holdings can never diverge.
- Supersede PR #18's inline workaround.

**Non-Goals:**
- Changing what the JESSX server sends (the server is correct to send elapsed time; it is the client's interpretation that is wrong).
- Reconstructing the exact simulated wall-clock time from period start (live deals arrive in real time; arrival time is accurate enough).
- Migrating already-persisted 1970 holdings from old repositories.

## Decisions

### 1. Single shared converter with a magnitude-based interpretation

Add one helper (e.g., `JessxTime.toAbsoluteDate(long timestamp)` in a new internal utility class) used by both `BrokerConnector` and `JessxTradeHistory`:

- **Below threshold** (elapsed-in-period value, bounded by period duration, e.g. `< 86,400,000 ms` — one day): interpret as elapsed → return arrival time (`new Date(System.currentTimeMillis())`).
- **At or above threshold** (a plausible epoch millis): return `new Date(timestamp)` unchanged.

The threshold is the maximum plausible period duration. Unlike PR #18's "before year 2000" check, this correctly preserves genuine epoch values while catching all realistic elapsed values (which are at most a few hours, i.e. millions of ms, never billions).

- **Rationale**: deals are received live, so `arrival time ≈ periodStart + elapsed` within network latency. Tracking period start on the client would add state and message plumbing for no observable gain.
- **Alternative considered**: track `periodBeginning` on the client and compute `periodStart + elapsed`. Rejected — requires new state and event handling across the feed, and its output equals arrival time in practice.

### 2. Apply at both consumption points

- `BrokerConnector` Deal handling: replace the inline PR #18 heuristic with `JessxTime.toAbsoluteDate(...)`.
- `JessxTradeHistory`: route `finalDeal.getTimestamp()` through the same converter before `IPropertyConstants.PURCHASE_DATE` is set.

### 3. Keep OHLC bar-merging behavior as-is

The existing merge condition (`last.getDate().equals(tradeData.getTime())`) is left untouched. Consecutive deals get distinct arrival timestamps (ms precision), so each becomes its own bar — this matches current behavior and satisfies the spec's "current-era date" requirement.

## Risks / Trade-offs

- **[Risk] Threshold heuristic misclassifies an extreme value** → an elapsed period longer than one day would be misread as epoch. Mitigation: threshold is a single named constant, documented; simulation period durations in JESSX are far smaller. Can be tuned without API change.
- **[Risk] Arrival time instead of true sim time on persisted holdings** → purchase dates are "when the platform received the deal", not the simulated timestamp. Accepted for a live simulation; consistent between chart and holdings (which is what the spec requires).
- **[Risk] Pre-existing 1970 holdings remain in repositories** → out of scope; only new deals are corrected. Flagged in the migration plan so it is not mistaken for a regression.

## Migration Plan

- No data migration. Existing broken holdings stay as-is (documented limitation); newly saved deals get correct dates.
- Rollback: revert the change — the converter is additive and localized to the two call sites.
- This change is independent of `modernize-chart-rendering` and of the extracted PR #18 bug fixes.
28 changes: 28 additions & 0 deletions openspec/changes/fix-jessx-deal-timestamps/proposal.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
## Why

The JESSX trading game sends deal/order timestamps as elapsed milliseconds within the current period (`ExperimentManager.getTimeInPeriod()` = `now - periodBeginning`), not epoch milliseconds. EclipseTrader misreads them as epoch millis and feeds them into `new Date(...)`, so streaming trades and persisted holdings show 1970 dates on charts. The closed PR #18 patched only the streaming path in `BrokerConnector` with a "before year 2000 → use now" heuristic; the persistence path (`JessxTradeHistory`) still writes 1970 purchase dates.

## What Changes

- Introduce a single, shared interpretation of JESSX deal/order timestamps so elapsed-in-period values are converted to absolute dates instead of being misread as epoch millis.
- Apply the conversion consistently at both consumption points:
- `BrokerConnector` streaming trade feed (charts/OHLC history).
- `JessxTradeHistory` persisted holdings (portfolio purchase dates).
- Absorb and replace the PR #18 workaround; the heuristic moves into the shared converter (or is superseded by a cleaner derivation) rather than living inline in `BrokerConnector`.

## Capabilities

### New Capabilities

- `jessx-time-handling`: how JESSX deal/order timestamps are interpreted and converted to absolute times, applied consistently across the streaming feed and persisted trade history.

### Modified Capabilities

None — this repo has no existing specs yet; this capability is new.

## Impact

- **Code**: `org.eclipsetrader.jessx` — `BrokerConnector` (streaming `Deal` handling) and `JessxTradeHistory` (holding persistence). A shared timestamp converter utility.
- **Behavior**: charts no longer show 1970 timestamps; portfolio holdings get correct purchase dates. No change for non-JESSX data feeds.
- **Dependencies**: none new.
- **Related work**: independent of the chart modernization change; this stays out of the merged PR #18 bug-fix extraction.
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
## Purpose

Defines how JESSX deal and order timestamps are interpreted and converted to absolute dates, consistently across streaming charts and persisted trade history.

## ADDED Requirements

### Requirement: Absolute deal timestamps

JESSX deal and order timestamps that represent elapsed time within the current period SHALL be converted to absolute dates when consumed by the platform, so trades and OHLC bars carry real-world dates.

#### Scenario: Live deal with elapsed timestamp

- **WHEN** a JESSX Deal message carries an elapsed-in-period timestamp
- **THEN** the resulting trade and OHLC bar carry an absolute date near the time the deal was received rather than the epoch (1970)

#### Scenario: Already-absolute timestamp

- **WHEN** a JESSX message carries a value that is already a valid absolute epoch millis
- **THEN** the value is used as-is without conversion

### Requirement: Consistent interpretation across consumption paths

The same timestamp interpretation SHALL apply to the streaming trade feed and to persisted trade history.

#### Scenario: Persisted holding purchase date

- **WHEN** a JESSX deal is saved to the repository as a holding
- **THEN** the purchase date is an absolute date consistent with the same deal's streaming chart time

#### Scenario: Chart and holdings agree

- **WHEN** the same JESSX deal appears both on a chart and in the portfolio holdings
- **THEN** both show the same absolute timestamp

### Requirement: No epoch dates from elapsed timestamps

The platform SHALL NOT display or persist dates derived from JESSX elapsed-in-period timestamps as 1970 epoch dates.

#### Scenario: Chart shows current-era date

- **WHEN** a JESSX deal is shown on a chart
- **THEN** the tooltip and summary bar show a date in the current era, not 1970
12 changes: 12 additions & 0 deletions openspec/changes/fix-jessx-deal-timestamps/tasks.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
## 1. Shared timestamp converter

- [x] 1.1 Add `JessxTime.toAbsoluteDate(long)` in the jessx bundle's internal package: values below the elapsed threshold (one-day constant) resolve to the arrival time, values at or above it are treated as epoch millis
- [x] 1.2 Replace the inline sub-2000 heuristic in `BrokerConnector` Deal handling with the shared converter
- [x] 1.3 Route `JessxTradeHistory` holding `PURCHASE_DATE` through the same converter
- [x] 1.4 Add unit tests for the converter (elapsed value → current-era date, epoch value → unchanged, boundary at the threshold) wired into the Maven reactor so they run in GitHub Actions CI

## 2. Verification

- [x] 2.1 Run `mvn package` on the branch (GitHub Actions `maven.yml` on JDK 21) and confirm `org.eclipsetrader.jessx` compiles and converter tests pass
- [ ] 2.2 Live-check in the running product (Codespaces virtual display or a local machine): start a JESSX simulation and confirm chart tooltips/summary show current-era timestamps, not 1970
- [ ] 2.3 Confirm a traded deal persists with a current-era purchase date in the portfolio holdings, and that chart and holdings agree (spec `jessx-time-handling`)
2 changes: 2 additions & 0 deletions openspec/changes/modernize-chart-rendering/.openspec.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
schema: spec-driven
created: 2026-08-07
69 changes: 69 additions & 0 deletions openspec/changes/modernize-chart-rendering/design.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
## Context

- See `proposal.md` for motivation. See `specs/chart-rendering/spec.md` and `specs/chart-theme/spec.md` for the behavioral requirements this design satisfies.
- The chart renders by drawing onto an offscreen `Image` (`ChartCanvas.onPaint`, `DateScaleCanvas`, vertical scale canvas), then blits it to the canvas. The image is recreated only on resize, and `Image.getBounds()` returns *logical* size, so a zoom change alone never triggers recreation — an image created at one zoom stays stale when the device zoom changes (mixed-DPI monitors, OS scale changes).
- Chart colors are inline `new RGB(...)` literals scattered across the charts package, `MainChartFactory`, and `MainPropertiesPage`.
- Geometry is partially cached already (`valid`/`pointArray` pooling in `CandleStickChart`, etc.), but `setDataBounds` re-filters the full series O(n) and there is no downsampling, so zoomed-out large histories draw every point.
- Target platform is **Eclipse 2024-03 / SWT 4.31, Java 21** (see `org.eclipsetrader.releng/eclipsetrader.target`). On SWT 4.31, `new Image(device, w, h)` already creates a device-zoom-aware backing image: width/height are logical points auto-scaled to native pixels in the constructor, and `getBounds()` reports logical size. Per-monitor zoom is `Monitor.getZoom()`; the zoom captured at image creation is `DPIUtil.getDeviceZoom()`. `GC.setAntialias()`/`GC.setTextAntialias()` are available. (`Display.getZoom()` and `Image.setScaleFactor()` are **not** in 4.31 — they landed in SWT 3.104 / 2024-09, one release after the target.)

## Goals / Non-Goals

**Goals:**
- Crisp chart output on any display zoom (HiDPI) with minimal change to chart-object drawing code.
- One source of truth for chart colors, with user-saved color preferences still winning.
- Bounded rendering work per frame regardless of history size.

**Non-Goals:**
- Dark mode UI / theme switching in the preferences UI (the theme layer enables it later; selecting themes is out of scope).
- Rewriting the `IChartObject` model or the axis classes.
- Overhauling gridline/`Calendar`-based tick computation (`ChartCanvas.paintBackground`) — tracked as a follow-up, not in this change.
- Changing chart templates or persisted layouts.

## Decisions

### 1. HiDPI via zoom-aware offscreen images

Create offscreen images at the chart's logical `clientArea` size with `new Image(display, w, h)`. SWT 4.31's image/GC pipeline is already device-zoom-aware: the constructor auto-scales the backing store to native pixels, `getBounds()` reports logical size, and GC drawing auto-scales logical coordinates. All existing drawing code (which uses logical coordinates) keeps working unchanged and already lands at native resolution.

The remaining defect is lifecycle, not size: canvases recreate the image only on resize, and because `getBounds()` returns logical size the resize check never fires on a zoom change. Fix with a single helper, `ChartUtils.createBackingImage(Canvas, Rectangle)`, that (a) creates the logical-size image and (b) records the zoom at creation — from the canvas's monitor via `Monitor.getZoom()`, falling back to `display.getDPI().x / 72` if unavailable — and lets the canvas detect on the next paint that the zoom differs and recreate the image. Used by the main canvas, the date scale canvas, and the vertical scale canvas.

- **Alternative considered**: create the image at `clientArea * zoom/100` pixels and apply `image.setScaleFactor(zoom, zoom)`. Rejected: `Image.setScaleFactor` does not exist in SWT 4.31 (added in 3.104/2024-09), and the 4.31 `Image(Device, w, h)` constructor already auto-scales — multiplying by zoom again would double-scale at zoom > 100%.
- **Alternative considered**: `GC.setTransform(new Transform(...scale))` per paint. Rejected: more invasive, risks double-scaling on every draw call, and doesn't help the blit step.
- **Alternative considered**: keep resize-only recreation (status quo). Rejected — that is the stale-zoom defect this change fixes.
- Antialiasing (shapes + text) is enabled in the `Graphics` constructor, as proposed in the superseded PR.

### 2. Theme layer: `ChartTheme` value object + default registry

Introduce an immutable `ChartTheme` (RGB: line, positive, negative, outline, grid, background) and a `ChartThemes` holder exposing the default light theme (the Material palette proposed in PR #18: blue line `33,150,243`, teal `38,166,154`, red `239,83,80`, outline `64,64,64`).

- Chart classes (`CandleStickChart`, `BarChart`, `HistogramBarChart`, `HistogramAreaChart`, `OHLCLineChart`, `LineChart`) replace their inline `new RGB(...)` field initializers/constructor fallbacks with lookups from `ChartThemes.getDefault()`. Their RGB constructor parameters stay — the `null`-fallback semantics are unchanged.
- `MainChartFactory` field defaults and `MainPropertiesPage` color-selector defaults read from the same theme.
- User preferences keep their existing path: `setParameters` → non-null RGB → `createObject(...)` passes them in, overriding theme defaults (spec `chart-theme`).
- **Alternative considered**: full dependency injection / OSGi service for themes. Rejected as over-engineering for this codebase; a plain value object + static default is enough to centralize the palette and later add a second theme.
- **Rationale for value-object over a live theme object**: RGB is a plain value in SWT; no listeners/observability are needed for this change.

### 3. Downsampling via per-renderer aggregators, cached per visible range

Extend the existing `setDataBounds`/`valid` pattern with an aggregation step and a cache keyed by `(firstDate, lastDate, pixelWidth)`:

- When the visible points exceed `clientArea.width`, aggregate before building geometry.
- **OHLC renderers** (candles, bars, OHLC line): min/max binning per pixel column — preserve each column's high/low (and first-open/last-close for candles) so zoomed-out bars stay honest.
- **Scalar renderers** (line, area, histogram): min/max per column as well (keeps spikes visible); LTTB is an acceptable alternative for line aesthetics — implementation detail, spec only requires one aggregate per column.
- Implemented as shared helpers (e.g., `OHLCDownsampler.downsample(IOHLC[], int width)` and a scalar counterpart) so all chart classes benefit without duplicating logic.
- Cache lives with the chart object alongside `pointArray`; `invalidate()` clears it. Work per frame becomes O(pixels) to draw plus O(n) only when data/bounds actually change.
- **Alternative considered**: always-on LTTB for everything. Rejected — min/max binning is O(n) and correct for OHLC extremes; LTTB is a polish option for lines only.
- **Trade-off**: zoomed-out tooltips report the aggregated candle/bar rather than one specific tick. Accepted (standard practice); per-tick detail returns on zoom-in.

## Risks / Trade-offs

- **[Risk] A scale-canvas or image path misses the zoom-change recreation** → inconsistent crispness across mixed-DPI moves. Mitigation: single `createBackingImage` helper used by all three canvases; verify visually at 200% during implementation.
- **[Risk] Downsampling alters zoomed-out visuals and tooltips** → expected behavior change. Mitigation: spec already defines one-aggregate-per-column; confirm the min/max OHLC result with a large history in review.
- **[Risk] Theme defaults shift user-visible colors** for charts that had no explicit colors → intended by design (spec `chart-theme`), but only affects charts with no saved colors.
- **[Risk] Per-frame allocation regressions** → geometry and aggregation caches reuse objects; keep pooling behavior from the current `pointArray` pattern.
- **[Risk] SWT version drift** (zoom APIs) → guard with fallback to `getDPI()`; `Monitor.getZoom()` exists in the fixed 2024-03 target.

## Migration Plan

- Pure UI change; no persisted data or API breakage. Chart-object constructors and `MainChartFactory` keep their signatures.
- Rollback: revert the change — existing layouts and preferences are untouched.
- Land independently of the extracted PR #18 bug fixes (tooltip + resource disposal), which merge first on master.
Loading
Loading