Issue #512: offline CSV review foundation

Selection and scope

Live queue: #581 and #582 are CLEAN drafts, 89 passing checks each, one scheduled-only skipped check each, and no review comments or requested changes. Their files do not overlap this slice.

Candidates assessed (10 total): #332 (P0), #495, #496, #298, #297 (P1), #580, #579 (decision defects), #515, #523 (P2), then #512 (P2). All except #512 already have corresponding local/remote branches or worktrees; those are excluded without deletion, reuse, or modification. #512 has no matching local branch, remote head, or registered worktree before this run.

Deliver the first bounded implementation slice, not the full Issue. Keep #512 open with Refs #512 and verify empty closing references. Navigator must retain its truthful unshipped-workflow diagnostic until the workflow is integrated.

Implementation plan

  1. Add failing Python 3.9 tests for a stdlib-only CSV loader and report CLI in portfolio-manager. Require symbol, asset_type=equity, quantity>0, LONG/SHORT, positive price, currency=USD; optional nonnegative cost_basis (per share). Require explicit timezone-aware –as-of. Reject empty/header-only input, unknown/duplicate headers, malformed rows, nonfinite or extreme numbers, duplicate normalized symbols (including lots), unsupported instruments/currencies. Accept fractional shares. Never echo raw row values or paths in diagnostics.
  2. Implement deterministic JSON and Markdown allocation reports. Calculate gross holdings value, signed net holdings value, gross weights, and unrealized P&L only where cost exists (reverse sign for shorts). Clearly exclude cash/equity, leverage, sector/beta, historical returns and recommendations. No broker/network imports, trader-memory writes or automatic orders. Source/as-of and limitations are explicit; filenames and errors exclude account data. Accept an optional account column but discard it before any output, and never record input path.
  3. Add fictional fixture and reference guide with offline invocation, exact schema, output meanings, privacy boundary and remaining integration work. Link it from SKILL.md as a standalone limited review, not core-portfolio-weekly support. Regenerate only this skill’s package and affected generated docs.
  4. Run targeted skill tests, lint/format, package check, all standard metadata/doc/ snapshot checks, pre-commit and diff check. Full suite only if scope warrants.
  5. Obtain independent implementation review (up to 3 rounds); address all findings. Commit and open one draft PR with test evidence, review counts and Refs #512. Check live PR CI and closing references. Do not merge.

Explicit remaining acceptance work

Navigator route, workflow/skillset integration and workflow replay, and API/CSV shared output contracts remain future work. This foundation does not claim full Issue acceptance or a runnable end-to-end portfolio workflow.

Review log

Round 1: reviewer requested explicit missing-cost P&L coverage and safe symbol / arithmetic bounds. Incorporated before implementation: symbol allowlist [A-Z][A-Z0-9.-]{0,23}, decimal input length/scale/magnitude limits, row limit, and high-precision arithmetic. Tests verify exact totals and reject markup/control characters. Missing cost yields null per-position P&L; no aggregate P&L is emitted. Hand-maintained EN/JA docs will be updated manually; broker prerequisites are scoped to the broker path. Round 2 approved with no remaining findings. Plan finalized before tests/implementation.

Implementation review and validation

  • Implementation round 1: reviewer reproduced input overwrite via a hardlinked report target and a path-disclosing symlink-loop exception. Added failing regressions first, then file-identity checks, temporary-file publication and safe handling of resolution errors.
  • Implementation round 2: approved, no remaining findings. Reviewer independently ran all 44 CSV tests under Python 3.9.
  • Final skill suite: 145 passed. Scoped ruff check and format check passed.
  • Packager source check and extracted-package offline CLI smoke passed.
  • Catalog, skill docs, strict index/workflow metadata, skillsets, workflow docs, skillset docs, Navigator snapshot and dependency checks passed.
  • pre-commit run --all-files passed after rerun. The first run overlapped edits and reported modified files; rerun on stable contents passed all hooks.
  • git diff --check passed. The mandatory pre-push full suite ran all 75 rows: 74 passed; repo-scripts failed its Git-index executable-bit test because nested Git tests inherited hook Git variables. That run also altered the isolated index and shared core.bare setting. Restored core.bare=false and only this run’s index to HEAD; root and isolated worktrees confirmed clean.
  • Reran repo-scripts outside the hook with GIT_DIR/GIT_WORK_TREE/GIT_INDEX_FILE unset: 2401 passed. The previously failing test also passed independently. No unrelated tests/code were changed. Retry push skips only pytest-pre-push, using the completed full run plus clean-environment repo-scripts result; remaining hooks run normally. The initial idle SSH connection also closed.
  • Issue #512 remains open: Navigator/workflow/replay and shared API contracts are explicitly outside this completed foundation. PR uses Refs #512.