feat: add stq2bars Stooq convenience command

[rustyeddy]

Closes #407

Adds a thin Go trader data stq2bars SYMBOL command and the bin/stq2bars convenience wrapper. The command applies Stooq D1 defaults, discovers configured native ZIP archives, resolves reference listings (SPY, QQQ, and AAPL), requires explicit identity metadata for unknown symbols, and delegates extraction/import/canonicalization to the existing conversion service.

Also adds archive-root configuration, README usage examples, repeat-run coverage protection, and focused/end-to-end tests.

Validation: make check

[Copilot]

Copilot review overview

🟡 Changes recommended

Unresolved provider/configuration, archive freshness, and rebuild correctness findings must be addressed before approval.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 4

Open (4)

• stq2bars overrides configured provider with stooq · New
• Archive root requirement regresses commands without HOME · New
• Sentinel bounds prevent coverage completion · New
• Rebuild flag does not force canonical data rebuild · New

What changed in this PR

Adds trader data stq2bars SYMBOL and bin/stq2bars for converting Stooq D1 archives into canonical bars.

Changes:

• Adds Stooq defaults, identity resolution, archive discovery, extraction, and conversion.
• Adds archive-root configuration and defaults.
• Adds documentation, focused tests, and integration coverage.

File	Reviewed changes and final findings
README.md	Documents stq2bars usage and configuration.
cmd/​trader/​data/​stq2bars.go	Implements the convenience command. Findings: inaccurate source bounds, ineffective rebuild/overwrite behavior, archive freshness blind spot, and substring archive matching (moderate; 1–3 votes).
cmd/​trader/​data/​stq2bars_test.go	Adds archive-discovery tests.
cmd/​trader/​data/​service.go	Adds archive-root configuration. Findings: missing UsersGuide documentation (nit, 1 vote) and incorrect empty-flag override handling (moderate, 1 vote).
cmd/​trader/​data/​dir.go	Adds archive-root defaults. Finding: may regress commands that only need store/raw roots (moderate, 3 votes).
cmd/​trader/​data/​convert_integration_test.go	Adds end-to-end conversion coverage.
cmd/​trader/​data/​command.go	Registers the command and Stooq provider default. Finding: overrides configured providers before config loading (moderate, 3 votes).
bin/​stq2bars	Adds the shell convenience wrapper.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[rustyeddy]

Reviewed PR #408 at head 011a9c369d6abe415a86948f8658196a3d568803.

The layering is good: stq2bars is thin and delegates parsing/import/build to Trader's existing services.

I found one correctness blocker plus a related range issue:

1. The currentness check happens before the source ZIP is imported. Coverage only compares canonical data to the existing managed raw data. If the native Stooq ZIP is refreshed with newer rows, coverageIsComplete can still report the old raw/canonical pair as current and skip without importing the new source. That defeats the intended fingerprint/staleness semantics.

   Please import/compare the native source before deciding the run is a no-op. The simplest safe behavior is to let the normal conversion path perform the idempotent import and then rely on Trader's existing fingerprint/build logic.

2. The default range 1900-01-01 -> 2100-01-01 is not really “earliest/latest available.” It also makes the default Coverage check span about 200 years, so missing months make coverageIsComplete unlikely to ever return true.

   Since StooqImportResult already reports FirstDate and LastDate, the clean direction is:

   locate/extract archive
      -> import
      -> get actual FirstDate/LastDate
      -> build that actual source range
   

   Explicit --from / --to can then clip those bounds.

   If necessary, adjust Service.Convert so the convenience command does not have to invent a DatasetRequest range before the import result exists. Do not duplicate Stooq parsing in the wrapper.

Minor: findStooqArchive uses strings.Contains(name, symbol), which can create false matches/ambiguity. A stricter filename-token match or ZIP-member inspection would be safer.

Everything else looks directionally right: Stooq/D1 defaults, explicit known listings, explicit metadata for unknown symbols, configurable roots, immutable source ZIP, and focused tests.

I would not merge #408 until the refreshed-source and actual-range behavior is corrected.

[rustyeddy]

Re-reviewed PR #408 at head af33b919d3e62a6fd5dd1a868cf6d5170ec18fa5.

The major issues from the previous review are now fixed well:

• native Stooq data is imported before canonical build/currentness decisions, so refreshed source ZIPs participate in raw fingerprint/staleness handling;
• omitted ranges are now derived from the import's actual FirstDate / LastDate;
• explicit ranges are clipped to the available source range;
• --rebuild now propagates a real force-build request through Service/Manager rather than merely changing the output label;
• archive-root defaulting is command-specific instead of regressing unrelated data commands;
• archive filename matching is tokenized rather than a loose substring match;
• docs and integration coverage were expanded.

I see one remaining blocker:

The early Stooq provider override still defeats configured provider values

cmd/trader/data/command.go still does this before buildDataContext / config loading:

if cmd.Name() == "stq2bars" && flags.provider == "" && !cmd.Flags().Changed("provider") {
    flags.provider = "stooq"
}

That turns the Stooq default into an explicit flag-layer override before environment/config values are resolved.

So if the operator has:

TRADER_PROVIDER=alpaca

and runs:

trader data stq2bars SPY ...

the pre-run code sets flags.provider = "stooq", and buildDatasetConfig sees a non-empty provider value / CLI-layer override rather than preserving the configured Alpaca value for the later validation.

The newer logic in buildDataContext:

if cmd.Name() == "stq2bars" && cfg.Provider == "oanda" &&
   os.Getenv("TRADER_PROVIDER") == "" &&
   !cmd.Flags().Changed("provider") {
    cfg.Provider = "stooq"
}

is the right place to apply the convenience default. The earlier mutation in command.go should therefore be removed.

That should also make TestStq2BarsPreservesConfiguredProvider genuinely validate the intended behavior.

After removing the early provider override, I do not see another substantive blocker in the current diff.

Expected state after that fix: merge-ready.