From 04eaa758822413a1f741200fda38ad725c646285 Mon Sep 17 00:00:00 2001 From: 0xrsydn Date: Mon, 6 Apr 2026 19:15:19 +0700 Subject: [PATCH] fix: handle missing scrip column in April 2026 KSEI PDF The April 2026 IDX ownership PDF omits the zero-valued scrip column for some holders, causing the parser to leave holdings_scripless/scrip as empty strings. This broke normalize_ksei_row which called parse_id_number on empty input. Two-layer fix: - Parser: pop_numeric_tail handles 2-column (missing scrip) rows by defaulting scrip to "0" - Normalizer: parse_id_number_or_zero backstop treats empty component share fields as 0 while still requiring total_shares Also includes AGENTS.md refresh, formatting cleanup (rustfmt), and version bump to 0.2.2. Co-Authored-By: Claude Opus 4.6 (1M context) --- AGENTS.md | 159 ++++++++++++++++++++------------------ Cargo.lock | 2 +- Cargo.toml | 2 +- src/cli/ownership.rs | 2 +- src/main.rs | 18 ++--- src/ownership/db.rs | 55 ++++++++----- src/ownership/entities.rs | 60 +++++++++++++- src/ownership/graph.rs | 14 ++-- src/ownership/parser.rs | 97 ++++++++++++++++++----- tests/cli.rs | 21 ++++- 10 files changed, 286 insertions(+), 144 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 644db20..e8bb38a 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -1,95 +1,102 @@ # AGENTS.md ## Project -`idx-cli` — CLI tool for Indonesian stock market (IDX) analysis. Built in Rust for humans and AI agents. Single binary, schema-driven, functional architecture. +`idx-cli` is a Rust CLI for Indonesian stock market analysis and ownership workflows. + +The repo currently has two main product areas: +- `stocks`: live market data and analysis +- `ownership`: import/sync once, then query locally from SQLite ## Stack -- **Language:** Rust (stable, via rust-overlay) -- **CLI:** clap 4 (derive) -- **HTTP:** ureq 3 (sync, no async runtime) -- **Output:** comfy-table, owo-colors -- **DB:** rusqlite (bundled SQLite, FTS5) — ownership module -- **Config:** TOML (`~/.config/idx/config.toml`) -- **Cache:** JSON file-based (`~/.cache/idx/`) -- **Testing:** cargo test, assert_cmd, predicates -- **Hooks:** prek (pre-commit: fmt+clippy, pre-push: test) -- **VCS:** jj (Jujutsu, colocated with git) +- Rust stable +- `clap` 4 for CLI +- `ureq` 3 for HTTP +- `comfy-table` and `owo-colors` for output +- `rusqlite` + bundled SQLite/FTS5 for ownership +- TOML config in `~/.config/idx/config.toml` +- file cache in `~/.cache/idx/` -## Structure -``` +## Source Map +```text src/ -├── main.rs # Entry point, command dispatch -├── cli/ # Command handlers (clap derive structs) -│ ├── stocks.rs # stocks quote/history/technical/fundamental/... -│ ├── config.rs # config get/set/init/path -│ ├── cache.rs # cache info/clear -│ └── ownership.rs # ownership import/query commands -├── api/ # Data providers (trait-based abstraction) -│ ├── mod.rs # MarketDataProvider trait + factory functions -│ ├── types.rs # All domain types (Quote, Ohlc, Fundamentals, ...) -│ ├── yahoo/ # Yahoo Finance provider (history/OHLCV) -│ └── msn/ # MSN Finance provider (quotes, fundamentals, ++) -├── analysis/ # Technical & fundamental analysis (pure functions) -├── ownership/ # Ownership intelligence module (SQLite-backed) -│ ├── types.rs # Ownership domain types -│ └── db.rs # Schema, migrations, queries -├── output/ # Rendering (table, json) -├── cache.rs # File-based TTL cache -├── config.rs # Config loading (flags > env > file > defaults) -└── error.rs # IdxError enum (thiserror) +├── main.rs +├── cli/ +│ ├── stocks.rs +│ ├── ownership.rs +│ ├── config.rs +│ └── cache.rs +├── api/ +│ ├── mod.rs +│ ├── types.rs +│ ├── yahoo/ +│ └── msn/ +├── analysis/ +├── ownership/ +│ ├── archive.rs +│ ├── db.rs +│ ├── entities.rs +│ ├── parser.rs +│ ├── remote.rs +│ ├── snapshot.rs +│ └── types.rs +├── output/ +├── cache.rs +├── config.rs +└── error.rs ``` -## Providers -- **MSN** = default provider (quotes, fundamentals, profile, earnings, financials, sentiment, insights, news, screener) -- **Yahoo** = automatic fallback for history/OHLCV (MSN doesn't support IDX history) -- Configurable: `IDX_PROVIDER=msn|yahoo`, `IDX_HISTORY_PROVIDER=auto|yahoo|msn` +## Provider Model +- `MSN` is the primary/default provider for quotes, fundamentals, profile, earnings, financials, sentiment, insights, news, and screener data. +- `Yahoo` is the fallback provider for history/OHLCV because MSN history for IDX is still not supported. +- Current config knobs: + - `IDX_PROVIDER=msn|yahoo` + - `IDX_HISTORY_PROVIDER=auto|yahoo|msn` -## Current Status -- Automated coverage is healthy: `cargo test` currently passes with 122 tests (86 unit, 36 integration). -- Live `stocks` commands are implemented and smoke-tested for: `quote`, `history`, `technical`, `growth`, `valuation`, `risk`, `fundamental`, `compare`, `profile`, `financials`, `earnings`, `sentiment`, `insights`, `news`, `screen`. -- `stocks history --history-provider msn` is intentionally unsupported for IDX; `auto` falls back to Yahoo. -- `ownership import --fetch-bing` is still intentionally unsupported; Bing client groundwork exists but the CLI path is deferred. +## Ownership Model +Ownership is local-first after bootstrap. -## Known Hardening Gaps -- MSN-only commands still bypass the shared cache/offline path in `src/cli/stocks.rs`; `--offline` is not reliable for `profile`/`financials`/`earnings`/`sentiment`/`insights`/`news`/`screen`. -- Core quote flow has a verified `--offline --no-cache` bug: stale cache can still be served. -- Startup/config failures do not yet honor the JSON error contract; runtime failures do. -- `stocks screen --filter` and `--region` still silently coerce invalid values instead of rejecting them. -- Some live MSN output is incomplete or misleading: - - `profile` can return sparse fields. - - `insights.last_updated` is still empty. - - `financials` table output has malformed negative-number formatting in some rows. +Preferred bootstrap/update path: +1. `idx ownership sync` +2. if no snapshot manifest is available: `idx ownership discover` then `idx ownership import --url ` +3. local `--file` imports remain available for manual/fallback use -## Development -```bash -nix develop # enter dev shell -cargo build # build -cargo run -- stocks quote BBCA # run -cargo run -- -o json stocks history BBCA # JSON output -cargo test # test -cargo fmt --check && cargo clippy -- -D warnings # lint -``` +Ownership input paths: +- primary remote source: discoverable IDX `above1` holder-register PDF +- maintained snapshot path: `ownership sync` +- local fallback path: PDF, plus local archive `.zip` / `.txt` + +Important scope note: +- archive ZIP/TXT ingest is a fallback/backstop path, not the primary product ingest surface +- `ownership import --fetch-bing` is still intentionally unsupported + +## Working Principles +1. Keep data access provider-driven where possible; avoid adding new ad hoc fetch paths at the CLI layer. +2. Prefer pure parse/normalize transforms over hidden state. +3. Use fixtures in tests; do not hit live network in automated tests. +4. Preserve the output contract: table to stdout, JSON with `--output json`, errors to stderr. +5. Treat ownership schema/query compatibility as important: `releases`, `ticker`, and `changes` should keep working across ingest paths. ## Verification +Core verification: ```bash -cargo build # must compile -cargo clippy -- -D warnings # zero warnings -cargo test # all tests pass +nix develop +cargo build +cargo clippy -- -D warnings +cargo test ``` -## Principles -1. **Schema-driven** — define types first, build logic around them. Types are the spec. -2. **Functional approach** — pure parse/transform functions (`parse_*`, `normalize_*`), no hidden state. -3. **Data types heavy** — rich enums, newtypes, composite structs. Precision via integer representations (basis points for %, i64 for shares). -4. **Provider abstraction first** — all data access should flow through traits/factories. Note: current MSN-only stock commands still instantiate `MsnProvider` directly in `src/cli/stocks.rs`; removing that split path is an active hardening target. -5. **Sync only** — no tokio/async. CLI tool, ureq is sufficient. -6. **Test with fixtures** — never hit live APIs in tests. Mock provider + fixture JSON. -7. **Output contract** — table to stdout (humans), `--output json` (machines), errors to stderr. -8. **Feature-gated modules** — `ownership` feature for SQLite dep, keeps base binary lean. +Smoke tooling: +```bash +scripts/live-smoke.sh +scripts/live-smoke.sh --mode mock +scripts/live-smoke.sh --mode full +``` -## Docs -Start with the repo-visible docs: -- `FEATURE_SPEC.md` — current hardening backlog and CLI truth-pass expectations -- `TODO.md` — working task list, including latest smoke findings -- `docs/ARCHITECTURE.md` — provider/capability design and error strategy +## Read First +Start with these repo docs before making changes: +- `FEATURE_SPEC.md` — active implementation backlog and remaining core gaps +- `TODO.md` — execution tracker and smoke notes +- `docs/ARCHITECTURE.md` — provider and ownership flow +- `docs/OWNERSHIP_SYNC.md` — snapshot sync contract +- `docs/SMOKE.md` — reusable smoke commands - `docs/CONVENTIONS.md` — repo conventions diff --git a/Cargo.lock b/Cargo.lock index 244ee73..8aa291f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -711,7 +711,7 @@ dependencies = [ [[package]] name = "idx-cli" -version = "0.2.1" +version = "0.2.2" dependencies = [ "assert_cmd", "chrono", diff --git a/Cargo.toml b/Cargo.toml index 50be806..26bdb22 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "idx-cli" -version = "0.2.1" +version = "0.2.2" edition = "2024" rust-version = "1.85" description = "CLI tool for Indonesian stock market (IDX) analysis" diff --git a/src/cli/ownership.rs b/src/cli/ownership.rs index e4c9827..ccd9d30 100644 --- a/src/cli/ownership.rs +++ b/src/cli/ownership.rs @@ -14,11 +14,11 @@ use crate::error::IdxError; use crate::output::OutputFormat; use crate::output::json; use crate::output::table::format_idr; -use crate::runtime; use crate::ownership::types::{ ChangeType, FlowSignal, HolderRow, KseiHolding, OwnershipRelease, OwnershipSource, }; use crate::ownership::{archive, db, entities, graph, parser, remote, search, snapshot}; +use crate::runtime; #[derive(Debug, Args)] #[command( diff --git a/src/main.rs b/src/main.rs index 83d863c..54ba5b6 100644 --- a/src/main.rs +++ b/src/main.rs @@ -78,16 +78,14 @@ fn run() -> Result<(), IdxError> { } Commands::Stocks(stocks) => { let provider = default_provider(config.provider, cli.verbose > 0); - if let Err(err) = - cli::stocks::handle( - stocks, - &config, - &provider, - cli.offline, - cli.no_cache, - cli.verbose > 0, - ) - { + if let Err(err) = cli::stocks::handle( + stocks, + &config, + &provider, + cli.offline, + cli.no_cache, + cli.verbose > 0, + ) { emit_error(&err, &config.output); return Err(err); } diff --git a/src/ownership/db.rs b/src/ownership/db.rs index d6cdb77..a35073d 100644 --- a/src/ownership/db.rs +++ b/src/ownership/db.rs @@ -159,7 +159,10 @@ pub fn upsert_ticker(conn: &Connection, code: &str, name: Option<&str>) -> Resul .map_err(|e| IdxError::DatabaseError(e.to_string())) } -fn insert_ksei_holdings_rows(conn: &Connection, holdings: &[KseiHolding]) -> Result { +fn insert_ksei_holdings_rows( + conn: &Connection, + holdings: &[KseiHolding], +) -> Result { if holdings.is_empty() { return Ok(0); } @@ -591,8 +594,10 @@ pub fn query_cross_holders( min_tickers: usize, limit: usize, ) -> Result, IdxError> { - let mut aggregates: std::collections::BTreeMap, i64)> = - std::collections::BTreeMap::new(); + let mut aggregates: std::collections::BTreeMap< + i64, + (Entity, std::collections::BTreeSet, i64), + > = std::collections::BTreeMap::new(); if let Some(latest_ksei) = latest_ksei_release(conn)? { let mut stmt = conn @@ -1437,7 +1442,10 @@ mod tests { row_count: holdings.len(), imported_at: 1, }; - assert_eq!(write_ksei_release(&conn, &release, &holdings, false).unwrap(), 2); + assert_eq!( + write_ksei_release(&conn, &release, &holdings, false).unwrap(), + 2 + ); let data = query_ticker_holdings(&conn, "BBCA").unwrap(); assert_eq!(data.ticker.code, "BBCA"); @@ -1512,8 +1520,13 @@ mod tests { }; assert_eq!( - write_ksei_release(&conn, &initial_release, std::slice::from_ref(&initial), false) - .unwrap(), + write_ksei_release( + &conn, + &initial_release, + std::slice::from_ref(&initial), + false + ) + .unwrap(), 1 ); @@ -1794,21 +1807,21 @@ mod tests { let holdings = rows .into_iter() .map(|(tid, bps, name)| KseiHolding { - id: 0, - ticker_id: tid, - entity_id: None, - raw_investor_name: name.to_string(), - investor_type: None, - locality: None, - nationality: None, - domicile: None, - holdings_scripless: 1, - holdings_scrip: 0, - total_shares: 1, - percentage_bps: bps, - report_date: NaiveDate::from_ymd_opt(2026, 2, 27).unwrap(), - release_sha256: "current-release".to_string(), - }) + id: 0, + ticker_id: tid, + entity_id: None, + raw_investor_name: name.to_string(), + investor_type: None, + locality: None, + nationality: None, + domicile: None, + holdings_scripless: 1, + holdings_scrip: 0, + total_shares: 1, + percentage_bps: bps, + report_date: NaiveDate::from_ymd_opt(2026, 2, 27).unwrap(), + release_sha256: "current-release".to_string(), + }) .collect::>(); let release = OwnershipRelease { id: 0, diff --git a/src/ownership/entities.rs b/src/ownership/entities.rs index a3fc227..9de47af 100644 --- a/src/ownership/entities.rs +++ b/src/ownership/entities.rs @@ -28,6 +28,14 @@ pub fn parse_id_number(s: &str) -> Result { .map_err(|e| IdxError::ParseError(format!("failed to parse Indonesian number '{s}': {e}"))) } +fn parse_id_number_or_zero(s: &str) -> Result { + if s.trim().is_empty() { + Ok(0) + } else { + parse_id_number(s) + } +} + /// Parse Indonesian locale percentage to basis points (i64). /// "54,94" → 5494, "0,00" → 0, "100,00" → 10000 pub fn parse_id_percentage(s: &str) -> Result { @@ -141,8 +149,8 @@ pub fn normalize_ksei_row(raw: &KseiRawRow) -> Result Result { fn query_all_edges(conn: &Connection) -> Result, IdxError> { let mut out = Vec::new(); - if let Ok(release_sha) = conn - .query_row( - "SELECT sha256 + if let Ok(release_sha) = conn.query_row( + "SELECT sha256 FROM ownership_releases ORDER BY as_of_date DESC, imported_at DESC LIMIT 1", - [], - |row| row.get::<_, String>(0), - ) - { + [], + |row| row.get::<_, String>(0), + ) { let mut stmt = conn .prepare( "SELECT 'entity:' || k.entity_id, @@ -371,7 +369,7 @@ mod tests { use chrono::NaiveDate; use rusqlite::Connection; - use crate::ownership::db::{ensure_schema, write_ksei_release, upsert_ticker}; + use crate::ownership::db::{ensure_schema, upsert_ticker, write_ksei_release}; use crate::ownership::types::{KseiHolding, OwnershipRelease}; use super::query_ownership_graph; diff --git a/src/ownership/parser.rs b/src/ownership/parser.rs index 223aa44..4239247 100644 --- a/src/ownership/parser.rs +++ b/src/ownership/parser.rs @@ -355,23 +355,23 @@ fn parse_row_segments(texts: &[String]) -> KseiRawRow { { row.percentage = remaining.pop().unwrap_or_default(); } - if remaining - .last() - .is_some_and(|segment| is_id_number_like(segment)) - { - row.total_holding_shares = remaining.pop().unwrap_or_default(); - } - if remaining - .last() - .is_some_and(|segment| is_id_number_like(segment)) - { - row.holdings_scrip = remaining.pop().unwrap_or_default(); - } - if remaining - .last() - .is_some_and(|segment| is_id_number_like(segment)) - { - row.holdings_scripless = remaining.pop().unwrap_or_default(); + let numeric_tail = pop_numeric_tail(&mut remaining); + match numeric_tail.as_slice() { + [scripless, scrip, total] => { + row.holdings_scripless = scripless.clone(); + row.holdings_scrip = scrip.clone(); + row.total_holding_shares = total.clone(); + } + // Some PDFs omit the zero-valued scrip column entirely. + [scripless, total] => { + row.holdings_scripless = scripless.clone(); + row.holdings_scrip = "0".to_string(); + row.total_holding_shares = total.clone(); + } + [total] => { + row.total_holding_shares = total.clone(); + } + _ => {} } let mut geo_fields = Vec::new(); @@ -439,6 +439,19 @@ fn is_share_code_like(value: &str) -> bool { .all(|ch| ch.is_ascii_uppercase() || ch.is_ascii_digit()) } +fn pop_numeric_tail(remaining: &mut Vec) -> Vec { + let mut numeric_tail = Vec::new(); + while numeric_tail.len() < 3 + && remaining + .last() + .is_some_and(|segment| is_id_number_like(segment)) + { + numeric_tail.push(remaining.pop().unwrap_or_default()); + } + numeric_tail.reverse(); + numeric_tail +} + fn is_id_number_like(value: &str) -> bool { let trimmed = value.trim(); !trimmed.is_empty() @@ -554,8 +567,11 @@ fn is_percentage_like(s: &str) -> bool { mod tests { use std::path::Path; + use crate::ownership::entities::normalize_ksei_row; + use super::{ - OwnershipPdfSchema, check_mutool, classify_stext_xml, parse_ksei_pdf, parse_stext_xml, + OwnershipPdfSchema, check_mutool, classify_stext_xml, parse_ksei_pdf, parse_row_segments, + parse_stext_xml, }; #[test] @@ -608,6 +624,51 @@ mod tests { assert_eq!(row.percentage, "41,10"); } + #[test] + fn test_parse_row_segments_missing_scrip_defaults_to_zero() { + let texts = vec![ + "27-Apr-2026 AADI".to_string(), + "ADARO ANDALAN INDONESIA Tbk".to_string(), + "ADARO STRATEGIC INVESTMENTS".to_string(), + "CP".to_string(), + "D".to_string(), + "INDONESIA".to_string(), + "3.200.142.830".to_string(), + "3.200.142.830".to_string(), + "66,18".to_string(), + ]; + + let row = parse_row_segments(&texts); + assert_eq!(row.date, "27-Apr-2026"); + assert_eq!(row.share_code, "AADI"); + assert_eq!(row.holdings_scripless, "3.200.142.830"); + assert_eq!(row.holdings_scrip, "0"); + assert_eq!(row.total_holding_shares, "3.200.142.830"); + assert_eq!(row.percentage, "66,18"); + } + + #[test] + fn test_parse_row_segments_missing_scrip_normalizes_without_error() { + let texts = vec![ + "27-Apr-2026 AADI".to_string(), + "ADARO ANDALAN INDONESIA Tbk".to_string(), + "ADARO STRATEGIC INVESTMENTS".to_string(), + "CP".to_string(), + "D".to_string(), + "INDONESIA".to_string(), + "3.200.142.830".to_string(), + "3.200.142.830".to_string(), + "66,18".to_string(), + ]; + + let row = parse_row_segments(&texts); + let draft = normalize_ksei_row(&row).expect("missing scrip column should normalize"); + + assert_eq!(draft.holdings_scripless, 3_200_142_830); + assert_eq!(draft.holdings_scrip, 0); + assert_eq!(draft.total_shares, 3_200_142_830); + } + #[test] fn classify_stext_xml_detects_supported_holder_register_schema() { let xml = include_str!("../../tests/fixtures/ksei_above1_stext_excerpt.xml"); diff --git a/tests/cli.rs b/tests/cli.rs index 3639acb..d8b21ec 100644 --- a/tests/cli.rs +++ b/tests/cli.rs @@ -77,7 +77,8 @@ fn candidate_is_idx_binary(path: &Path) -> bool { return false; }; - output.status.success() && String::from_utf8_lossy(&output.stdout).trim() == env!("CARGO_PKG_VERSION") + output.status.success() + && String::from_utf8_lossy(&output.stdout).trim() == env!("CARGO_PKG_VERSION") } #[cfg(unix)] @@ -1062,7 +1063,9 @@ fn ownership_releases_uses_xdg_data_home_default_db_path() { .args(["ownership", "releases"]) .assert() .success() - .stdout(predicate::str::contains("No ownership releases imported yet.")); + .stdout(predicate::str::contains( + "No ownership releases imported yet.", + )); assert!( data_home.join("idx").join("ownership.db").exists(), @@ -1584,7 +1587,15 @@ fn ownership_import_file_zip_archive_supports_releases_ticker_and_changes() { .stdout(predicate::str::contains("KSEI AGGREGATE FOREIGN MF")); let ticker_output = bin_with_root(&root) - .args(["-o", "json", "ownership", "ticker", "AADI", "--source", "ksei"]) + .args([ + "-o", + "json", + "ownership", + "ticker", + "AADI", + "--source", + "ksei", + ]) .output() .expect("ownership ticker json output"); assert!(ticker_output.status.success()); @@ -1860,7 +1871,9 @@ fn invalid_quote_ttl_env_returns_non_zero() { .args(["version"]) .assert() .failure() - .stderr(predicate::str::contains("invalid IDX_CACHE_QUOTE_TTL value")); + .stderr(predicate::str::contains( + "invalid IDX_CACHE_QUOTE_TTL value", + )); } #[test]