Claude/market data fixes improvements oc05s7 - #164
Open
GBRCenter wants to merge 12 commits into
Open
Conversation
…7320008 Add Claude Code GitHub Workflow
Design doc for the market data subsystem: unified MarketDataSource API,
GBM simulator, and Massive REST client, with implementation-ready snippets.
Grounded in the current code rather than the existing planning docs:
- Records actual test state (73 passing, 91% coverage; stream.py at 33%),
correcting the stale 84% in MARKET_DATA_SUMMARY.md
- Documents the two reproduced defects in massive_client.py: last_trade
has no `timestamp` attribute (correct name is `sip_timestamp`) and the
divisor treats nanoseconds as milliseconds, so the Massive path writes
nothing to the cache at all
- Specifies the four missing pieces: rolling price history in PriceCache,
GET /api/prices/{ticker}/history, the SSE keepalive, and lifespan wiring
with startup reconciliation over watchlist union positions
- Adds an ordered 5-step implementation plan
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VBCDeXTMgiy4SEZsVfgziA
The Massive REST client silently wrote nothing to the cache: it read
last_trade.timestamp (the real attribute is sip_timestamp) and divided
by 1000 instead of 1e9, so every snapshot was skipped by a blanket
except AttributeError. The parse loop is now _apply_snapshots(), tested
against the real TickerSnapshot model instead of MagicMock, which is
what let the original bug ship at 94% coverage.
Also adds the three other backend/app/market/ gaps the design doc
tracked as missing: a bounded rolling price history on PriceCache
(get_history, cleared on remove), the GET /api/prices/{ticker}/history
endpoint that serves it, and an SSE keepalive ping so a Massive-backed
feed's 15s polling gaps don't read as a dead connection.
Lifespan wiring (the design doc's fifth gap) is intentionally left out
-- it depends on the DB/portfolio/watchlist routes that are still to be
built per the root CLAUDE.md.
Co-authored-by: GBRCenter <225887058+GBRCenter@users.noreply.github.com>
Close the four outstanding market data gaps from MARKET_DATA_DESIGN.md
Exhaustive static review of backend/app/market/ and its 96-test suite, requested in issue #5. Tests could not be executed in this environment (no permission to run uv/python3), so the review is based on tracing every test against the source it exercises. Confirms all issues from the archived 2026-02-10 review are fixed except the shared module-level router in stream.py (upgraded to Medium, now demonstrated by the test suite's own route-registration pattern) and two trivial/low items. Co-authored-by: GBRCenter <225887058+GBRCenter@users.noreply.github.com>
The issue-triggered run could not execute tests: claude_args was commented out entirely, so Bash was unavailable and there is no human to approve it in a non-interactive run. Two blockers, not one: - uv is not preinstalled on the GitHub runner image (Ubuntu 24.04 ships Python, Pipx and Miniconda, but no uv), so allowing the tool alone would still fail with "command not found". Adds astral-sh/setup-uv, pinned to v10.0.1 since no moving v10 tag exists, with the cache keyed on backend/. - Adds --allowedTools "Bash(cd:*),Bash(uv:*)" to both workflows. `cd` is needed because the uv project lives in backend/. The flag accumulates rather than overwriting, so the action's own tool list is preserved -- the review workflow keeps its inline-comment tool. Also raises claude.yml to contents: write so fixes can be pushed, not just read. The gate against untrusted triggers is the action's actor check (write access required, bots blocked by default), not repo visibility. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VBCDeXTMgiy4SEZsVfgziA
The class carried @pytest.mark.asyncio but every test in it is
synchronous, so pytest emitted a warning for each of the five:
PytestWarning: ... is marked with '@pytest.mark.asyncio' but it is
not an async function.
They exercise _apply_snapshots directly, which is a plain method. The
sibling class TestMassiveDataSourcePolling keeps its mark -- its tests
really are async.
97 passed, no warnings. ruff clean.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VBCDeXTMgiy4SEZsVfgziA
docs: add market data backend code review
Resolves the 4 open issues in planning/MARKET_DATA_REVIEW.md (2026-09-02): - create_stream_router()/create_history_router() built their route onto a shared module-level router, so calling either factory more than once per process (a normal pytest app-fixture pattern) silently accumulated duplicate routes. Each factory now constructs a fresh APIRouter() per call. - PriceCache.update() used `timestamp or time.time()`, which silently replaces an explicit timestamp=0.0 (a legitimate Unix epoch instant) because 0.0 is falsy. Now checks `is not None`. - MassiveDataSource's poll task died silently on a revoked API key after start() had already succeeded, with no signal beyond an easy-to-miss "Task exception was never retrieved" log at GC time. It now attaches a done-callback that logs loudly and exposes an is_healthy property for a future health endpoint to read. - PriceCache.version read outside the lock; now consistent with the rest of the class. Also actually ran the suite for the first time (the review's own environment couldn't execute the interpreter) — 103 tests pass at 99% coverage, ruff clean. Updated planning/MARKET_DATA_SUMMARY.md to record the fixes and the verified test/coverage numbers. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LS6tvwQ2DMYkeTzmSLr8GP
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.