231 lines
12 KiB
Markdown
231 lines
12 KiB
Markdown
# Working on this repo
|
|
|
|
## Read current context first
|
|
|
|
Before planning work, read [`docs/plan.md`](docs/plan.md) for the decisions
|
|
and the reasoning behind them, and
|
|
[`docs/implementation.md`](docs/implementation.md) for the dated record of what
|
|
actually went wrong and how it was resolved — that one is the faster read when
|
|
debugging, because most entries describe something that looked like one bug and
|
|
turned out to be another. Then
|
|
[`docs/NEXT_STEPS.md`](docs/NEXT_STEPS.md) for current recommendations and known
|
|
deferred fixes. Mobile interaction work also has its own detailed plan in
|
|
[`docs/mobile_enhance.md`](docs/mobile_enhance.md). The CDN-to-Vite move is
|
|
[`docs/vite_build.md`](docs/vite_build.md). Light/dark theme constraints are
|
|
[`docs/plan_light_dark_themes.md`](docs/plan_light_dark_themes.md).
|
|
Daily MA alert toggles are [`docs/plan_dma_alerts.md`](docs/plan_dma_alerts.md).
|
|
|
|
## Tests earn their place by catching a real bug
|
|
|
|
When a bug is found, ask whether a unit test could reasonably have caught it. If
|
|
yes, write that test with the fix. If no — a rendering artefact, a browser
|
|
quirk, a data-source oddity — say so and don't add one.
|
|
|
|
The bar is "would this have failed before the fix, and would it fail again if
|
|
someone reintroduced it". Tests that restate the implementation, assert
|
|
constructor defaults, or exercise paths nothing depends on are noise; they make
|
|
the suite slow to run and expensive to change, which is how a suite stops being
|
|
trusted.
|
|
|
|
What has actually paid off here: bar aggregation and bucket boundaries, the
|
|
store's replace-vs-append rules, level and alert arithmetic, parsing real
|
|
market-data payloads (fixtures are trimmed real responses, not invented), and
|
|
the invariants that would otherwise be silent — a comment must never become a
|
|
level, a tick must never overwrite a settled bar, volume must be counted once.
|
|
|
|
Name the test after the failure, not the function: `test_a_tick_cannot_overwrite
|
|
_a_settled_bar` beats `test_put`.
|
|
|
|
## Where things run
|
|
|
|
**The agent works on a remote machine over SSH. The user's browser runs on a
|
|
different machine.** Consequences, all learned the hard way:
|
|
|
|
- You cannot see the user's screen, console, or cursor. Screenshots and pasted
|
|
console output are the only window into it. Browser extensions that drive
|
|
"your" Chrome do not help — they attach to the machine the browser is on.
|
|
- Headless Chromium here renders on server hardware: different screen, window
|
|
size and device pixel ratio from the user's. "Works in my headless run" is not
|
|
evidence that it works for them. When a UI bug will not reproduce, match their
|
|
viewport and `deviceScaleFactor` explicitly before concluding anything.
|
|
- The dev stack is served to them over the network (e.g. `hera.local:8010`),
|
|
which is the same app the headless browser reaches as `http://api:8000`.
|
|
|
|
When a visual bug resists reproduction, prefer putting the numbers **on screen**
|
|
in the app over asking for another console paste — one screenshot then carries
|
|
the whole diagnosis.
|
|
|
|
## Verify UI in a real browser
|
|
|
|
Chart bugs are invisible from the outside — the API, the socket and the
|
|
frontend source can each be correct while the screen is wrong. Drive the
|
|
Playwright container against the dev stack:
|
|
|
|
```
|
|
docker exec -i chart-playwright-1 node - <<'EOF'
|
|
const { chromium } = require('/usr/lib/node_modules/playwright');
|
|
// launch with args:['--lang=en-US'] — see below
|
|
EOF
|
|
```
|
|
|
|
**Always launch Chromium with `args: ['--lang=en-US']`.** The container has no
|
|
usable locale, so Chromium reports `en-US@posix`, `Intl` throws, and the chart
|
|
renders as a blank canvas that looks exactly like a broken app.
|
|
|
|
`window.__chart` is a deliberate debug handle. Querying it separates "the data
|
|
is missing" from "the data is off-screen" — which is how a viewport bug that
|
|
three passing API checks had missed was finally found.
|
|
|
|
## Diagnostic mode
|
|
|
|
Chart geometry bugs live in the browser, which is usually on a different machine
|
|
from whoever is debugging them. Rather than asking for console pastes:
|
|
|
|
```
|
|
open the chart with ?diag=1 # remembered until ?diag=0
|
|
docker compose logs api | grep SNAPDBG
|
|
```
|
|
|
|
With it on, every snap the trendline tool computes is posted to
|
|
`/api/debug/snap` and logged server-side — the cursor's time, price and x, the
|
|
snapped time and price, how many bars were held, the first and last bar, and the
|
|
chart's width. Throttled to about one a second. It reads the client's own
|
|
numbers, which is exactly what "works in my headless run" cannot tell you.
|
|
|
|
Extend it when the next geometry puzzle appears; the endpoint takes whatever
|
|
fields `SnapReport` declares.
|
|
|
|
`?diag=1` also exposes **Capture diagnostic**. The uploaded PNG URL at
|
|
`/api/debug/captures/{id}` is deliberately public: its 72-bit id is the
|
|
handoff from a browser to an agent on a different machine. Capture upload and
|
|
metadata remain authenticated. Inspect only a URL the user explicitly shares,
|
|
then immediately `DELETE /api/debug/captures/{id}`. The server also expires
|
|
captures after 24 hours and caps the directory at 50 files.
|
|
|
|
After the browser's required share picker closes, capture waits five seconds so
|
|
the user can restore a hover tooltip. `Alt+Shift+C` starts the same delayed flow
|
|
without clicking the status-bar button.
|
|
|
|
Diagnostic mode also shows a compact projection readout for visible manual
|
|
trendlines: historical/future canonical price changes, their screen slopes, and
|
|
whether the future canvas point exists. Include it in a capture when a line
|
|
looks kinked at the live edge; it separates bad geometry from a bad renderer.
|
|
|
|
## Future whitespace is a high-risk boundary
|
|
|
|
Drawing bugs repeatedly appear to the right of the last real candle. Treat any
|
|
change involving future slots, projection, drawing movement, or selection as a
|
|
geometry change that needs explicit browser verification.
|
|
|
|
- The displayed time scale, a drawing's source-timeframe bar space, and the
|
|
server session calendar are different coordinate systems. Never substitute
|
|
wall-clock seconds or the displayed grid for canonical source geometry.
|
|
- Daily future slots must skip non-session days. Intraday future slots cross the
|
|
settlement break and weekend today; an endpoint that looks valid before the
|
|
break can become unresolved when real history arrives.
|
|
- Do not clamp a future click to the last real candle. Do not let one null or
|
|
unpriceable future sample disable an otherwise valid object's whole hit target.
|
|
- DOM/SVG overlays must use the chart's actual future coordinates. Extrapolating
|
|
from the last two real candles is wrong when sparse-gap slots were inserted.
|
|
- Audit every drawing type, not just trendlines: Fibonacci hit testing and body
|
|
targets, pinned comments/symbols, keyboard nudges, duplicate, cutoff, handles,
|
|
selection glow, and drag persistence have separate paths.
|
|
- Tests must cover placement, selection, body drag, endpoint drag, duplicate,
|
|
nudge, and cutoff beyond the live edge and across session boundaries. Several
|
|
older future-interaction E2E tests remain quarantined against mutable live
|
|
state; a skipped test is not protection.
|
|
|
|
Relevant history is concentrated near the trendline/future entries in
|
|
`docs/implementation.md`. Before fixing another symptom, measure timestamps,
|
|
logical/x coordinates, canonical prices, and painted pixels in the user's
|
|
viewport; self-consistent chart API numbers have missed real rendering bugs.
|
|
|
|
## Keep the two documents current
|
|
|
|
This is a running system under continual change, not a build being executed, so
|
|
both live documents decay unless updating them is part of finishing the work —
|
|
not a tidy-up afterwards.
|
|
|
|
- **`docs/plan.md`** — when a decision changes, change it here. A plan that
|
|
contradicts the code is worse than no plan, because someone believes it. If
|
|
you find a section describing behaviour that no longer exists, that is a bug
|
|
in the document; fix it in the same commit that revealed it.
|
|
- **`docs/implementation.md`** — append when a fix was not obvious. The bar is
|
|
"would this have saved me an hour": wrong theories that were measured and
|
|
killed, the evidence that settled it, the thing that looked like one bug and
|
|
was another. Not every fix. A log of trivia stops being read, and then the
|
|
useful entries go unread too.
|
|
|
|
Rule of thumb: if you needed a measurement to be sure, write down what it was.
|
|
Git records what changed; these record why it was hard.
|
|
|
|
## Direction of travel
|
|
|
|
Two live planning documents, both written to be refactored toward rather than
|
|
implemented in one go:
|
|
|
|
- `docs/async_refactor.md` — nothing blocks the event loop. P0 and P1 are done;
|
|
`/api/status` reports `loop_lag_ms`, and a rise there is the signal.
|
|
- `docs/multi_user.md` — separate people with their own drawings and alerts,
|
|
authenticated by OIDC. Read it before adding state to `Runtime`: new state is
|
|
either genuinely shared (market data) or belongs to a user, and knowing which
|
|
now is much cheaper than untangling it later.
|
|
- `docs/vite_build.md` — pin and hash the frontend, stay on Coolify, do not
|
|
split components on the way. A production Dockerfile first, then Vite;
|
|
never a root `package.json` while nixpacks is still the builder.
|
|
|
|
Do not build local user accounts. The destination is OIDC, so password storage
|
|
would be written and then deleted.
|
|
|
|
## Running tests
|
|
|
|
```
|
|
docker exec chart-api-1 sh -c "cd /app && python -m pytest -q"
|
|
```
|
|
|
|
pytest + pytest-asyncio, declared in `requirements-dev.txt`. Tests live in
|
|
`tests/`, import from `app.*`, and use `tmp_path` for anything that persists.
|
|
Async paths are driven with `asyncio.run(...)` directly rather than async test
|
|
markers.
|
|
|
|
## Things that will cost you an hour
|
|
|
|
- **Never write scratch `.py` files into the repo root.** It is bind-mounted, so
|
|
`--reload` restarts the app, and startup takes ~82 seconds. Pipe throwaway
|
|
scripts over stdin instead: `docker exec -i chart-api-1 python - <<'EOF'`.
|
|
Screenshots into `artifacts/` are safe; only `.py` triggers the reloader.
|
|
- **Dev and production keep separate drawing stores.** Dev writes
|
|
`data/manual_lines.json`; production has its own Coolify volume. A fix that
|
|
"didn't land" is often the other store.
|
|
- **Rebuild the image after touching `requirements.txt`.** The bind mount makes
|
|
source edits look live while an added dependency is simply absent.
|
|
- **A deploy resets alert cooldowns**, so production may re-alert on whatever
|
|
price is sitting on. There is no durable state yet.
|
|
- Times are epoch seconds, UTC, everywhere. Only the display is localised —
|
|
never shift the stored values.
|
|
- **Do not hardcode how many bars a client gets.** A leftover `1000` on the
|
|
snapshot made 1m look empty past ~1am while the store held 5,000. The
|
|
store cap is `max_bars_per_tf`. The next step is a visible-window fetch,
|
|
not another silent number.
|
|
|
|
## Stay cheap
|
|
|
|
Performance is a product feature, not a later cleanup. The app already
|
|
pays for a live stream, 5k bars, and a canvas. New work must not add
|
|
cost on the hot path unless the screen or an alert has to change.
|
|
|
|
- Nothing extra on the event loop each closed bar or tick. Watch
|
|
`loop_lag_ms`. CPU stays in the threadpool or off the loop — see
|
|
`docs/async_refactor.md`.
|
|
- Do not grow a payload because it is easier than asking what the
|
|
client needs. Caps are named settings, not leftover literals.
|
|
- Crosshair move is for the cursor (OHLC, drawing tooltip). Do not
|
|
rebuild overlays that only depend on the viewport.
|
|
- Overlay redraws go through `scheduleOverlays`. Force when data or
|
|
size changed; let the range-key skip no-ops.
|
|
- `window` pointermove/up attach only while a tool is armed or a drag
|
|
is live. Do not leave them on for the life of the page.
|
|
- Prefer one rAF over N DOM rebuilds. Read `offsetWidth` only if the
|
|
layout actually changed.
|
|
- A feature that needs a per-frame or per-tick loop needs a reason,
|
|
and an off switch.
|