Skip to content

feat: ground trader in market data and harden unattended runs - #12

Merged
agb1986 merged 1 commit into
mainfrom
feature/pipeline-hardening
Aug 8, 2026
Merged

agb1986 merged 1 commit into
mainfrom
feature/pipeline-hardening

Conversation

@agb1986

@agb1986 agb1986 commented Aug 8, 2026

Copy link
Copy Markdown
Owner

Summary

Follow-up to #11 (merged). A full review of the repo surfaced one correctness gap and a set of robustness holes in the unattended path; this lands all of them, plus the three follow-ups #11 explicitly deferred.

Trader verdicts are now grounded in real market data

The headline fix. run_panel.py has always accepted --context-file, but the daily pipeline never passed one — so the analyst panel worked entirely from training data. The technical analyst was emitting concrete ENTRY/STOP/TARGET price levels and the fundamental analyst was quoting P/E ratios for prices it had never seen.

The pipeline now fetches each symbol's quote and history via check_stock (free Yahoo data, zero marginal cost) before the trader runs, and passes a condensed context file to the panel. New daily_pipeline/context.py renders quote, period range, SMA20/50/200 and the last 10 bars — deliberately kept under ~2.5KB, since the context goes to all five panel roles and the raw history JSON is tens of KB. A failed fetch logs and falls back to an ungrounded run rather than failing the day. Lookback is trader.history_period (default 6mo).

Judge output is schema-enforced

run_judge now passes a JSON schema via structured outputs (output_config.format), so a malformed scorecard is no longer possible. call_role grew an optional output_schema parameter to support it. The fence-stripping parser and the report's "malformed output" branch stay as a safety net.

Unattended-run robustness

  • Failure alerting — a StageError now sends a best-effort "pipeline FAILED" email through the existing SMTP config before exiting non-zero. Previously a failed stage exited quietly into a cron log that's indistinguishable from a slow news day. Alert delivery failure is logged and swallowed so it can never mask the original error.
  • HTTP retries — common.http_client mounts urllib3 retries (3 attempts, backoff, honours Retry-After on 429/5xx) and sets a User-Agent. The 07:30 run hits CoinGecko (aggressive 429s on the free tier), Trading212 and Crypto.com; one transient failure used to mark a stage failed for the day. One change in common hardens every skill.
  • Atomic lock — acquire_lock used exists-then-write, so two simultaneous cron fires could both pass. Now open("x").
  • MCP server — _run had no subprocess timeout, so a hung yfinance call would wedge the tool and its SSE connection indefinitely; now capped at 900s to match the pipeline. Also wired into common.logger, which it was bypassing.

Analysis quality

  • News dedup — six of the ten feeds are Investing.com and the same story is frequently syndicated across several. Each copy counted as a separate ticker mention, so duplicated news was directly driving trader API spend. Articles are now deduplicated by URL and normalised title before ranking.
  • Keyword score — was the fraction of all configured keywords matched, so with 12 keywords a strong single-keyword hit scored 0.083 and --min-keyword 0.5 would have required 6 of 12 matches. Now a capped match count (1 = 0.33, 2 = 0.67, 3+ = 1.0), independent of list size. Worth landing before more scoring data is gathered.

Deferred items from #11, now done

  • Crypto portfolio in the report — the stage was fetching and storing data nothing downstream consumed. Now rendered as its own section (balance + positions sorted by value, with defensive string→float coercion since Crypto.com returns numbers as strings). build_report.py gained --crypto.
  • CI — ci.yml runs pytest + ruff check + ruff format --check on every PR.
  • Image publishing — docker.yml builds and pushes to GHCR on merge to main, so the CasaOS host can docker compose pull instead of building ~1GB of dependencies locally. Uses Buildx with GHA cache; the Dockerfile's legacy-builder compatibility is unaffected since that constraint only applies to the CasaOS host.

Docs

README gained /trader and /daily-pipeline sections (both were missing entirely) and lost a duplicated financial-news scoring-filter block; the verify snippet now imports every workspace package. docs/deployment.md swapped its "Not yet done" section for the shipped alerting and CI behaviour. SKILL.md versions bumped per repo convention (financial-news 0.2.0→0.2.1, daily-pipeline 0.1.2→0.1.3), and daily-pipeline's "Known limitations" now reflects that only crypto trader-gating remains.

Test plan

  • uv run pytest — 439 passed, 97.32% coverage (was 399 / 97.24%; threshold 85%)
  • uv run ruff check . — clean
  • uv run ruff format --check . — 81 files formatted
  • New unit coverage for every change: test_context.py (context builder, SMA edges, compactness assertion), context-file plumbing and ungrounded-fallback in test_run_daily.py, failure-alert paths incl. alert-failure-doesn't-mask-exit-code, lock race, crypto section formatting, dedup cases, keyword-score independence from list size, judge schema enforcement, HTTP retry adapter config
  • Verified model IDs and pricing in pipeline.yaml against current Anthropic rates while reviewing the cost table — claude-sonnet-4-6 $3/$15 and claude-opus-4-8/claude-opus-5 $5/$25 per MTok are all correct and current; no change needed
  • CI green on this PR (ci.yml runs here for the first time — the workflows have never executed, since they only exist on this branch)
  • End-to-end container run to confirm run_<date>/context_<SYMBOL>.md is written and the crypto section renders in the emailed report
  • docker.yml publishes to GHCR on merge; then optionally repoint compose at ghcr.io/agb1986/finance-agent:latest

Notes

Based on 24cac86; merge-base with main is clean, so this is exactly the 27-file diff with no rebase needed. The work was originally committed to feature/daily-pipeline after #11 had already merged, leaving it stranded on a branch with no open PR — this branch carries the identical commit onto a PR that actually targets main. No changes to #11's merged content.

Not included

  • Crypto holdings still aren't gated into select_candidates (stock positions only)
  • Host cron installation on the CasaOS box remains a manual step, per docs/deployment.md
  • Larger feature ideas from the review — verdict performance tracking, weekly digest, portfolio charts in the email, expanded MCP tool surface — are not started

🤖 Generated with Claude Code

- daily-pipeline: fetch check_stock quote/history before each trader run and
  pass a condensed context file to the panel, so verdicts are based on real
  prices instead of training data
- daily-pipeline: render the crypto portfolio in the report, send a best-effort
  failure alert email on stage errors, and make the run lock atomic
- trader: enforce the judge's verdict JSON with structured outputs
- financial-news: dedup syndicated articles by URL/title; keyword score is now
  a capped match count instead of a fraction of all configured keywords
- common: retries with backoff and a User-Agent on the shared HTTP client
- mcp: per-script timeout and shared logger in the MCP server
- ci: GitHub Actions workflows for tests/lint and GHCR image publishing
- docs: README sections for trader and daily-pipeline; deployment guide
  updated for alerting and CI

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@agb1986
agb1986 merged commit 83ffb5b into main Aug 8, 2026
1 check passed
@agb1986
agb1986 deleted the feature/pipeline-hardening branch August 8, 2026 12:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant