Commit graph

4 commits

Author SHA1 Message Date
hermes
1d3fe5ca88 fix(ownership): correct changes, alias search, graph, and name lookups
From a full-repo Codex audit, each reproduced on the synced ownership DB:
- `changes` paired an investor's accounts across releases instead of
  comparing totals: KREN's KRESNA PRIMA INVEST went 9.44% -> 13.45% (a
  second 4.01% account) but was reported as decreased 944 -> 401 bps.
  Sum a holder's accounts per ticker, and read one release per date (the
  latest imported, as other queries do), so a corrected file supersedes
  the original instead of mixing with it.
- `changes` / get_ownership_changes accepted any dates: `from: ""`
  reported all 7,154 holders as new, unknown dates returned no changes.
  Dates must be YYYY-MM-DD imported releases with `from` before `to`.
- get_investor_holdings / `entity` with an empty name matched every
  investor and returned an arbitrary one ("AMY"); `%` and `_` acted as
  LIKE wildcards. Reject empty names; match wildcards literally.
- Alias search never worked: the FTS table declared an `aliases` column
  its content table lacks (`no such column: T.aliases`), so searches
  fell back to canonical names and a raw KSEI name such as
  "PT ASTRA INTERNATIONAL TBK" found nothing. Search names and aliases
  directly, ranked exact name, exact alias, prefix, then shorter names.
- `graph` enumerated every path (exponential in depth) and its cycle
  check matched substrings, dropping entity:1 when entity:10 was on the
  path. Traverse breadth-first with an exact visited set.
- Name normalization removed leading PT tokens from the front of a Vec,
  quadratic in hostile names; narrow a slice instead.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01V3syn2fN2LNo5pvTSqc6eh
2026-10-17 04:09:51 +00:00
hermes
1e551385d0 fix: close the remaining review findings (cross-process lock, partials)
Second Codex (gpt-6.1-sol, xhigh) pass over 60634d3, each item verified
before fixing:

- new: the sync recheck hashed an existing empty database before schema
  setup, so the first sync falsely reported an external change. Hash
  after setup; a published snapshot already has the schema and keeps
  its bytes.
- sync, import, and entity edits now hold a cross-process lock
  (<db>.lock, std File::lock), so a sync cannot replace the database
  under an import running in another process. Import validates and
  downloads before taking the lock, keeping its error order. MSRV
  1.85 -> 1.89 for File::lock.
- the row-count rebuild branch also never downgrades history.
- cache reads no longer delete corrupt entries (a concurrent writer may
  have just replaced them); the next put overwrites atomically.
- idx-mcp validates the changes limit before querying and runs each
  requested section or analysis once.
- XLSX parsing bounds total cell text, so one large shared string
  referenced from many cells cannot exhaust memory.
- PDF/archive percentages must be 0-100%; HHI uses saturating math.

CLI goldens unchanged. Live A/B vs 0.2.4: 16 passed, discover hits the
IDX 403 on both builds (exit 4 vs 1, intended).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
2026-10-14 22:20:57 +00:00
hermes
60634d3aef fix: address final review (path traversal, sync races, MCP limits)
Findings from a Codex (gpt-6.1-sol, xhigh) review of v0.2.4..HEAD, each
verified in the code before fixing:

- high: ticker symbols reached cache file paths unchecked, so a symbol
  like `../../REPORT` read (and deleted, if unparseable) .json files
  outside the cache, reachable through MCP get_quotes. resolve_symbol now
  accepts only ticker characters, and cache keys are encoded as single
  safe path components.
- high: sync decided before downloading and replaced the database
  without rechecking; an import that landed meanwhile was erased. It now
  rechecks the local state before installing, and idx-mcp runs
  ownership_sync exclusively with ownership reads.
- replaces_sqlite_sha256 never downgrades history: the client requires
  the rebuild to keep at least the local release count, and the
  publisher lists a replaced build only then.
- cache writes are atomic (temp file + rename), so concurrent readers
  never see, and delete, a partial entry.
- idx-mcp caps symbols (20, deduplicated) and limits, and runs at most 4
  tool calls at once.
- each Yahoo client gets its own cookie jar, paired with its crumb.
- ZIP entries (XLSX, KSEI archives) are read with a 256 MiB bound;
  non-ASCII archive dates and huge percentages are parse errors instead
  of panics or overflow.
- agent-smoke --keep deletes every copied agent config, not only auth
  files.

CLI goldens unchanged; the MCP tool golden changes only to state the
new caps.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
2026-10-14 21:58:37 +00:00
hermes
15947de51a refactor: split into idx-core library and idx-cli binary workspace
Phase 2 of docs/DESIGN.md, no behavior change:

- crates/idx-core (library): api, analysis, cache, config file handling
  and provider selection, curl_impersonate, error, ownership, runtime,
  plus the shared test fixtures. clap derives are behind an optional
  `clap` feature so the library does not require clap.
- crates/idx-cli (binary `idx`): cli/, output/, and IdxConfig (output
  format and color resolution). Package and binary names are unchanged.
- config.rs is split: file read/write and validation move to core; a
  CLI test keeps core's OUTPUT_VALUES in sync with OutputSetting.
- ownership::db::open_db/db_path drop their unused IdxConfig argument.
- One workspace version; shared dependency versions; no third-party
  version changes in Cargo.lock. LICENSE is symlinked into each crate.
- flake: workspace version, new source paths, and a per-crate package
  check (idx-cli is built from its tarball against packaged idx-core,
  since idx-core is not on crates.io yet). CI cache key, release path
  filter, and the golden whitespace-hook exclusion follow the new layout.

Parity: all 143 golden outputs match byte for byte without
regeneration; 157 core + 17 CLI unit tests (173 before plus the new
OUTPUT_VALUES sync test), 62 CLI tests pass. Live A/B against main:
16/16 stocks cases identical; ownership discover got IDX 403 on both
(exit 4 vs 1 is the phase 1 exit-code change).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
2026-10-14 09:13:46 +00:00
Renamed from src/ownership/entities.rs (Browse further)