diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 91a4a52..e77d4d9 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -19,13 +19,13 @@ jobs: steps: - name: Checkout - uses: actions/checkout@v4 + uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4 - name: Install Nix - uses: cachix/install-nix-action@v31 + uses: cachix/install-nix-action@448cde739abec1eaf0646249e3d25bb8d9bc7702 # v31 - name: Restore and save Nix store - uses: nix-community/cache-nix-action@v7 + uses: nix-community/cache-nix-action@7df957e333c1e5da7721f60227dbba6d06080569 # v7 with: primary-key: nix-v2-${{ runner.os }}-${{ runner.arch }}-${{ hashFiles('flake.nix', 'flake.lock', 'Cargo.lock', 'Cargo.toml') }}-${{ hashFiles('crates/**', 'scripts/**', 'install.sh', 'README.md', 'LICENSE') }} restore-prefixes-first-match: | @@ -47,7 +47,7 @@ jobs: steps: - name: Checkout - uses: actions/checkout@v4 + uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4 - name: Test install.sh with sh run: scripts/install-sh-test.sh diff --git a/.github/workflows/publish-ownership-snapshot.yml b/.github/workflows/publish-ownership-snapshot.yml index 3204670..179c76e 100644 --- a/.github/workflows/publish-ownership-snapshot.yml +++ b/.github/workflows/publish-ownership-snapshot.yml @@ -26,10 +26,10 @@ jobs: steps: - name: Checkout - uses: actions/checkout@v4 + uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4 - name: Install Nix - uses: cachix/install-nix-action@v30 + uses: cachix/install-nix-action@08dcb3a5e62fa31e2da3d490afc4176ef55ecd72 # v30 - name: Build ownership publisher run: nix build .#ownership-publisher @@ -54,7 +54,7 @@ jobs: - name: Upload workflow artifact copy # A legitimate up-to-date run produces no new local artifacts. - uses: actions/upload-artifact@v4 + uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4 with: name: ownership-snapshot-${{ github.run_id }} path: dist/ownership-snapshot/* diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 135a50f..7dc0bdd 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -20,8 +20,9 @@ on: - "scripts/install.js" - "install.sh" +# Read-only by default; only the publish job writes the release. permissions: - contents: write + contents: read env: RELEASE_TAG: ${{ github.event_name == 'workflow_dispatch' && inputs.tag || github.ref_name }} @@ -49,12 +50,12 @@ jobs: steps: - name: Check out source - uses: actions/checkout@v4 + uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4 with: ref: ${{ github.event_name == 'workflow_dispatch' && inputs.tag || github.ref }} - name: Install Rust toolchain and target - uses: dtolnay/rust-toolchain@stable + uses: dtolnay/rust-toolchain@b725eda525362e7cf34af9a7243cac1e3f01a7ec # stable with: targets: ${{ matrix.target }} @@ -83,7 +84,7 @@ jobs: ./idx-mcp-${{ matrix.asset }} --help >/dev/null - name: Upload target artifacts - uses: actions/upload-artifact@v4 + uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4 with: name: idx-${{ matrix.asset }} path: | @@ -96,9 +97,11 @@ jobs: needs: build if: github.event_name != 'pull_request' runs-on: ubuntu-latest + permissions: + contents: write steps: - name: Check out install script - uses: actions/checkout@v4 + uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4 with: ref: ${{ github.event_name == 'workflow_dispatch' && inputs.tag || github.ref }} sparse-checkout: install.sh @@ -106,7 +109,7 @@ jobs: path: src - name: Download target artifacts - uses: actions/download-artifact@v4 + uses: actions/download-artifact@d3f86a106a0bac45b974a628896c90dbdf5c8093 # v4 with: pattern: idx-* merge-multiple: true @@ -119,7 +122,7 @@ jobs: > SHA256SUMS - name: Attach binaries to the version tag - uses: softprops/action-gh-release@v2 + uses: softprops/action-gh-release@3bb12739c298aeb8a4eeaf626c5b8d85266b0e65 # v2 with: tag_name: ${{ env.RELEASE_TAG }} files: | diff --git a/Cargo.lock b/Cargo.lock index c3be9fc..306781f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -107,15 +107,6 @@ dependencies = [ "windows-sys 0.61.2", ] -[[package]] -name = "arbitrary" -version = "1.4.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c3d036a3c4ab069c7b410a2ce876bd74808d2d0888a82667669f8e783a898bf1" -dependencies = [ - "derive_arbitrary", -] - [[package]] name = "arrayvec" version = "0.7.8" @@ -203,6 +194,12 @@ version = "1.0.4" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "9330f8b2ff13f34540b44e946ef35111825727b38d33286ef986142615121801" +[[package]] +name = "cfg_aliases" +version = "0.2.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f079e83a288787bcd14a6aea84cee5c87a67c5a3e660c30f557a3d24761b3527" + [[package]] name = "chrono" version = "0.4.44" @@ -350,12 +347,6 @@ dependencies = [ "cfg-if", ] -[[package]] -name = "crossbeam-utils" -version = "0.8.21" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d0a5c400df2834b80a4c3327b3aad3a4c4cd4de0629063962b03235697506a28" - [[package]] name = "crossterm" version = "0.29.0" @@ -453,17 +444,6 @@ dependencies = [ "powerfmt", ] -[[package]] -name = "derive_arbitrary" -version = "1.4.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1e567bd82dcff979e4b03460c307b3cdc9e96fde3d73bed1496d2bc75d9dd62a" -dependencies = [ - "proc-macro2", - "quote", - "syn 2.0.117", -] - [[package]] name = "difflib" version = "0.4.0" @@ -581,6 +561,7 @@ checksum = "843fba2746e448b37e26a819579957415c8cef339bf08564fe8b7ddbd959573c" dependencies = [ "crc32fast", "miniz_oxide", + "zlib-rs", ] [[package]] @@ -901,7 +882,7 @@ dependencies = [ [[package]] name = "idx-cli" -version = "0.3.2" +version = "0.3.3" dependencies = [ "assert_cmd", "chrono", @@ -921,7 +902,7 @@ dependencies = [ [[package]] name = "idx-core" -version = "0.3.2" +version = "0.3.3" dependencies = [ "chrono", "clap", @@ -940,7 +921,7 @@ dependencies = [ [[package]] name = "idx-mcp" -version = "0.3.2" +version = "0.3.3" dependencies = [ "clap", "idx-core", @@ -987,9 +968,9 @@ dependencies = [ [[package]] name = "libc" -version = "0.2.182" +version = "0.2.190" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "6800badb6cb2082ffd7b6a67e6125bb39f18782f793520caee8cb8846be06112" +checksum = "ce5d3ddc6d3fa000eb1536d85e147bfe31aacaba692ed6a876f95cb7c855be78" [[package]] name = "libredox" @@ -1066,6 +1047,18 @@ dependencies = [ "simd-adler32", ] +[[package]] +name = "nix" +version = "0.31.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "cf20d2fde8ff38632c426f1165ed7436270b44f199fc55284c38276f9db47c3d" +dependencies = [ + "bitflags", + "cfg-if", + "cfg_aliases", + "libc", +] + [[package]] name = "nom" version = "7.1.3" @@ -1820,6 +1813,12 @@ dependencies = [ "once_cell", ] +[[package]] +name = "typed-path" +version = "0.12.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8e28f89b80c87b8fb0cf04ab448d5dd0dd0ade2f8891bae878de66a75a28600e" + [[package]] name = "typenum" version = "1.19.0" @@ -2364,21 +2363,25 @@ dependencies = [ [[package]] name = "zip" -version = "2.4.2" +version = "9.0.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "fabe6324e908f85a1c52063ce7aa26b68dcb7eb6dbc83a2d148403c9bc3eba50" +checksum = "a954114b47a54df49f83d2722814e86b23d4bb1b54a1b90fb7d92b41619ddc22" dependencies = [ - "arbitrary", "crc32fast", - "crossbeam-utils", - "displaydoc", "flate2", "indexmap", "memchr", - "thiserror 2.0.18", + "nix", + "typed-path", "zopfli", ] +[[package]] +name = "zlib-rs" +version = "0.6.8" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b268e58e7c693d7c271f93ffc4ba3b380412554231c85bf61ca7af91042a4112" + [[package]] name = "zmij" version = "1.0.21" diff --git a/Cargo.toml b/Cargo.toml index 59b3c94..32dab2d 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -3,7 +3,7 @@ resolver = "3" members = ["crates/idx-core", "crates/idx-cli", "crates/idx-mcp"] [workspace.package] -version = "0.3.2" +version = "0.3.3" edition = "2024" rust-version = "1.89" license = "MIT" @@ -11,7 +11,7 @@ repository = "https://github.com/0xrsydn/idx-cli" homepage = "https://idx.rasyidanaf.com" [workspace.dependencies] -idx-core = { path = "crates/idx-core", version = "=0.3.2" } +idx-core = { path = "crates/idx-core", version = "=0.3.3" } clap = { version = "4", features = ["derive"] } serde = { version = "1", features = ["derive"] } serde_json = { version = "1", features = ["raw_value"] } @@ -20,4 +20,4 @@ chrono = { version = "0.4", features = ["serde"] } ureq = { version = "3", features = ["json", "cookies"] } rusqlite = { version = "0.32", features = ["bundled-full"] } sha2 = "0.10" -zip = { version = "2", default-features = false, features = ["deflate"] } +zip = { version = "9", default-features = false, features = ["deflate"] } diff --git a/README.md b/README.md index 27089da..d00b12e 100644 --- a/README.md +++ b/README.md @@ -13,7 +13,7 @@ curl -fsSL https://raw.githubusercontent.com/0xrsydn/idx-cli/main/install.sh | s Downloads the prebuilt `idx` binary for your platform from GitHub Releases, verifies it against the release `SHA256SUMS`, and installs it into `~/.local/bin` without sudo. Pin a version or change the directory with -`sh -s -- --version v0.3.2 --dir ~/bin`. See [docs/INSTALL.md](docs/INSTALL.md). +`sh -s -- --version v0.3.3 --dir ~/bin`. See [docs/INSTALL.md](docs/INSTALL.md). ### npm / npx diff --git a/crates/idx-cli/src/cli/cache.rs b/crates/idx-cli/src/cli/cache.rs index f542d10..6e13895 100644 --- a/crates/idx-cli/src/cli/cache.rs +++ b/crates/idx-cli/src/cli/cache.rs @@ -34,7 +34,10 @@ pub fn handle(cmd: &CacheCmd, format: &OutputFormat) -> Result<(), IdxError> { "newest": info.newest, })); } - println!("path: {}", info.path.display()); + println!( + "path: {}", + crate::output::text::terminal_safe(&info.path.display().to_string()) + ); println!("files: {}", info.files); println!("size_bytes: {}", info.total_size); println!( diff --git a/crates/idx-cli/src/cli/config.rs b/crates/idx-cli/src/cli/config.rs index cd77da2..95c6c79 100644 --- a/crates/idx-cli/src/cli/config.rs +++ b/crates/idx-cli/src/cli/config.rs @@ -1,10 +1,15 @@ use clap::{Args, Subcommand}; use crate::config::{config_path, ensure_default_config, get_config_value, set_config_value}; +use crate::output::{OutputSetting, json}; use idx_core::error::IdxError; #[derive(Debug, Args)] -#[command(about = "Manage configuration")] +#[command( + about = "Manage configuration", + long_about = "Manage configuration in ~/.config/idx/config.toml (or $XDG_CONFIG_HOME/idx/config.toml).\n\nResults print as plain text, even when piped, so `$(idx config path)` works in scripts. Ask for JSON with `-o json`, `IDX_OUTPUT=json`, or `general.output = \"json\"`.", + after_help = "Keys:\n general.provider, general.history_provider, general.exchange, general.output,\n general.color, cache.quote_ttl, cache.fundamental_ttl, ownership.db_path,\n ownership.snapshot_manifest\n\nExamples:\n idx config init\n idx config set general.output json\n idx config get cache.quote_ttl\n idx config path" +)] pub struct ConfigCmd { #[command(subcommand)] pub command: ConfigSubcommand, @@ -15,31 +20,54 @@ pub enum ConfigSubcommand { #[command(about = "Create default configuration file")] Init, #[command(about = "Get a configuration value by key")] - Get { key: String }, + Get { + /// Key such as general.provider (see `idx config --help`). + key: String, + }, #[command(about = "Set a configuration value by key")] - Set { key: String, value: String }, + Set { + /// Key such as general.provider (see `idx config --help`). + key: String, + value: String, + }, #[command(about = "Print configuration file path")] Path, } -pub fn handle(cmd: &ConfigCmd) -> Result<(), IdxError> { +pub fn handle(cmd: &ConfigCmd, setting: OutputSetting) -> Result<(), IdxError> { + // Plain text, as scripts read `$(idx config path)`, unless JSON is asked + // for (-o json, IDX_OUTPUT, or general.output); auto stays text when piped. + let print = |text: String, document: serde_json::Value| { + if matches!(setting, OutputSetting::Json | OutputSetting::Jsonl) { + json::print_json(&document) + } else { + println!("{}", crate::output::text::terminal_safe(&text)); + Ok(()) + } + }; match &cmd.command { ConfigSubcommand::Init => { - let path = ensure_default_config()?; - println!("{}", path.display()); + let path = ensure_default_config()?.display().to_string(); + print(path.clone(), serde_json::json!({ "path": path })) } ConfigSubcommand::Get { key } => { let value = get_config_value(key)? .ok_or_else(|| IdxError::ConfigError(format!("key not found: {key}")))?; - println!("{value}"); + print( + value.to_string(), + serde_json::json!({ "key": key, "value": value.to_string() }), + ) } ConfigSubcommand::Set { key, value } => { set_config_value(key, value)?; - println!("ok"); + print( + "ok".to_string(), + serde_json::json!({ "key": key, "value": value, "ok": true }), + ) } ConfigSubcommand::Path => { - println!("{}", config_path()?.display()); + let path = config_path()?.display().to_string(); + print(path.clone(), serde_json::json!({ "path": path })) } } - Ok(()) } diff --git a/crates/idx-cli/src/cli/ownership.rs b/crates/idx-cli/src/cli/ownership.rs index d9b4261..125952f 100644 --- a/crates/idx-cli/src/cli/ownership.rs +++ b/crates/idx-cli/src/cli/ownership.rs @@ -1,13 +1,14 @@ use std::path::PathBuf; use clap::{Args, Subcommand}; -use comfy_table::{Cell, ContentArrangement, Table, presets::UTF8_FULL}; +use comfy_table::{ContentArrangement, Table, presets::UTF8_FULL}; use serde::Serialize; use crate::config::IdxConfig; use crate::output::json; use crate::output::style::Paint; use crate::output::table::format_idr; +use crate::output::text::{cell, painted, terminal_safe}; use idx_core::error::IdxError; use idx_core::investor::{ ConcentrationRank, HolderSource, ImportSource, Investor, parse_report_family, @@ -49,8 +50,15 @@ pub enum OwnershipCommand { /// Show all holders for a ticker (KSEI + Bing combined). Ticker(TickerArgs), /// Show all holdings for an entity across tickers. + #[command( + after_help = "Examples:\n idx ownership entity \"ASTRA INTERNATIONAL\"\n idx ownership search astra # list candidates first" + )] Entity(EntityArgs), - /// Search entities by name. + /// Search entities by name or KSEI alias. + #[command( + long_about = "Search entities by name or KSEI alias.\n\nMatches canonical names and the raw names KSEI prints (e.g. `PT ASTRA INTERNATIONAL TBK`), case-insensitively. Ranked: exact name, exact alias, prefix, then substring.", + after_help = "Examples:\n idx ownership search astra\n idx ownership search \"PT ASTRA INTERNATIONAL TBK\" --limit 5" + )] Search(SearchArgs), /// Rank entities by cross-ownership breadth. CrossHolders(CrossHolderArgs), @@ -59,6 +67,10 @@ pub enum OwnershipCommand { /// Show Bing institutional flow for a ticker. Flow(FlowArgs), /// Diff two KSEI releases. + #[command( + long_about = "Diff two KSEI releases: new, exited, increased, and decreased holders.\n\nBoth dates must be imported releases (`idx ownership releases` lists them), with --from before --to. A holder's accounts in a ticker are summed, so an investor with several accounts compares by its total stake.", + after_help = "Examples:\n idx ownership releases\n idx ownership changes --from 2026-08-31 --to 2026-09-30" + )] Changes(ChangesArgs), /// Ownership network graph. Graph(GraphArgs), @@ -113,11 +125,13 @@ pub struct TickerArgs { #[derive(Debug, Args)] pub struct EntityArgs { + /// Entity name or part of one, case-insensitive; the shortest match is shown. pub name: String, } #[derive(Debug, Args)] pub struct SearchArgs { + /// Name, KSEI alias, or part of one. pub query: String, #[arg(long, default_value_t = 20)] pub limit: usize, @@ -146,8 +160,10 @@ pub struct FlowArgs { #[derive(Debug, Args)] pub struct ChangesArgs { + /// Earlier release date, YYYY-MM-DD. #[arg(long)] pub from: String, + /// Later release date, YYYY-MM-DD. #[arg(long)] pub to: String, } @@ -227,7 +243,7 @@ fn handle_sync(investor: &Investor, args: &SyncArgs, config: &IdxConfig) -> Resu result.latest_as_of_date.format("%Y-%m-%d"), result.release_count, result.ticker_count, - result.db_path + terminal_safe(&result.db_path) ); } snapshot::OwnershipSyncAction::Updated => { @@ -236,20 +252,20 @@ fn handle_sync(investor: &Investor, args: &SyncArgs, config: &IdxConfig) -> Resu result.latest_as_of_date.format("%Y-%m-%d"), result.release_count, result.ticker_count, - result.db_path + terminal_safe(&result.db_path) ); } snapshot::OwnershipSyncAction::Refreshed => { println!( "Refreshed ownership snapshot {} in {}.", result.latest_as_of_date.format("%Y-%m-%d"), - result.db_path + terminal_safe(&result.db_path) ); } snapshot::OwnershipSyncAction::NoChange | snapshot::OwnershipSyncAction::SkippedNewer | snapshot::OwnershipSyncAction::SkippedDiverged => { - println!("{}", result.reason); + println!("{}", terminal_safe(&result.reason)); } } @@ -262,7 +278,7 @@ fn handle_discover( config: &IdxConfig, ) -> Result<(), IdxError> { if args.limit == 0 { - return Err(IdxError::ParseError( + return Err(IdxError::InvalidInput( "--limit must be greater than 0".to_string(), )); } @@ -284,17 +300,17 @@ fn handle_discover( for report in reports { table.add_row(vec![ - Cell::new(report.publish_date.split('T').next().unwrap_or("-")), - Cell::new(report.family.label()), - Cell::new(report.status.label()), - Cell::new(if report.is_attachment { + cell(report.publish_date.split('T').next().unwrap_or("-")), + cell(report.family.label()), + cell(report.status.label()), + cell(if report.is_attachment { "attachment" } else { "main" }), - Cell::new(report.original_filename.unwrap_or_else(|| "-".to_string())), - Cell::new(report.title), - Cell::new(report.pdf_url), + cell(report.original_filename.unwrap_or_else(|| "-".to_string())), + cell(report.title), + cell(report.pdf_url), ]); } @@ -317,11 +333,11 @@ fn handle_ticker( println!( "{} {} KSEI={} Bing={}", "Ownership:".bold(), - data.ticker.code.bold(), + terminal_safe(&data.ticker.code).bold(), data.ksei_as_of .map(|d| d.format("%Y-%m-%d").to_string()) .unwrap_or_else(|| "-".to_string()), - data.bing_as_of.unwrap_or_else(|| "-".to_string()) + terminal_safe(&data.bing_as_of.unwrap_or_else(|| "-".to_string())) ); let mut table = Table::new(); @@ -334,21 +350,21 @@ fn handle_ticker( for h in &data.holders { table.add_row(vec![ - Cell::new(h.rank), - Cell::new(match h.source { + cell(h.rank), + cell(match h.source { OwnershipSource::Ksei => "KSEI", OwnershipSource::Bing => "BING", }), - Cell::new(&h.name), - Cell::new(h.investor_type.clone().unwrap_or_else(|| "-".to_string())), - Cell::new(match h.locality { + cell(&h.name), + cell(h.investor_type.clone().unwrap_or_else(|| "-".to_string())), + cell(match h.locality { Some(idx_core::ownership::types::Locality::Local) => "L", Some(idx_core::ownership::types::Locality::Foreign) => "F", None => "-", }), - Cell::new(format_idr(h.shares)), - Cell::new(format_bps(h.percentage_bps)), - Cell::new(format_signal(h.signal)), + cell(format_idr(h.shares)), + cell(format_bps(h.percentage_bps)), + painted(format_signal(h.signal)), ]); } println!("{table}"); @@ -380,7 +396,7 @@ fn handle_entity( println!( "{} {}", "Entity:".bold(), - data.entity.canonical_name.as_str().bold() + terminal_safe(&data.entity.canonical_name).bold() ); let mut table = Table::new(); table @@ -390,14 +406,14 @@ fn handle_entity( for row in &data.holdings { table.add_row(vec![ - Cell::new(&row.ticker.code), - Cell::new(match row.source { + cell(&row.ticker.code), + cell(match row.source { OwnershipSource::Ksei => "KSEI", OwnershipSource::Bing => "BING", }), - Cell::new(format_idr(row.shares)), - Cell::new(format_bps(row.percentage_bps)), - Cell::new(row.report_date.format("%Y-%m-%d").to_string()), + cell(format_idr(row.shares)), + cell(format_bps(row.percentage_bps)), + cell(row.report_date.format("%Y-%m-%d").to_string()), ]); } println!("{table}"); @@ -418,7 +434,10 @@ fn handle_search( } if entities.is_empty() { - println!("No entities found for query: {}", args.query); + println!( + "No entities found for query: {}", + terminal_safe(&args.query) + ); return Ok(()); } @@ -430,14 +449,14 @@ fn handle_search( for entity in &entities { table.add_row(vec![ - Cell::new(&entity.canonical_name), - Cell::new( + cell(&entity.canonical_name), + cell( entity .entity_type .clone() .unwrap_or_else(|| "-".to_string()), ), - Cell::new(entity.country.clone().unwrap_or_else(|| "-".to_string())), + cell(entity.country.clone().unwrap_or_else(|| "-".to_string())), ]); } println!("{table}"); @@ -463,10 +482,10 @@ fn handle_cross_holders( for (i, row) in rows.iter().enumerate() { table.add_row(vec![ - Cell::new(i + 1), - Cell::new(&row.entity.canonical_name), - Cell::new(row.ticker_count), - Cell::new(format_bps(row.total_bps)), + cell(i + 1), + cell(&row.entity.canonical_name), + cell(row.ticker_count), + cell(format_bps(row.total_bps)), ]); } @@ -502,13 +521,13 @@ fn handle_concentration( for (idx, (ticker, m)) in rows.iter().enumerate() { table.add_row(vec![ - Cell::new(idx + 1), - Cell::new(ticker), - Cell::new(format_bps(m.top1_bps)), - Cell::new(format_bps(m.top3_bps)), - Cell::new(m.hhi), - Cell::new(format_bps(m.free_float_bps)), - Cell::new(m.holder_count), + cell(idx + 1), + cell(ticker), + cell(format_bps(m.top1_bps)), + cell(format_bps(m.top3_bps)), + cell(m.hhi), + cell(format_bps(m.free_float_bps)), + cell(m.holder_count), ]); } @@ -525,7 +544,10 @@ fn handle_flow(investor: &Investor, args: &FlowArgs, config: &IdxConfig) -> Resu &Option::::None, ); } - println!("No institutional flow data available for {symbol}."); + println!( + "No institutional flow data available for {}.", + terminal_safe(&symbol) + ); return Ok(()); }; @@ -533,7 +555,12 @@ fn handle_flow(investor: &Investor, args: &FlowArgs, config: &IdxConfig) -> Resu return json::print_json(&flow); } - println!("{} {} ({})", "Flow:".bold(), symbol.bold(), flow.period); + println!( + "{} {} ({})", + "Flow:".bold(), + terminal_safe(&symbol).bold(), + terminal_safe(&flow.period) + ); print_flow_section("TOP BUYERS", &flow.top_buyers); print_flow_section("TOP SELLERS", &flow.top_sellers); print_flow_section("NEW POSITIONS", &flow.new_positions); @@ -556,11 +583,11 @@ fn print_flow_section(title: &str, rows: &[HolderRow]) { for (i, row) in rows.iter().enumerate() { table.add_row(vec![ - Cell::new(i + 1), - Cell::new(&row.name), - Cell::new(format_idr(row.shares)), - Cell::new(format_bps(row.percentage_bps)), - Cell::new(format_signal(row.signal)), + cell(i + 1), + cell(&row.name), + cell(format_idr(row.shares)), + cell(format_bps(row.percentage_bps)), + painted(format_signal(row.signal)), ]); } println!("{table}"); @@ -583,14 +610,20 @@ fn handle_graph(investor: &Investor, args: &GraphArgs, config: &IdxConfig) -> Re match args.format.trim().to_lowercase().as_str() { "table" => { - print!("{}", graph::format_graph_text(&nodes, &edges)); + print!( + "{}", + terminal_safe(&graph::format_graph_text(&nodes, &edges)) + ); Ok(()) } "dot" => { - print!("{}", graph::format_graph_dot(&nodes, &edges)); + print!( + "{}", + terminal_safe(&graph::format_graph_dot(&nodes, &edges)) + ); Ok(()) } - other => Err(IdxError::ParseError(format!( + other => Err(IdxError::InvalidInput(format!( "invalid --format '{other}', expected table|dot" ))), } @@ -624,20 +657,20 @@ fn handle_changes( for row in changes { table.add_row(vec![ - Cell::new(row.ticker_code), - Cell::new(row.entity_name), - Cell::new(format_change_type(row.change_type)), - Cell::new( + cell(row.ticker_code), + cell(row.entity_name), + painted(format_change_type(row.change_type)), + cell( row.old_bps .map(format_bps) .unwrap_or_else(|| "-".to_string()), ), - Cell::new( + cell( row.new_bps .map(format_bps) .unwrap_or_else(|| "-".to_string()), ), - Cell::new(format_signed_bps(row.delta_bps)), + cell(format_signed_bps(row.delta_bps)), ]); } @@ -677,11 +710,11 @@ fn handle_resolve( for row in rows { table.add_row(vec![ - Cell::new(row.raw_name), - Cell::new(row.source), - Cell::new(row.ticker_code), - Cell::new(row.current_entity.unwrap_or_else(|| "-".to_string())), - Cell::new( + cell(row.raw_name), + cell(row.source), + cell(row.ticker_code), + cell(row.current_entity.unwrap_or_else(|| "-".to_string())), + cell( row.confidence .map(|v| format!("{v:.2}")) .unwrap_or_else(|| "-".to_string()), @@ -700,7 +733,11 @@ fn handle_resolve( "entity": entity, })); } - println!("Mapped alias '{alias}' -> '{entity}'."); + println!( + "Mapped alias '{}' -> '{}'.", + terminal_safe(alias), + terminal_safe(entity) + ); Ok(()) } ResolveCommand::Merge { keep, merge } => { @@ -734,13 +771,13 @@ fn handle_import( config: &IdxConfig, ) -> Result<(), IdxError> { if args.file.is_none() && args.url.is_none() && args.fetch_bing.is_none() { - return Err(IdxError::ParseError( + return Err(IdxError::InvalidInput( "provide one of: --file, --url, or --fetch-bing".to_string(), )); } if args.file.is_some() && args.url.is_some() { - return Err(IdxError::ParseError( + return Err(IdxError::InvalidInput( "--file and --url are mutually exclusive".to_string(), )); } @@ -802,11 +839,11 @@ fn handle_releases(investor: &Investor, config: &IdxConfig) -> Result<(), IdxErr for r in releases { table.add_row(vec![ - Cell::new(r.as_of_date.format("%Y-%m-%d").to_string()), - Cell::new(r.row_count), - Cell::new(r.sha256), - Cell::new(r.source_url.unwrap_or_else(|| "-".to_string())), - Cell::new(r.imported_at), + cell(r.as_of_date.format("%Y-%m-%d").to_string()), + cell(r.row_count), + cell(r.sha256), + cell(r.source_url.unwrap_or_else(|| "-".to_string())), + cell(r.imported_at), ]); } diff --git a/crates/idx-cli/src/config.rs b/crates/idx-cli/src/config.rs index ba85b4c..a0ad8f3 100644 --- a/crates/idx-cli/src/config.rs +++ b/crates/idx-cli/src/config.rs @@ -14,6 +14,8 @@ use idx_core::error::IdxError; pub struct IdxConfig { pub core: CoreConfig, pub output: OutputFormat, + /// The setting `output` came from, before auto was resolved. + pub output_setting: OutputSetting, pub no_color: bool, } @@ -32,7 +34,7 @@ impl IdxConfig { pub fn load_with_cli(cli: &Cli) -> Result { let mut core = CoreConfig::load()?; core.verbose = cli.verbose > 0; - let mut output = OutputSetting::Auto.resolve(stdout_is_terminal()); + let mut setting = OutputSetting::Auto; let mut no_color = false; let path = config_path()?; @@ -41,8 +43,8 @@ impl IdxConfig { let parsed: FileConfig = toml::from_str(&raw).map_err(|e| IdxError::ConfigError(e.to_string()))?; if let Some(general) = parsed.general { - if let Some(setting) = general.output { - output = setting.resolve(stdout_is_terminal()); + if let Some(file_setting) = general.output { + setting = file_setting; } if let Some(color) = general.color { no_color = !color; @@ -51,25 +53,25 @@ impl IdxConfig { } if let Ok(value) = std::env::var("IDX_OUTPUT") { - let setting = OutputSetting::parse(&value).ok_or_else(|| { + setting = OutputSetting::parse(&value).ok_or_else(|| { IdxError::ConfigError(format!( "invalid IDX_OUTPUT value: '{value}', expected 'auto', 'table', 'json', or 'jsonl'" )) })?; - output = setting.resolve(stdout_is_terminal()); } if let Ok(value) = std::env::var("IDX_NO_COLOR") { no_color = value == "1" || value.eq_ignore_ascii_case("true"); } - if let Some(setting) = cli.output { - output = setting.resolve(stdout_is_terminal()); + if let Some(cli_setting) = cli.output { + setting = cli_setting; } no_color = no_color || cli.no_color || !stdout_is_terminal(); Ok(Self { core, - output, + output: setting.resolve(stdout_is_terminal()), + output_setting: setting, no_color, }) } diff --git a/crates/idx-cli/src/main.rs b/crates/idx-cli/src/main.rs index 91f75a1..f5a7dec 100644 --- a/crates/idx-cli/src/main.rs +++ b/crates/idx-cli/src/main.rs @@ -20,7 +20,10 @@ fn main() { } fn run() -> Result<(), IdxError> { - let cli = Cli::parse(); + let cli = match Cli::try_parse() { + Ok(cli) => cli, + Err(err) => return Err(argument_error(err)), + }; runtime::set_quiet(cli.quiet); let config = match IdxConfig::load_with_cli(&cli) { Ok(config) => config, @@ -30,6 +33,7 @@ fn run() -> Result<(), IdxError> { } }; output::json::configure(config.output, config::stdout_is_terminal()); + runtime::set_json(config.output.is_json()); output::style::set_color(!config.no_color); match &cli.command { @@ -58,7 +62,7 @@ fn run() -> Result<(), IdxError> { } } Commands::Config(cfg) => { - if let Err(err) = cli::config::handle(cfg) { + if let Err(err) = cli::config::handle(cfg, config.output_setting) { emit_error(&err, &config.output); return Err(err); } @@ -82,6 +86,85 @@ fn run() -> Result<(), IdxError> { } /// Output format for errors raised before the configuration has loaded. +/// Argument errors honor the output contract: in JSON mode they are one JSON +/// error on stderr like any other failure (clap prints plain text). Help and +/// `--version` print as usual. +fn argument_error(err: clap::Error) -> IdxError { + use clap::error::ErrorKind; + // A missing subcommand prints help but is an error (exit 2), so in JSON + // mode it is a JSON error too. + if matches!( + err.kind(), + ErrorKind::DisplayHelp | ErrorKind::DisplayVersion + ) || !requested_output_from_args().is_json() + { + err.exit(); + } + let rendered = err.render().to_string(); + let message = rendered + .trim() + .strip_prefix("error: ") + .unwrap_or(rendered.trim()) + .to_string(); + let error = IdxError::InvalidInput(message); + emit_error(&error, &requested_output_from_args()); + error +} + +/// The output format the raw arguments ask for, when they did not parse: +/// `-o`/`--output` as clap reads it (clustered short flags, the last one +/// at any subcommand level), then `IDX_OUTPUT`, then auto. +fn requested_output_from_args() -> OutputFormat { + // args_os: an argument that is not UTF-8 (itself the error being + // reported) must not panic here. + let setting = output_flag(std::env::args_os().collect()) + .or_else(|| { + std::env::var("IDX_OUTPUT") + .ok() + .and_then(|value| OutputSetting::parse(&value)) + }) + .unwrap_or_default(); + setting.resolve(config::stdout_is_terminal()) +} + +/// clap stops at an unknown argument, so an `-o` after it would be missed; +/// drop each unknown argument and parse again. Each pass is read with +/// ignore_errors, which keeps the matches made before any other error; the +/// last pass that read an `-o` wins (a later pass can reach `--help`, which +/// returns no matches at all). +fn output_flag(mut args: Vec) -> Option { + use clap::CommandFactory; + use clap::error::{ContextKind, ContextValue, ErrorKind}; + let read = |args: &[std::ffi::OsString]| { + Cli::command() + .ignore_errors(true) + .try_get_matches_from(args) + .ok() + .and_then(|matches| matches.get_one::("output").copied()) + }; + let mut found = read(&args); + for _ in 0..args.len() { + let Err(err) = Cli::command().try_get_matches_from(&args) else { + break; + }; + if err.kind() != ErrorKind::UnknownArgument { + break; + } + let Some(ContextValue::String(bad)) = err.get(ContextKind::InvalidArg) else { + break; + }; + let Some(index) = args.iter().skip(1).position(|arg| { + let arg = arg.to_string_lossy(); + arg == bad.as_str() || arg.starts_with(&format!("{bad}=")) + }) else { + break; + }; + args.remove(index + 1); + found = read(&args).or(found); + } + found +} + fn bootstrap_output_format(cli: &Cli) -> OutputFormat { let setting = cli .output diff --git a/crates/idx-cli/src/output/mod.rs b/crates/idx-cli/src/output/mod.rs index 2f1b8e4..f9057b2 100644 --- a/crates/idx-cli/src/output/mod.rs +++ b/crates/idx-cli/src/output/mod.rs @@ -1,6 +1,7 @@ pub mod json; pub mod style; pub mod table; +pub mod text; use std::io::IsTerminal; @@ -210,7 +211,8 @@ pub fn render_screener( pub fn emit_error(err: &IdxError, format: &OutputFormat) { if !format.is_json() { - eprintln!("Error: {err}"); + // Errors can carry provider, KSEI, or manifest text. + eprintln!("Error: {}", text::terminal_safe(&err.to_string())); return; } let payload = serde_json::json!({ diff --git a/crates/idx-cli/src/output/table.rs b/crates/idx-cli/src/output/table.rs index fb5df0c..05f0907 100644 --- a/crates/idx-cli/src/output/table.rs +++ b/crates/idx-cli/src/output/table.rs @@ -1,4 +1,5 @@ use crate::output::style::Paint; +use crate::output::text::{cell, painted, terminal_safe, truncate_chars}; use comfy_table::{Cell, Color, ContentArrangement, Table, presets::UTF8_FULL}; use idx_core::analysis::fundamental::{ @@ -56,33 +57,33 @@ pub fn print_quotes(quotes: &[Quote], no_color: bool) -> Result<(), IdxError> { for q in quotes { let pct_cell = match q.change_pct { - None => Cell::new("-"), - Some(pct) if no_color => Cell::new(format!("{pct:+.2}%")), - Some(pct) if pct >= 0.0 => Cell::new(format!("{pct:+.2}%")).fg(Color::Green), - Some(pct) => Cell::new(format!("{pct:+.2}%")).fg(Color::Red), + None => cell("-"), + Some(pct) if no_color => cell(format!("{pct:+.2}%")), + Some(pct) if pct >= 0.0 => cell(format!("{pct:+.2}%")).fg(Color::Green), + Some(pct) => cell(format!("{pct:+.2}%")).fg(Color::Red), }; table.add_row(vec![ - Cell::new(&q.symbol), - Cell::new(format_idr(q.price)), - Cell::new( + cell(&q.symbol), + cell(format_idr(q.price)), + cell( q.change .map(|c| format!("{c:+}")) .unwrap_or_else(|| "-".to_string()), ), pct_cell, - Cell::new(q.volume.map(format_u64).unwrap_or_else(|| "-".to_string())), - Cell::new( + cell(q.volume.map(format_u64).unwrap_or_else(|| "-".to_string())), + cell( q.market_cap .map(format_u64) .unwrap_or_else(|| "-".to_string()), ), - Cell::new(format_52w_range_bar(q.week52_position)), + cell(format_52w_range_bar(q.week52_position)), ]); } println!("{table}"); for note in stale_quote_notes(quotes, chrono::Utc::now()) { - println!("{note}"); + println!("{}", terminal_safe(¬e)); } Ok(()) } @@ -106,7 +107,10 @@ fn stale_quote_notes(quotes: &[Quote], now: chrono::DateTime) -> Ve } pub fn print_history(symbol: &str, history: &[Ohlc]) -> Result<(), IdxError> { - println!("{}", format!("History for {symbol}").bold()); + println!( + "{}", + format!("History for {}", terminal_safe(symbol)).bold() + ); let mut table = Table::new(); table .load_preset(UTF8_FULL) @@ -114,12 +118,12 @@ pub fn print_history(symbol: &str, history: &[Ohlc]) -> Result<(), IdxError> { .set_header(vec!["DATE", "OPEN", "HIGH", "LOW", "CLOSE", "VOLUME"]); for item in history { table.add_row(vec![ - Cell::new(item.date), - Cell::new(format_idr(item.open)), - Cell::new(format_idr(item.high)), - Cell::new(format_idr(item.low)), - Cell::new(format_idr(item.close)), - Cell::new(format_u64(item.volume)), + cell(item.date), + cell(format_idr(item.open)), + cell(format_idr(item.high)), + cell(format_idr(item.low)), + cell(format_idr(item.close)), + cell(format_u64(item.volume)), ]); } println!("{table}"); @@ -131,7 +135,8 @@ pub fn print_technical(report: &TechnicalReport, no_color: bool) -> Result<(), I "{}", format!( "Technical Analysis for {} ({})", - report.symbol, report.as_of + terminal_safe(&report.symbol), + report.as_of ) .bold() ); @@ -143,54 +148,54 @@ pub fn print_technical(report: &TechnicalReport, no_color: bool) -> Result<(), I .set_header(vec!["METRIC", "VALUE", "SIGNAL"]); table.add_row(vec![ - Cell::new("Current Price"), - Cell::new(format_idr(report.current_price)), - Cell::new("-"), + cell("Current Price"), + cell(format_idr(report.current_price)), + cell("-"), ]); table.add_row(vec![ - Cell::new("SMA 20"), - Cell::new(format_idr_option(report.sma20)), - Cell::new("-"), + cell("SMA 20"), + cell(format_idr_option(report.sma20)), + cell("-"), ]); table.add_row(vec![ - Cell::new("SMA 50"), - Cell::new(format_idr_option(report.sma50)), - Cell::new("-"), + cell("SMA 50"), + cell(format_idr_option(report.sma50)), + cell("-"), ]); table.add_row(vec![ - Cell::new("SMA 200"), - Cell::new(format_idr_option(report.sma200)), - Cell::new("-"), + cell("SMA 200"), + cell(format_idr_option(report.sma200)), + cell("-"), ]); table.add_row(vec![ - Cell::new("RSI (14)"), - Cell::new(format_float(report.rsi14, 2)), - Cell::new(format_signal(report.signals.rsi, no_color, false)), + cell("RSI (14)"), + cell(format_float(report.rsi14, 2)), + painted(format_signal(report.signals.rsi, no_color, false)), ]); table.add_row(vec![ - Cell::new("MACD (12,26,9)"), - Cell::new(format!( + cell("MACD (12,26,9)"), + cell(format!( "{}/{}/{}", format_float(report.macd.line, 2), format_float(report.macd.signal, 2), format_float(report.macd.histogram, 2) )), - Cell::new(format_signal(report.signals.macd, no_color, false)), + painted(format_signal(report.signals.macd, no_color, false)), ]); table.add_row(vec![ - Cell::new("Trend"), - Cell::new(trend_context(report)), - Cell::new(format_signal(report.signals.trend, no_color, false)), + cell("Trend"), + cell(trend_context(report)), + painted(format_signal(report.signals.trend, no_color, false)), ]); table.add_row(vec![ - Cell::new("Volume Ratio (20)"), - Cell::new(format_volume_ratio(report)), - Cell::new("-"), + cell("Volume Ratio (20)"), + cell(format_volume_ratio(report)), + cell("-"), ]); table.add_row(vec![ - Cell::new("Overall Signal"), - Cell::new("-"), - Cell::new(format_signal(report.signals.overall, no_color, true)), + cell("Overall Signal"), + cell("-"), + painted(format_signal(report.signals.overall, no_color, true)), ]); println!("{table}"); @@ -198,7 +203,10 @@ pub fn print_technical(report: &TechnicalReport, no_color: bool) -> Result<(), I } pub fn print_growth(symbol: &str, report: &GrowthReport, no_color: bool) -> Result<(), IdxError> { - println!("{}", format!("Growth Analysis for {symbol}").bold()); + println!( + "{}", + format!("Growth Analysis for {}", terminal_safe(symbol)).bold() + ); let mut table = Table::new(); table @@ -207,19 +215,19 @@ pub fn print_growth(symbol: &str, report: &GrowthReport, no_color: bool) -> Resu .set_header(vec!["METRIC", "VALUE", "SIGNAL"]); table.add_row(vec![ - Cell::new("Revenue Growth"), - Cell::new(format_pct(report.revenue_growth_pct)), - Cell::new(format_growth_signal(&report.revenue_signal, no_color)), + cell("Revenue Growth"), + cell(format_pct(report.revenue_growth_pct)), + painted(format_growth_signal(&report.revenue_signal, no_color)), ]); table.add_row(vec![ - Cell::new("Earnings Growth"), - Cell::new(format_pct(report.earnings_growth_pct)), - Cell::new(format_growth_signal(&report.earnings_signal, no_color)), + cell("Earnings Growth"), + cell(format_pct(report.earnings_growth_pct)), + painted(format_growth_signal(&report.earnings_signal, no_color)), ]); table.add_row(vec![ - Cell::new("Overall"), - Cell::new("-"), - Cell::new(format_growth_signal(&report.overall_signal, no_color)), + cell("Overall"), + cell("-"), + painted(format_growth_signal(&report.overall_signal, no_color)), ]); println!("{table}"); @@ -231,7 +239,10 @@ pub fn print_valuation( report: &ValuationReport, no_color: bool, ) -> Result<(), IdxError> { - println!("{}", format!("Valuation Analysis for {symbol}").bold()); + println!( + "{}", + format!("Valuation Analysis for {}", terminal_safe(symbol)).bold() + ); let mut table = Table::new(); table @@ -240,39 +251,39 @@ pub fn print_valuation( .set_header(vec!["METRIC", "VALUE", "SIGNAL"]); table.add_row(vec![ - Cell::new("P/E (Trailing)"), - Cell::new(format_opt_f64(report.pe_trailing, 2)), - Cell::new(format_valuation_signal(&report.pe_signal, no_color)), + cell("P/E (Trailing)"), + cell(format_opt_f64(report.pe_trailing, 2)), + painted(format_valuation_signal(&report.pe_signal, no_color)), ]); table.add_row(vec![ - Cell::new("P/E (Forward)"), - Cell::new(format_opt_f64(report.pe_forward, 2)), - Cell::new("-"), + cell("P/E (Forward)"), + cell(format_opt_f64(report.pe_forward, 2)), + cell("-"), ]); table.add_row(vec![ - Cell::new("Price/Book"), - Cell::new(format_opt_f64(report.pb, 2)), - Cell::new(format_valuation_signal(&report.pb_signal, no_color)), + cell("Price/Book"), + cell(format_opt_f64(report.pb, 2)), + painted(format_valuation_signal(&report.pb_signal, no_color)), ]); table.add_row(vec![ - Cell::new("ROE"), - Cell::new(format_pct(report.roe_pct)), - Cell::new(format_valuation_signal(&report.roe_signal, no_color)), + cell("ROE"), + cell(format_pct(report.roe_pct)), + painted(format_valuation_signal(&report.roe_signal, no_color)), ]); table.add_row(vec![ - Cell::new("Net Margin"), - Cell::new(format_pct(report.net_margin_pct)), - Cell::new(format_valuation_signal(&report.margin_signal, no_color)), + cell("Net Margin"), + cell(format_pct(report.net_margin_pct)), + painted(format_valuation_signal(&report.margin_signal, no_color)), ]); table.add_row(vec![ - Cell::new("EV/EBITDA"), - Cell::new(format_opt_f64(report.ev_ebitda, 2)), - Cell::new(format_valuation_signal(&report.ev_ebitda_signal, no_color)), + cell("EV/EBITDA"), + cell(format_opt_f64(report.ev_ebitda, 2)), + painted(format_valuation_signal(&report.ev_ebitda_signal, no_color)), ]); table.add_row(vec![ - Cell::new("Overall"), - Cell::new("-"), - Cell::new(format_valuation_signal(&report.overall_signal, no_color)), + cell("Overall"), + cell("-"), + painted(format_valuation_signal(&report.overall_signal, no_color)), ]); println!("{table}"); @@ -280,7 +291,10 @@ pub fn print_valuation( } pub fn print_risk(symbol: &str, report: &RiskReport, no_color: bool) -> Result<(), IdxError> { - println!("{}", format!("Risk Analysis for {symbol}").bold()); + println!( + "{}", + format!("Risk Analysis for {}", terminal_safe(symbol)).bold() + ); let mut table = Table::new(); table @@ -289,24 +303,24 @@ pub fn print_risk(symbol: &str, report: &RiskReport, no_color: bool) -> Result<( .set_header(vec!["METRIC", "VALUE", "SIGNAL"]); table.add_row(vec![ - Cell::new("Debt/Equity"), - Cell::new(format_opt_f64(report.debt_to_equity, 2)), - Cell::new(format_risk_signal(&report.de_signal, no_color)), + cell("Debt/Equity"), + cell(format_opt_f64(report.debt_to_equity, 2)), + painted(format_risk_signal(&report.de_signal, no_color)), ]); table.add_row(vec![ - Cell::new("Current Ratio"), - Cell::new(format_opt_f64(report.current_ratio, 2)), - Cell::new(format_risk_signal(&report.current_ratio_signal, no_color)), + cell("Current Ratio"), + cell(format_opt_f64(report.current_ratio, 2)), + painted(format_risk_signal(&report.current_ratio_signal, no_color)), ]); table.add_row(vec![ - Cell::new("ROA"), - Cell::new(format_pct(report.roa_pct)), - Cell::new("-"), + cell("ROA"), + cell(format_pct(report.roa_pct)), + cell("-"), ]); table.add_row(vec![ - Cell::new("Overall"), - Cell::new("-"), - Cell::new(format_risk_signal(&report.overall_signal, no_color)), + cell("Overall"), + cell("-"), + painted(format_risk_signal(&report.overall_signal, no_color)), ]); println!("{table}"); @@ -316,7 +330,7 @@ pub fn print_risk(symbol: &str, report: &RiskReport, no_color: bool) -> Result<( pub fn print_fundamental(report: &FundamentalReport, no_color: bool) -> Result<(), IdxError> { println!( "{}", - format!("Fundamental Analysis for {}", report.symbol).bold() + format!("Fundamental Analysis for {}", terminal_safe(&report.symbol)).bold() ); println!(); print_growth(&report.symbol, &report.growth, no_color)?; @@ -341,8 +355,8 @@ pub fn print_compare(reports: &[FundamentalReport], no_color: bool) -> Result<() .load_preset(UTF8_FULL) .set_content_arrangement(ContentArrangement::Dynamic); - let mut header = vec![Cell::new("METRIC")]; - header.extend(reports.iter().map(|report| Cell::new(&report.symbol))); + let mut header = vec![cell("METRIC")]; + header.extend(reports.iter().map(|report| cell(&report.symbol))); table.set_header(header); add_compare_row( @@ -350,7 +364,7 @@ pub fn print_compare(reports: &[FundamentalReport], no_color: bool) -> Result<() "Symbol", reports .iter() - .map(|report| report.symbol.clone()) + .map(|report| terminal_safe(&report.symbol)) .collect::>(), ); add_compare_row( @@ -519,10 +533,11 @@ fn format_earnings_period(period: &str) -> String { return format!("FY{trimmed}"); } + // ASCII only, so the byte slices below fall on character boundaries. if trimmed.len() == 6 + && trimmed.is_ascii() && trimmed.starts_with('Q') - && trimmed[1..2].chars().all(|ch| ch.is_ascii_digit()) - && trimmed[2..].chars().all(|ch| ch.is_ascii_digit()) + && trimmed[1..].bytes().all(|byte| byte.is_ascii_digit()) { return format!("{} {}", &trimmed[..2], &trimmed[2..]); } @@ -630,21 +645,23 @@ fn format_text_signal( positive: &[&str], negative: &[&str], ) -> String { + // The label comes from the analysis report; make it safe before styling. + let label = terminal_safe(signal); if no_color { - return signal.to_string(); + return label; } if positive.contains(&signal) { - signal.green().to_string() + label.green().to_string() } else if negative.contains(&signal) { - signal.red().to_string() + label.red().to_string() } else { - signal.yellow().to_string() + label.yellow().to_string() } } fn add_compare_row(table: &mut Table, label: &str, values: Vec) { - let mut row = vec![Cell::new(label)]; + let mut row = vec![cell(label)]; row.extend(values.into_iter().map(Cell::new)); table.add_row(row); } @@ -664,7 +681,7 @@ pub fn print_profile(profile: &CompanyProfile) -> Result<(), IdxError> { let add_if_present = |t: &mut Table, label: &str, value: &str| { if !value.is_empty() { - t.add_row(vec![Cell::new(label), Cell::new(value)]); + t.add_row(vec![cell(label), cell(value)]); } }; @@ -677,24 +694,17 @@ pub fn print_profile(profile: &CompanyProfile) -> Result<(), IdxError> { add_if_present(&mut table, "City", &profile.city); add_if_present(&mut table, "Phone", &profile.phone); if profile.employees > 0 { - table.add_row(vec![ - Cell::new("Employees"), - Cell::new(profile.employees.to_string()), - ]); + table.add_row(vec![cell("Employees"), cell(profile.employees.to_string())]); } if !profile.description.is_empty() { // Truncate long descriptions for table display - let desc = if profile.description.len() > 200 { - format!("{}...", &profile.description[..200]) - } else { - profile.description.clone() - }; - table.add_row(vec![Cell::new("Description"), Cell::new(desc)]); + let desc = truncate_chars(&profile.description, 200); + table.add_row(vec![cell("Description"), cell(desc)]); } if !profile.officers.is_empty() { table.add_row(vec![ - Cell::new("Executives"), - Cell::new( + cell("Executives"), + cell( profile .officers .iter() @@ -711,9 +721,12 @@ pub fn print_profile(profile: &CompanyProfile) -> Result<(), IdxError> { pub fn print_financials(fin: &FinancialStatements) -> Result<(), IdxError> { let print_section = |label: &str, section: &idx_core::api::types::StatementSection| { - println!("\n── {label} ({}) ──", format_table_date(§ion.end_date)); + println!( + "\n── {label} ({}) ──", + terminal_safe(&format_table_date(§ion.end_date)) + ); let mut t = Table::new(); - let value_header = format!("VALUE ({})", section.currency); + let value_header = format!("VALUE ({})", terminal_safe(§ion.currency)); t.load_preset(UTF8_FULL) .set_content_arrangement(ContentArrangement::Dynamic) .set_header(vec!["LINE ITEM", value_header.as_str()]); @@ -726,7 +739,7 @@ pub fn print_financials(fin: &FinancialStatements) -> Result<(), IdxError> { entries.sort_by(|(left, _), (right, _)| left.cmp(right)); for (label, value) in entries { - t.add_row(vec![Cell::new(label), Cell::new(format_idr(*value as i64))]); + t.add_row(vec![cell(label), cell(format_idr(*value as i64))]); } println!("{t}"); }; @@ -794,7 +807,7 @@ pub fn print_earnings(report: &EarningsReport) -> Result<(), IdxError> { pub fn print_sentiment(data: &SentimentData) -> Result<(), IdxError> { if data.statistics.is_empty() { - println!("No sentiment votes for {}.", data.symbol); + println!("No sentiment votes for {}.", terminal_safe(&data.symbol)); return Ok(()); } let mut table = Table::new(); @@ -803,10 +816,10 @@ pub fn print_sentiment(data: &SentimentData) -> Result<(), IdxError> { .set_header(vec!["RANGE", "BULLISH", "BEARISH", "NEUTRAL"]); for row in &data.statistics { table.add_row(vec![ - Cell::new(&row.time_range), - Cell::new(row.bullish), - Cell::new(row.bearish), - Cell::new(row.neutral), + cell(&row.time_range), + cell(row.bullish), + cell(row.bearish), + cell(row.neutral), ]); } println!("{table}"); @@ -814,20 +827,20 @@ pub fn print_sentiment(data: &SentimentData) -> Result<(), IdxError> { } pub fn print_insights(data: &InsightData) -> Result<(), IdxError> { - println!("{}", data.summary); + println!("{}", terminal_safe(&data.summary)); if !data.last_updated.is_empty() { - println!("Last updated: {}", data.last_updated); + println!("Last updated: {}", terminal_safe(&data.last_updated)); } if !data.highlights.is_empty() { println!("Highlights:"); for h in &data.highlights { - println!("- {h}"); + println!("- {}", terminal_safe(h)); } } if !data.risks.is_empty() { println!("Risks:"); for r in &data.risks { - println!("- {r}"); + println!("- {}", terminal_safe(r)); } } Ok(()) @@ -840,11 +853,11 @@ pub fn print_news(items: &[NewsItem]) -> Result<(), IdxError> { .set_header(vec!["TITLE", "ABOUT", "PROVIDER", "DATE", "URL"]); for item in items { table.add_row(vec![ - Cell::new(&item.title), - Cell::new(item.relevance.as_str()), - Cell::new(&item.provider), - Cell::new(&item.published_at), - Cell::new(truncate_url(&item.url)), + cell(&item.title), + cell(item.relevance.as_str()), + cell(&item.provider), + cell(&item.published_at), + cell(truncate_url(&item.url)), ]); } println!("{table}"); @@ -853,13 +866,13 @@ pub fn print_news(items: &[NewsItem]) -> Result<(), IdxError> { fn add_earnings_history_row(table: &mut Table, row: &EarningsData) { table.add_row(vec![ - Cell::new(format_earnings_period(&row.period_type)), - Cell::new(format_float(row.eps_actual, 2)), - Cell::new(format_float(row.eps_forecast, 2)), - Cell::new(format_float(row.eps_surprise, 2)), - Cell::new(format_float(row.eps_surprise_pct, 2)), - Cell::new(format_idr_option_from_f64(row.revenue_actual)), - Cell::new(format_table_date_option( + cell(format_earnings_period(&row.period_type)), + cell(format_float(row.eps_actual, 2)), + cell(format_float(row.eps_forecast, 2)), + cell(format_float(row.eps_surprise, 2)), + cell(format_float(row.eps_surprise_pct, 2)), + cell(format_idr_option_from_f64(row.revenue_actual)), + cell(format_table_date_option( row.earning_release_date.as_deref(), )), ]); @@ -867,21 +880,17 @@ fn add_earnings_history_row(table: &mut Table, row: &EarningsData) { fn add_earnings_forecast_row(table: &mut Table, row: &EarningsData) { table.add_row(vec![ - Cell::new(format_earnings_period(&row.period_type)), - Cell::new(format_float(row.eps_forecast, 2)), - Cell::new(format_idr_option_from_f64(row.revenue_forecast)), - Cell::new(format_table_date_option( + cell(format_earnings_period(&row.period_type)), + cell(format_float(row.eps_forecast, 2)), + cell(format_idr_option_from_f64(row.revenue_forecast)), + cell(format_table_date_option( row.earning_release_date.as_deref(), )), ]); } fn truncate_url(url: &str) -> String { - if url.len() > 72 { - format!("{}...", &url[..72]) - } else { - url.to_string() - } + truncate_chars(url, 72) } #[cfg(test)] diff --git a/crates/idx-cli/src/output/text.rs b/crates/idx-cli/src/output/text.rs new file mode 100644 index 0000000..91ffad4 --- /dev/null +++ b/crates/idx-cli/src/output/text.rs @@ -0,0 +1,48 @@ +//! Terminal-safe table cells; see [`idx_core::text`]. + +use comfy_table::Cell; + +pub use idx_core::text::terminal_safe; + +/// A table cell whose content is made [`terminal_safe`]. +pub fn cell(value: impl ToString) -> Cell { + Cell::new(terminal_safe(&value.to_string())) +} + +/// A cell for text idx produced itself (labels, numbers) that may carry its +/// own color codes, which [`cell`] would strip. Never for provider text. +pub fn painted(styled: String) -> Cell { + Cell::new(styled) +} + +/// At most `max` characters of `text` (never splitting one), with `...` +/// appended when it was cut. +pub fn truncate_chars(text: &str, max: usize) -> String { + match text.char_indices().nth(max) { + Some((end, _)) => format!("{}...", &text[..end]), + None => text.to_string(), + } +} + +#[cfg(test)] +mod tests { + use super::{painted, truncate_chars}; + + #[test] + fn painted_cells_keep_idx_color_codes() { + // comfy-table appends a reset; the color codes themselves survive. + let green = "\u{1b}[32mBullish\u{1b}[39m".to_string(); + assert!(painted(green.clone()).content().starts_with(&green)); + } + + #[test] + fn truncation_never_splits_a_character() { + let text = format!("{}é tail", "a".repeat(199)); + assert_eq!( + truncate_chars(&text, 200), + format!("{}é...", "a".repeat(199)) + ); + assert_eq!(truncate_chars("short", 200), "short"); + assert_eq!(truncate_chars("ééé", 2), "éé..."); + } +} diff --git a/crates/idx-cli/tests/cli.rs b/crates/idx-cli/tests/cli.rs index f99edd5..0fb9812 100644 --- a/crates/idx-cli/tests/cli.rs +++ b/crates/idx-cli/tests/cli.rs @@ -980,6 +980,43 @@ fn config_set_and_get_provider_round_trip() { .stdout(predicate::str::contains("msn")); } +#[test] +fn config_prints_json_only_when_json_is_asked_for() { + // Piped with auto output, scripts read plain text: `$(idx config path)`. + let root = test_env_dir("config-output-format"); + bin_with_root(&root) + .args(["config", "set", "general.provider", "msn"]) + .assert() + .success() + .stdout("ok\n"); + bin_with_root(&root) + .args(["config", "get", "general.provider"]) + .assert() + .success() + .stdout("msn\n"); + + let flag = run_success_json(bin_with_root(&root).args([ + "-o", + "json", + "config", + "get", + "general.provider", + ])); + assert_eq!( + flag, + serde_json::json!({"key": "general.provider", "value": "msn"}) + ); + let env = run_success_json( + bin_with_root(&root) + .env("IDX_OUTPUT", "json") + .args(["config", "path"]), + ); + assert!( + env["path"].as_str().unwrap().ends_with("config.toml"), + "{env}" + ); +} + #[test] fn config_set_and_get_ownership_db_path_round_trip() { let root = test_env_dir("config-ownership-db-path"); @@ -1924,12 +1961,24 @@ fn ownership_sync_installs_snapshot_and_preserves_query_behavior() { } let bytes = fs::read(&target_db).unwrap(); + // A pre-0.3.3 install interrupted between its two renames left only a + // backup. Sync restores it before deciding, so it is kept like any local + // database (no_change, not a reinstall), and survives a failed download. + let backup = sync_root.join(".ownership.db.sync-backup"); + fs::rename(&target_db, &backup).unwrap(); + let result = + run_success_json(bin_with_root(&sync_root).args(["-o", "json", "ownership", "sync"])); + assert_eq!(result["action"], "no_change"); + assert_eq!(fs::read(&target_db).unwrap(), bytes); + assert!(!backup.exists()); + // Both failures happen after a usable local snapshot has been installed. write_snapshot_manifest( &manifest_path, &source_db, Some("cccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccc"), ); + fs::rename(&target_db, &backup).unwrap(); run_error_json( bin_with_root(&sync_root).args(["-o", "json", "ownership", "sync", "--force"]), "PARSEERROR", @@ -2105,6 +2154,61 @@ fn offline_and_no_cache_flags_are_rejected() { ); } +#[test] +fn argument_errors_follow_the_output_format_clap_resolves() { + // The last -o wins at any subcommand level, and -o may sit in a cluster: + // argument errors pick the same format a successful command would. + for args in [ + &["-o", "table", "config", "-o", "json", "get"][..], + &["-qojson", "config", "get"][..], + ] { + run_error_json(test_bin("argument-error-json").args(args), "INVALIDINPUT"); + } + // An -o after an unknown argument still counts, over IDX_OUTPUT too. + run_error_json( + test_bin("argument-error-json") + .env("IDX_OUTPUT", "table") + .args(["config", "get", "--bogus", "-o", "json"]), + "INVALIDINPUT", + ); + // ...and so does one before it when --help or --version follows. + for args in [ + &["-o", "json", "config", "get", "--bogus", "--help"][..], + &["-o", "json", "--bogus", "--version"][..], + ] { + run_error_json( + test_bin("argument-error-json") + .env("IDX_OUTPUT", "table") + .args(args), + "INVALIDINPUT", + ); + } + test_bin("argument-error-table") + .env("IDX_OUTPUT", "json") + .args(["-o", "table", "config", "get", "--bogus", "--help"]) + .assert() + .failure() + .stdout("") + .stderr(predicate::str::starts_with("error: ")); + test_bin("argument-error-table") + .args(["config", "get", "--bogus", "-o", "table"]) + .assert() + .failure() + .stdout("") + .stderr(predicate::str::starts_with("error: ")); + for args in [ + &["-o", "json", "config", "-o", "table", "get"][..], + &["-qotable", "config", "get"][..], + ] { + test_bin("argument-error-table") + .args(args) + .assert() + .failure() + .stdout("") + .stderr(predicate::str::starts_with("error: ")); + } +} + #[test] fn msn_screen_rejects_invalid_filter() { test_bin("msn-screen-invalid-filter") diff --git a/crates/idx-cli/tests/golden/error-usage-missing-symbol.json.golden b/crates/idx-cli/tests/golden/error-usage-missing-symbol.json.golden index e653302..60dd4a1 100644 --- a/crates/idx-cli/tests/golden/error-usage-missing-symbol.json.golden +++ b/crates/idx-cli/tests/golden/error-usage-missing-symbol.json.golden @@ -1,9 +1,4 @@ exit: 2 --- stdout --- stderr -error: the following required arguments were not provided: - ... - -Usage: idx stocks quote ... - -For more information, try '--help'. +{"code":"INVALIDINPUT","error":true,"message":"invalid input: the following required arguments were not provided:\n ...\n\nUsage: idx stocks quote ...\n\nFor more information, try '--help'."} diff --git a/crates/idx-cli/tests/golden/help-config.golden b/crates/idx-cli/tests/golden/help-config.golden index e594fbc..845a21d 100644 --- a/crates/idx-cli/tests/golden/help-config.golden +++ b/crates/idx-cli/tests/golden/help-config.golden @@ -1,6 +1,8 @@ exit: 0 --- stdout -Manage configuration +Manage configuration in ~/.config/idx/config.toml (or $XDG_CONFIG_HOME/idx/config.toml). + +Results print as plain text, even when piped, so `$(idx config path)` works in scripts. Ask for JSON with `-o json`, `IDX_OUTPUT=json`, or `general.output = "json"`. Usage: idx config [OPTIONS] @@ -12,11 +14,37 @@ Commands: help Print this message or the help of the given subcommand(s) Options: - -o, --output Output format [default: auto = table on a terminal, json when piped] [possible values: auto, table, json, jsonl] - --no-color Disable colored output - -q, --quiet Suppress non-essential output - -v, --verbose... Increase verbosity (-v, -vv) - --offline Run without network requests - --no-cache Bypass local cache - -h, --help Print help + -o, --output + Output format [default: auto = table on a terminal, json when piped] + + [possible values: auto, table, json, jsonl] + + --no-color + Disable colored output + + -q, --quiet + Suppress non-essential output + + -v, --verbose... + Increase verbosity (-v, -vv) + + --offline + Run without network requests + + --no-cache + Bypass local cache + + -h, --help + Print help (see a summary with '-h') + +Keys: + general.provider, general.history_provider, general.exchange, general.output, + general.color, cache.quote_ttl, cache.fundamental_ttl, ownership.db_path, + ownership.snapshot_manifest + +Examples: + idx config init + idx config set general.output json + idx config get cache.quote_ttl + idx config path --- stderr diff --git a/crates/idx-cli/tests/golden/help-ownership.golden b/crates/idx-cli/tests/golden/help-ownership.golden index 54501d7..a7e4741 100644 --- a/crates/idx-cli/tests/golden/help-ownership.golden +++ b/crates/idx-cli/tests/golden/help-ownership.golden @@ -15,7 +15,7 @@ Commands: sync Install or refresh a maintained ownership SQLite snapshot ticker Show all holders for a ticker (KSEI + Bing combined) entity Show all holdings for an entity across tickers - search Search entities by name + search Search entities by name or KSEI alias cross-holders Rank entities by cross-ownership breadth concentration Rank tickers by ownership concentration flow Show Bing institutional flow for a ticker diff --git a/crates/idx-core/src/analysis/fundamental.rs b/crates/idx-core/src/analysis/fundamental.rs index aec0e80..557a4cc 100644 --- a/crates/idx-core/src/analysis/fundamental.rs +++ b/crates/idx-core/src/analysis/fundamental.rs @@ -148,12 +148,16 @@ pub fn analyze_risk(fundamentals: &Fundamentals) -> RiskReport { let de_signal = de_signal(fundamentals.debt_to_equity); let current_ratio_signal = cr_signal(fundamentals.current_ratio); - let overall_signal = if de_signal == "no data" && current_ratio_signal == "no data" { + // A known adverse signal decides on its own: negative equity or heavy + // leverage is high risk even when the current ratio is missing. + let overall_signal = if matches!(de_signal, "highly leveraged" | "negative equity") + || current_ratio_signal == "weak" + { + "high risk" + } else if de_signal == "no data" && current_ratio_signal == "no data" { "no data" } else if de_signal == "no data" || current_ratio_signal == "no data" { "incomplete data" - } else if de_signal == "highly leveraged" || current_ratio_signal == "weak" { - "high risk" } else if de_signal == "conservative" && matches!(current_ratio_signal, "strong" | "adequate") { "low risk" } else { @@ -176,9 +180,14 @@ pub fn analyze_fundamental(symbol: &str, fundamentals: &Fundamentals) -> Fundame let valuation = analyze_valuation(fundamentals); let risk = analyze_risk(fundamentals); + // "healthy" needs evidence on all three: missing valuation or risk data + // must not pass for "not expensive" or "not risky". let overall_signal = if growth.overall_signal == "growing" - && valuation.overall_signal != "expensive" - && risk.overall_signal != "high risk" + && matches!( + valuation.overall_signal.as_str(), + "undervalued" | "fairly valued" + ) + && matches!(risk.overall_signal.as_str(), "low risk" | "moderate risk") { "healthy" } else if growth.overall_signal == "shrinking" && risk.overall_signal == "high risk" { @@ -375,6 +384,35 @@ mod tests { } } + #[test] + fn missing_valuation_and_risk_data_is_not_healthy() { + // Growth alone (12% revenue, 22% earnings) used to rate "healthy". + let fundamentals = Fundamentals { + revenue_growth: Some(0.12), + earnings_growth: Some(0.22), + ..Fundamentals::default() + }; + let report = analyze_fundamental("X.JK", &fundamentals); + assert_eq!(report.growth.overall_signal, "growing"); + assert_eq!(report.overall_signal, "mixed"); + } + + #[test] + fn negative_equity_is_high_risk_and_not_healthy() { + let mut fundamentals = sample_fundamentals(); + fundamentals.debt_to_equity = Some(-10.0); + assert_eq!(analyze_risk(&fundamentals).overall_signal, "high risk"); + assert_eq!( + analyze_fundamental("X.JK", &fundamentals).overall_signal, + "mixed" + ); + + // Known heavy leverage decides even with the current ratio missing. + fundamentals.debt_to_equity = Some(250.0); + fundamentals.current_ratio = None; + assert_eq!(analyze_risk(&fundamentals).overall_signal, "high risk"); + } + #[test] fn growth_signal_thresholds_match_python() { assert_eq!(growth_signal(None), "no data"); diff --git a/crates/idx-core/src/api/msn/client.rs b/crates/idx-core/src/api/msn/client.rs index f61ef71..5c1fbaf 100644 --- a/crates/idx-core/src/api/msn/client.rs +++ b/crates/idx-core/src/api/msn/client.rs @@ -30,6 +30,9 @@ impl MsnClient { let agent: ureq::Agent = ureq::Agent::config_builder() .timeout_connect(Some(Duration::from_secs(5))) .timeout_recv_body(Some(Duration::from_secs(10))) + // A server that stalls before sending headers must not hold a + // call (and an MCP call slot) forever. + .timeout_global(Some(Duration::from_secs(30))) .build() .into(); diff --git a/crates/idx-core/src/api/msn/map.rs b/crates/idx-core/src/api/msn/map.rs index b47f1a9..b0f445f 100644 --- a/crates/idx-core/src/api/msn/map.rs +++ b/crates/idx-core/src/api/msn/map.rs @@ -25,11 +25,15 @@ pub(super) fn parse_quote(symbol: &str, quotes: &[MsnQuote]) -> Result Result) -> Option { value.filter(|number| number.is_finite() && *number >= 0.01) } -fn round_price(value: f64) -> i64 { - value.round() as i64 +/// Prices are IDR; anything non-finite, negative, or above this is not a +/// real quote, and treating it as missing keeps arithmetic from overflowing. +const MAX_PRICE: f64 = 1e12; + +fn round_price(value: f64) -> Option { + (value.is_finite() && (0.0..=MAX_PRICE).contains(&value)).then(|| value.round() as i64) +} + +/// A price change can be negative; same bound on its size. +fn round_change(value: f64) -> Option { + (value.is_finite() && value.abs() <= MAX_PRICE).then(|| value.round() as i64) } fn round_u64(value: Option) -> Option { @@ -667,11 +680,11 @@ pub(super) fn parse_screener_results(raw: &RawScreenerResponse) -> Result 0.0)?; - let price = round_price(raw_price); - let prev_close = q.price_previous_close.map(round_price); + let price = round_price(raw_price)?; + let prev_close = q.price_previous_close.and_then(round_price); let change = prev_close .map(|pc| price - pc) - .or_else(|| q.price_change.map(round_price)); + .or_else(|| q.price_change.and_then(round_change)); let ticker = q .symbol .as_deref() @@ -698,8 +711,8 @@ pub(super) fn parse_screener_results(raw: &RawScreenerResponse) -> Result], idx: usize) -> Option { .copied() .flatten() .filter(|value| value.is_finite()) - .map(round_price) + .and_then(round_price) } fn series_volume_at(series: &RawChartSeries, idx: usize) -> u64 { @@ -889,6 +901,30 @@ mod tests { assert_eq!(sentiment.statistics[0].neutral, 3); } + #[test] + fn parse_quote_rejects_implausible_prices_instead_of_overflowing() { + let raw: Vec = + serde_json::from_str(r#"[{"symbol":"BBCA","price":1e20,"pricePreviousClose":-1e20}]"#) + .unwrap(); + assert!(parse_quote("BBCA.JK", &raw).is_err()); + + let raw: Vec = serde_json::from_str( + r#"[{"symbol":"BBCA","price":6075.0,"pricePreviousClose":-1e20}]"#, + ) + .unwrap(); + let quote = parse_quote("BBCA.JK", &raw).unwrap(); + assert_eq!(quote.price, 6075); + assert_eq!(quote.prev_close, None); + } + + #[test] + fn parse_quote_keeps_a_negative_change_without_a_previous_close() { + let raw: Vec = + serde_json::from_str(r#"[{"symbol":"BBCA","price":100.0,"priceChange":-5.0}]"#) + .unwrap(); + assert_eq!(parse_quote("BBCA.JK", &raw).unwrap().change, Some(-5)); + } + #[test] fn parses_news_fixture_items() { let raw: RawNewsFeed = diff --git a/crates/idx-core/src/api/types.rs b/crates/idx-core/src/api/types.rs index 8949e8f..920a238 100644 --- a/crates/idx-core/src/api/types.rs +++ b/crates/idx-core/src/api/types.rs @@ -74,7 +74,7 @@ pub struct Ohlc { } /// Fundamental metrics normalized from Yahoo Finance `/v10/finance/quoteSummary`. -#[derive(Debug, Clone, Serialize, Deserialize)] +#[derive(Debug, Clone, Default, Serialize, Deserialize)] pub struct Fundamentals { pub trailing_pe: Option, pub forward_pe: Option, diff --git a/crates/idx-core/src/api/yahoo/client.rs b/crates/idx-core/src/api/yahoo/client.rs index 6dc3154..2029a84 100644 --- a/crates/idx-core/src/api/yahoo/client.rs +++ b/crates/idx-core/src/api/yahoo/client.rs @@ -30,6 +30,9 @@ impl YahooClient { let agent: ureq::Agent = ureq::Agent::config_builder() .timeout_connect(Some(Duration::from_secs(5))) .timeout_recv_body(Some(Duration::from_secs(10))) + // A server that stalls before sending headers must not hold a + // call (and an MCP call slot) forever. + .timeout_global(Some(Duration::from_secs(30))) .build() .into(); @@ -141,7 +144,14 @@ impl YahooClient { // Step 1: fetch fc.yahoo.com to set A3 cookie (returns 404 but writes cookie jar). // We allow non-zero exit here since 404 still writes the cookie. let _ = Command::new(binary) - .args(["--silent", "--cookie-jar", cookie_jar_str, COOKIE_FETCH_URL]) + .args([ + "--silent", + "--max-time", + "30", + "--cookie-jar", + cookie_jar_str, + COOKIE_FETCH_URL, + ]) .stdout(Stdio::null()) .stderr(Stdio::null()) .output(); diff --git a/crates/idx-core/src/api/yahoo/map.rs b/crates/idx-core/src/api/yahoo/map.rs index 20d8212..75c415f 100644 --- a/crates/idx-core/src/api/yahoo/map.rs +++ b/crates/idx-core/src/api/yahoo/map.rs @@ -20,8 +20,12 @@ pub(super) fn parse_quote(symbol: &str, chart: &ChartResponse) -> Result Result i64 { - value.round() as i64 +/// Prices are IDR; anything non-finite, negative, or above this is not a +/// real quote, and treating it as missing keeps arithmetic from overflowing. +const MAX_PRICE: f64 = 1e12; + +fn round_price(value: f64) -> Option { + (value.is_finite() && (0.0..=MAX_PRICE).contains(&value)).then(|| value.round() as i64) } pub(super) fn map_yahoo_error(symbol: &str, endpoint: &str, err: &ChartError) -> IdxError { diff --git a/crates/idx-core/src/cache.rs b/crates/idx-core/src/cache.rs index 5e1abb5..85669f8 100644 --- a/crates/idx-core/src/cache.rs +++ b/crates/idx-core/src/cache.rs @@ -185,13 +185,11 @@ impl Cache { } let mut removed = 0usize; let mut failed = Vec::new(); - Self::walk(&self.root, &mut |p| { - if p.is_file() { - match fs::remove_file(p) { - Ok(_) => removed += 1, - Err(_) => failed.push(p.to_path_buf()), - } - } + // `walk` yields files and symlinks, never entering a linked + // directory; remove_file on a symlink removes the link, not its target. + Self::walk(&self.root, &mut |p| match fs::remove_file(p) { + Ok(_) => removed += 1, + Err(_) => failed.push(p.to_path_buf()), })?; Ok((removed, failed)) } @@ -200,7 +198,10 @@ impl Cache { for entry in fs::read_dir(dir).map_err(|e| IdxError::Io(e.to_string()))? { let entry = entry.map_err(|e| IdxError::Io(e.to_string()))?; let path = entry.path(); - if path.is_dir() { + // file_type() does not follow symlinks, so a link to a directory + // outside the cache is never walked (or emptied by `clear`). + let file_type = entry.file_type().map_err(|e| IdxError::Io(e.to_string()))?; + if file_type.is_dir() { Self::walk(&path, f)?; } else { f(&path); @@ -297,6 +298,30 @@ pub fn cache_dir() -> Result { #[cfg(test)] mod tests { + + #[cfg(unix)] + #[test] + fn clear_removes_symlinks_without_touching_their_targets() { + let base = std::env::temp_dir().join(format!("idx-cache-link-{}", fastrand::u64(..))); + let outside = base.join("documents"); + let root = base.join("cache"); + std::fs::create_dir_all(&outside).unwrap(); + std::fs::create_dir_all(root.join("quote")).unwrap(); + std::fs::write(outside.join("keep.txt"), b"mine").unwrap(); + std::fs::write(root.join("quote").join("BBCA.json"), b"{}").unwrap(); + std::os::unix::fs::symlink(&outside, root.join("link")).unwrap(); + + let (removed, failed) = super::Cache::at(root.clone()).clear().unwrap(); + + assert!(failed.is_empty(), "{failed:?}"); + assert_eq!(removed, 2, "the cache file and the link itself"); + assert!( + outside.join("keep.txt").exists(), + "the link's target is left alone" + ); + assert!(!root.join("link").exists()); + std::fs::remove_dir_all(&base).unwrap(); + } use std::fs; use std::sync::atomic::{AtomicUsize, Ordering}; diff --git a/crates/idx-core/src/curl_impersonate.rs b/crates/idx-core/src/curl_impersonate.rs index dbabb8c..a157b09 100644 --- a/crates/idx-core/src/curl_impersonate.rs +++ b/crates/idx-core/src/curl_impersonate.rs @@ -1,4 +1,5 @@ -use std::process::{Command, Output}; +use std::io::Read; +use std::process::{Command, Output, Stdio}; use crate::error::IdxError; @@ -97,8 +98,34 @@ pub fn run_owned_with_fallback( Err(last_error.expect("at least one profile was tried")) } +/// Every curl run gives up after this long, so a stalled server cannot hang +/// a command or an MCP call. +const MAX_TIME_SECS: &str = "600"; +/// Largest response kept in memory (IDX report files are a few MB). +const MAX_RESPONSE_BYTES: usize = 256 * 1024 * 1024; + fn run_binary(stage: &str, binary: &str, args: &[String]) -> Result { - let output = Command::new(binary).args(args).output().map_err(|e| { + run_binary_limited(stage, binary, args, MAX_RESPONSE_BYTES) +} + +/// `--max-filesize` cannot stop a response without a length, nor (before +/// curl 8.20) a small compressed one that decodes to more, so stdout is +/// also capped while it is read: past `limit`, curl is killed and reaped. +fn run_binary_limited( + stage: &str, + binary: &str, + args: &[String], + limit: usize, +) -> Result { + let max_filesize = limit.to_string(); + let mut child = Command::new(binary) + .args(["--max-time", MAX_TIME_SECS, "--max-filesize", max_filesize.as_str()]) + .args(args) + .stdin(Stdio::null()) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + .spawn() + .map_err(|e| { if e.kind() == std::io::ErrorKind::NotFound { return IdxError::Http(format!( "curl-impersonate binary '{binary}' not found; set {OVERRIDE_ENV} or install nixpkgs#curl-impersonate-chrome" @@ -107,6 +134,44 @@ fn run_binary(stage: &str, binary: &str, args: &[String]) -> Result limit { + // The curl_* profiles are scripts that run curl without exec, so the + // kill reaches only the wrapper; curl itself still holds stderr. Do + // not wait for it: closing stdout ends it on its next write (or + // --max-time ends a stalled one), and the stderr thread exits with it. + drop(stdout_pipe); + let _ = child.kill(); + let _ = child.wait(); + drop(stderr_reader); + return Err(match read { + Err(e) => IdxError::Http(format!( + "failed to read {stage} response from {binary}: {e}" + )), + Ok(_) => IdxError::Http(format!("{stage} response exceeds {limit} bytes")), + }); + } + let status = child + .wait() + .map_err(|e| IdxError::Http(format!("failed to wait for {binary} for {stage}: {e}")))?; + let stderr = stderr_reader.join().unwrap_or_default(); + let output = Output { + status, + stdout, + stderr, + }; + if output.status.success() { return Ok(output); } @@ -123,3 +188,32 @@ fn run_binary(stage: &str, binary: &str, args: &[String]) -> Result Result { + let name = name.trim(); + // An empty pattern would match every investor and return an arbitrary one. + if name.is_empty() { + return Err(IdxError::InvalidInput( + "investor name must not be empty".into(), + )); + } let conn = self.connect()?; - let pattern = format!("%{}%", name.trim()); + // `%` and `_` in the name are literal characters, not LIKE wildcards. + let escaped = name + .replace('\\', "\\\\") + .replace('%', "\\%") + .replace('_', "\\_"); + let pattern = format!("%{escaped}%"); let entity_id = conn .query_row( - "SELECT id FROM entities WHERE canonical_name LIKE ?1 COLLATE NOCASE ORDER BY LENGTH(canonical_name) ASC LIMIT 1", + "SELECT id FROM entities WHERE canonical_name LIKE ?1 ESCAPE '\\' COLLATE NOCASE ORDER BY LENGTH(canonical_name) ASC LIMIT 1", rusqlite::params![pattern], |row| row.get::<_, i64>(0), ) @@ -147,7 +159,7 @@ impl Investor { } pub fn search(&self, query: &str, limit: usize) -> Result, IdxError> { - search::fts_search(&*self.connect()?, query, limit) + search::search_entities(&*self.connect()?, query, limit) } pub fn cross_holders( @@ -175,14 +187,29 @@ impl Investor { db::query_bing_flow(&conn, ticker_id) } - /// Holder changes between two release dates, optionally for one ticker. + /// Holder changes between two release dates (`from` earlier than `to`, + /// both imported releases), optionally for one ticker. pub fn changes( &self, from: &str, to: &str, ticker: Option<&str>, ) -> Result, IdxError> { - let mut rows = db::query_changes(&*self.connect()?, from, to)?; + let date = |field: &str, value: &str| { + NaiveDate::parse_from_str(value.trim(), "%Y-%m-%d").map_err(|_| { + IdxError::InvalidInput(format!( + "{field} must be a release date as YYYY-MM-DD (got {value:?})" + )) + }) + }; + let (from_date, to_date) = (date("from", from)?, date("to", to)?); + if from_date >= to_date { + return Err(IdxError::InvalidInput(format!( + "from ({from_date}) must be earlier than to ({to_date})" + ))); + } + let (from, to) = (from_date.to_string(), to_date.to_string()); + let mut rows = db::query_changes(&*self.connect()?, &from, &to)?; if let Some(ticker) = ticker { let ticker = ticker.trim().to_uppercase(); rows.retain(|row| row.ticker_code == ticker); @@ -457,6 +484,14 @@ fn lock_path(db_path: &Path) -> PathBuf { /// not need write access, so readers fall back to opening an existing lock /// file read-only (e.g. one created by another user). fn lock_at(db_path: &Path, exclusive: bool) -> Result { + lock_at_within(db_path, exclusive, LOCK_WAIT) +} + +fn lock_at_within( + db_path: &Path, + exclusive: bool, + wait: std::time::Duration, +) -> Result { let path = lock_path(db_path); let opened = fs::OpenOptions::new() .create(true) @@ -469,15 +504,38 @@ fn lock_at(db_path: &Path, exclusive: bool) -> Result { .map_err(|e| IdxError::Io(format!("failed to open ownership lock: {e}")))?, Err(e) => return Err(IdxError::Io(format!("failed to open ownership lock: {e}"))), }; - if exclusive { - file.lock() - } else { - file.lock_shared() + // Wait a bounded time: a blocking lock behind a long sync or import (or + // a stuck process) would hold every MCP call slot indefinitely. + let deadline = std::time::Instant::now() + wait; + loop { + let attempt = if exclusive { + file.try_lock() + } else { + file.try_lock_shared() + }; + match attempt { + Ok(()) => return Ok(file), + Err(fs::TryLockError::WouldBlock) if std::time::Instant::now() < deadline => { + std::thread::sleep(std::time::Duration::from_millis(100)); + } + Err(fs::TryLockError::WouldBlock) => { + return Err(IdxError::Io(format!( + "ownership database is busy (another idx is syncing or importing); try again in a moment ({})", + path.display() + ))); + } + Err(fs::TryLockError::Error(e)) => { + return Err(IdxError::Io(format!( + "failed to lock ownership database: {e}" + ))); + } + } } - .map_err(|e| IdxError::Io(format!("failed to lock ownership database: {e}")))?; - Ok(file) } +/// How long ownership commands wait for another process's lock. +const LOCK_WAIT: std::time::Duration = std::time::Duration::from_secs(30); + /// True when the lock file is absent and cannot be created. fn unlockable(db_path: &Path) -> bool { let path = lock_path(db_path); @@ -528,7 +586,7 @@ impl FromStr for HolderSource { "all" => Ok(Self::All), "ksei" => Ok(Self::Ksei), "bing" => Ok(Self::Bing), - _ => Err(IdxError::ParseError( + _ => Err(IdxError::InvalidInput( "invalid --source, expected: all|ksei|bing".to_string(), )), } @@ -561,7 +619,7 @@ impl FromStr for ConcentrationRank { "top1" => Ok(Self::Top1), "top3" => Ok(Self::Top3), "hhi" => Ok(Self::Hhi), - _ => Err(IdxError::ParseError( + _ => Err(IdxError::InvalidInput( "invalid --by, expected: top1|top3|hhi".to_string(), )), } @@ -577,7 +635,7 @@ pub fn parse_report_family(raw: &str) -> Result, I "investor-type" | "investor_type" | "investortype" => { Ok(Some(OwnershipReportFamily::InvestorTypeBreakdown)) } - _ => Err(IdxError::ParseError( + _ => Err(IdxError::InvalidInput( "invalid --family, expected: all|above1|above5|investor-type".to_string(), )), } @@ -753,3 +811,21 @@ mod tests { assert!(parse_report_family("above10").is_err()); } } + +#[cfg(test)] +mod lock_tests { + use std::time::Duration; + + use super::lock_at_within; + + #[test] + fn a_held_lock_times_out_with_a_busy_error() { + let db = std::env::temp_dir().join(format!("idx-lock-{}.db", fastrand::u64(..))); + let _held = lock_at_within(&db, true, Duration::ZERO).unwrap(); + + let err = lock_at_within(&db, false, Duration::from_millis(200)).unwrap_err(); + + assert!(err.to_string().contains("busy"), "{err}"); + let _ = std::fs::remove_file(super::lock_path(&db)); + } +} diff --git a/crates/idx-core/src/lib.rs b/crates/idx-core/src/lib.rs index bcc2e52..0e67aca 100644 --- a/crates/idx-core/src/lib.rs +++ b/crates/idx-core/src/lib.rs @@ -15,6 +15,7 @@ pub mod investor; pub mod market; #[cfg(feature = "ownership")] pub mod ownership; +pub mod text; pub use client::{CachePolicy, Fetched, Idx}; @@ -23,6 +24,7 @@ pub mod runtime { use std::sync::atomic::{AtomicBool, Ordering}; static QUIET: AtomicBool = AtomicBool::new(false); + static JSON: AtomicBool = AtomicBool::new(false); pub fn set_quiet(quiet: bool) { QUIET.store(quiet, Ordering::Relaxed); @@ -32,15 +34,32 @@ pub mod runtime { QUIET.load(Ordering::Relaxed) } - pub fn info(message: impl Display) { - if !is_quiet() { - eprintln!("info: {message}"); + /// In JSON mode diagnostics are JSON lines, so stderr stays + /// machine-readable alongside a JSON error. + pub fn set_json(json: bool) { + JSON.store(json, Ordering::Relaxed); + } + + fn emit(level: &str, message: impl Display) { + if is_quiet() { + return; + } + if JSON.load(Ordering::Relaxed) { + let line = serde_json::json!({ "level": level, "message": message.to_string() }); + eprintln!("{line}"); + } else { + eprintln!( + "{level}: {}", + crate::text::terminal_safe(&message.to_string()) + ); } } + pub fn info(message: impl Display) { + emit("info", message); + } + pub fn warn(message: impl Display) { - if !is_quiet() { - eprintln!("warning: {message}"); - } + emit("warning", message); } } diff --git a/crates/idx-core/src/ownership/archive.rs b/crates/idx-core/src/ownership/archive.rs index f883c47..d042308 100644 --- a/crates/idx-core/src/ownership/archive.rs +++ b/crates/idx-core/src/ownership/archive.rs @@ -172,7 +172,10 @@ fn extract_txt_from_zip(path: &Path) -> Result { continue; } - let name = file.name().to_ascii_lowercase(); + // An undecodable entry name cannot be the TXT payload. + let Ok(name) = file.name().map(|name| name.to_ascii_lowercase()) else { + continue; + }; if !name.ends_with(".txt") { continue; } diff --git a/crates/idx-core/src/ownership/db.rs b/crates/idx-core/src/ownership/db.rs index 54ed388..20cab5d 100644 --- a/crates/idx-core/src/ownership/db.rs +++ b/crates/idx-core/src/ownership/db.rs @@ -7,7 +7,6 @@ use rusqlite::{Connection, params}; use crate::config::get_config_value; use crate::error::IdxError; -use crate::ownership::search; use crate::ownership::types::{ BingHolding, ChangeRow, ChangeType, ConcentrationMetrics, CrossHolderRow, Entity, EntityHoldings, EntityTickerRow, FlowSignal, HolderRow, InstitutionalFlow, KseiHolding, @@ -36,14 +35,6 @@ CREATE TABLE IF NOT EXISTS entities ( updated_at INTEGER NOT NULL ); -CREATE VIRTUAL TABLE IF NOT EXISTS entity_fts USING fts5( - canonical_name, - aliases, - content='entities', - content_rowid='id', - tokenize='trigram' -); - CREATE TABLE IF NOT EXISTS entity_aliases ( id INTEGER PRIMARY KEY, entity_id INTEGER NOT NULL REFERENCES entities(id), @@ -364,7 +355,13 @@ pub fn write_ksei_release( } let inserted = insert_ksei_holdings_rows(conn, holdings)?; - let _ = insert_release(conn, release)?; + // Record the rows stored, not parsed: INSERT OR IGNORE drops repeated + // ticker/investor rows, and snapshot validation counts what is stored. + let stored = OwnershipRelease { + row_count: inserted, + ..release.clone() + }; + let _ = insert_release(conn, &stored)?; Ok(inserted) })(); @@ -806,7 +803,7 @@ pub fn query_releases(conn: &Connection) -> Result, IdxErr .prepare( "SELECT id, source_url, sha256, as_of_date, row_count, imported_at FROM ownership_releases - ORDER BY as_of_date DESC, imported_at DESC", + ORDER BY as_of_date DESC, imported_at DESC, id DESC", ) .map_err(|e| IdxError::DatabaseError(e.to_string()))?; @@ -916,43 +913,79 @@ pub fn query_bing_flow( })) } -/// Compare two KSEI snapshots by date and return ownership changes. +/// The release used for a report date: the latest imported one, as +/// `latest_ksei_release` picks, so a corrected file supersedes the original. +/// The latest import holding rows reported on `date`. A file can carry rows +/// from several report dates, so a release is matched by its rows rather +/// than by its `as_of_date` (the latest date in it). +fn release_for_date(conn: &Connection, date: &str) -> Result, IdxError> { + conn.query_row( + "SELECT r.sha256 FROM ownership_releases r + WHERE EXISTS ( + SELECT 1 FROM ksei_holdings k + WHERE k.release_sha256 = r.sha256 AND k.report_date = ?1 + ) + ORDER BY r.imported_at DESC, r.id DESC + LIMIT 1", + params![date], + |row| row.get::<_, String>(0), + ) + .map(Some) + .or_else(|e| match e { + rusqlite::Error::QueryReturnedNoRows => Ok(None), + other => Err(IdxError::DatabaseError(other.to_string())), + }) +} + +/// Compare two KSEI releases by date and return ownership changes. /// Finds: new holders, exited holders, percentage increases/decreases. +/// Each date reads its rows from one release, and a holder's accounts in a ticker are +/// summed, so an investor with two accounts compares by its total stake. pub fn query_changes( conn: &Connection, from_date: &str, to_date: &str, ) -> Result, IdxError> { + let release = |date: &str| { + release_for_date(conn, date)?.ok_or_else(|| { + IdxError::InvalidInput(format!( + "no KSEI release imported for {date}; list the imported dates with `idx ownership releases` or the ownership_status tool" + )) + }) + }; + let (from_release, to_release) = (release(from_date)?, release(to_date)?); let mut stmt = conn .prepare( "WITH from_snapshot AS ( SELECT t.code AS ticker_code, - COALESCE(e.canonical_name, k.raw_investor_name) AS entity_name, + MIN(COALESCE(e.canonical_name, k.raw_investor_name)) AS entity_name, CASE WHEN k.entity_id IS NOT NULL THEN 'id:' || k.entity_id ELSE 'raw:' || UPPER(TRIM(k.raw_investor_name)) END AS holder_key, - k.percentage_bps AS old_bps + SUM(k.percentage_bps) AS old_bps FROM ksei_holdings k JOIN tickers t ON t.id = k.ticker_id LEFT JOIN entities e ON e.id = k.entity_id - WHERE k.report_date = ?1 + WHERE k.release_sha256 = ?1 AND k.report_date = ?3 + GROUP BY t.code, holder_key ), to_snapshot AS ( SELECT t.code AS ticker_code, - COALESCE(e.canonical_name, k.raw_investor_name) AS entity_name, + MIN(COALESCE(e.canonical_name, k.raw_investor_name)) AS entity_name, CASE WHEN k.entity_id IS NOT NULL THEN 'id:' || k.entity_id ELSE 'raw:' || UPPER(TRIM(k.raw_investor_name)) END AS holder_key, - k.percentage_bps AS new_bps + SUM(k.percentage_bps) AS new_bps FROM ksei_holdings k JOIN tickers t ON t.id = k.ticker_id LEFT JOIN entities e ON e.id = k.entity_id - WHERE k.report_date = ?2 + WHERE k.release_sha256 = ?2 AND k.report_date = ?4 + GROUP BY t.code, holder_key ), keys AS ( SELECT ticker_code, holder_key FROM from_snapshot @@ -978,14 +1011,17 @@ pub fn query_changes( .map_err(|e| IdxError::DatabaseError(e.to_string()))?; let rows = stmt - .query_map(params![from_date, to_date], |row| { - Ok(( - row.get::<_, String>(0)?, - row.get::<_, String>(1)?, - row.get::<_, Option>(2)?, - row.get::<_, Option>(3)?, - )) - }) + .query_map( + params![from_release, to_release, from_date, to_date], + |row| { + Ok(( + row.get::<_, String>(0)?, + row.get::<_, String>(1)?, + row.get::<_, Option>(2)?, + row.get::<_, Option>(3)?, + )) + }, + ) .map_err(|e| IdxError::DatabaseError(e.to_string()))?; let mut out = Vec::new(); @@ -1123,7 +1159,6 @@ pub fn manual_map(conn: &Connection, raw_name: &str, canonical_name: &str) -> Re ) .map_err(|e| IdxError::DatabaseError(e.to_string()))?; - let _ = search::rebuild_fts(conn); Ok(()) })(); @@ -1194,7 +1229,6 @@ pub fn merge_entities(conn: &Connection, keep_id: i64, merge_id: i64) -> Result< conn.execute("DELETE FROM entities WHERE id = ?1", params![merge_id]) .map_err(|e| IdxError::DatabaseError(e.to_string()))?; - let _ = search::rebuild_fts(conn); Ok(()) })(); @@ -1279,7 +1313,7 @@ fn latest_ksei_release(conn: &Connection) -> Result, I conn.query_row( "SELECT sha256, as_of_date FROM ownership_releases - ORDER BY as_of_date DESC, imported_at DESC + ORDER BY as_of_date DESC, imported_at DESC, id DESC LIMIT 1", [], |row| { @@ -1383,13 +1417,15 @@ mod tests { use chrono::NaiveDate; use rusqlite::{Connection, params}; + use crate::error::IdxError; use crate::ownership::db::{ compute_concentration, ensure_schema, get_ticker_id, insert_bing_holdings, - insert_ksei_holdings, insert_release, query_concentration, query_cross_holders, - query_ticker_holdings, release_exists, upsert_ticker, write_ksei_release, + insert_ksei_holdings, insert_release, query_changes, query_concentration, + query_cross_holders, query_ticker_holdings, release_exists, upsert_ticker, + write_ksei_release, }; use crate::ownership::types::{ - BingHolding, FlowSignal, KseiHolding, Locality, OwnershipRelease, + BingHolding, ChangeType, FlowSignal, KseiHolding, Locality, OwnershipRelease, }; fn setup() -> Connection { @@ -1398,6 +1434,247 @@ mod tests { conn } + /// Writes one release of `(entity_id, raw_name, bps)` rows for ticker_id. + fn write_release( + conn: &Connection, + sha256: &str, + date: &str, + imported_at: i64, + ticker_id: i64, + rows: &[(Option, &str, i64)], + ) { + let as_of_date = NaiveDate::parse_from_str(date, "%Y-%m-%d").unwrap(); + let holdings: Vec = rows + .iter() + .map(|&(entity_id, raw, bps)| KseiHolding { + id: 0, + ticker_id, + entity_id, + raw_investor_name: raw.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: as_of_date, + release_sha256: sha256.to_string(), + }) + .collect(); + let release = OwnershipRelease { + id: 0, + source_url: None, + sha256: sha256.to_string(), + as_of_date, + row_count: holdings.len(), + imported_at, + }; + write_ksei_release(conn, &release, &holdings, false).unwrap(); + } + + fn entity(conn: &Connection, name: &str) -> i64 { + conn.execute( + "INSERT INTO entities (canonical_name, entity_type, country, created_at, updated_at) + VALUES (?1, NULL, NULL, 0, 0)", + params![name], + ) + .unwrap(); + conn.last_insert_rowid() + } + + #[test] + fn changes_sum_an_investors_accounts_in_a_ticker() { + // KREN, 2026-09: a second account took KRESNA from 9.44% to 13.45%. + let conn = setup(); + let kren = upsert_ticker(&conn, "KREN", None).unwrap(); + let kresna = entity(&conn, "KRESNA PRIMA INVEST"); + write_release( + &conn, + "aug", + "2026-08-31", + 1, + kren, + &[(Some(kresna), "PT KRESNA PRIMA INVEST", 944)], + ); + write_release( + &conn, + "sep", + "2026-09-30", + 2, + kren, + &[ + (Some(kresna), "PT KRESNA PRIMA INVEST", 944), + (Some(kresna), "PT. KRESNA PRIMA INVEST", 401), + ], + ); + + let rows = query_changes(&conn, "2026-08-31", "2026-09-30").unwrap(); + + assert_eq!(rows.len(), 1, "{rows:?}"); + assert_eq!(rows[0].change_type, ChangeType::Increased); + assert_eq!( + (rows[0].old_bps, rows[0].new_bps, rows[0].delta_bps), + (Some(944), Some(1345), 401) + ); + } + + #[test] + fn changes_read_the_latest_import_for_a_date() { + // A corrected January file supersedes the original, so Jan -> Feb at + // the corrected 12% shows no change, not a spurious increase. + let conn = setup(); + let bbca = upsert_ticker(&conn, "BBCA", None).unwrap(); + let alpha = entity(&conn, "ALPHA"); + write_release( + &conn, + "jan", + "2026-01-30", + 1, + bbca, + &[(Some(alpha), "ALPHA", 1000)], + ); + write_release( + &conn, + "jan-fix", + "2026-01-30", + 2, + bbca, + &[(Some(alpha), "ALPHA", 1200)], + ); + write_release( + &conn, + "feb", + "2026-02-27", + 3, + bbca, + &[(Some(alpha), "ALPHA", 1200)], + ); + + assert!( + query_changes(&conn, "2026-01-30", "2026-02-27") + .unwrap() + .is_empty() + ); + } + + #[test] + fn changes_keep_each_date_to_its_own_rows() { + // A file with a January ALPHA row and February BETA row is the + // February release; ALPHA was never held in February, so Feb -> Mar + // must not report it as exited. + let conn = setup(); + let bbca = upsert_ticker(&conn, "BBCA", None).unwrap(); + write_release( + &conn, + "mixed", + "2026-02-27", + 1, + bbca, + &[(None, "ALPHA", 1000), (None, "BETA", 200)], + ); + conn.execute( + "UPDATE ksei_holdings SET report_date = '2026-01-30' WHERE raw_investor_name = 'ALPHA'", + [], + ) + .unwrap(); + write_release(&conn, "mar", "2026-03-31", 2, bbca, &[(None, "BETA", 200)]); + + let rows = query_changes(&conn, "2026-02-27", "2026-03-31").unwrap(); + assert!(rows.is_empty(), "{rows:?}"); + + let rows = query_changes(&conn, "2026-01-30", "2026-03-31").unwrap(); + let alpha = rows.iter().find(|r| r.entity_name == "ALPHA").unwrap(); + assert_eq!(alpha.change_type, ChangeType::Exited); + } + + #[test] + fn same_second_imports_resolve_to_the_later_file_everywhere() { + // Original and corrected file for one date imported in the same + // second: the ticker view and `changes` must read the same one. + let conn = setup(); + let bbca = upsert_ticker(&conn, "BBCA", None).unwrap(); + let alpha = entity(&conn, "ALPHA"); + write_release( + &conn, + "jan", + "2026-01-30", + 7, + bbca, + &[(Some(alpha), "ALPHA", 1000)], + ); + write_release( + &conn, + "jan-fix", + "2026-01-30", + 7, + bbca, + &[(Some(alpha), "ALPHA", 1200)], + ); + + let ticker = query_ticker_holdings(&conn, "BBCA").unwrap(); + + assert_eq!( + ticker.holders[0].percentage_bps, 1200, + "{:?}", + ticker.holders + ); + } + + #[test] + fn a_release_records_the_rows_it_stored() { + // A file listing the same ticker/investor twice stores one row; the + // release metadata must say 1, or snapshot validation rejects it. + let conn = setup(); + let bbca = upsert_ticker(&conn, "BBCA", None).unwrap(); + write_release( + &conn, + "dup", + "2026-02-27", + 1, + bbca, + &[(None, "ALPHA", 1000), (None, "ALPHA", 1000)], + ); + + let row_count: i64 = conn + .query_row( + "SELECT row_count FROM ownership_releases WHERE sha256 = 'dup'", + [], + |row| row.get(0), + ) + .unwrap(); + let stored: i64 = conn + .query_row( + "SELECT COUNT(*) FROM ksei_holdings WHERE release_sha256 = 'dup'", + [], + |row| row.get(0), + ) + .unwrap(); + assert_eq!((row_count, stored), (1, 1)); + } + + #[test] + fn changes_reject_a_date_without_a_release() { + let conn = setup(); + let bbca = upsert_ticker(&conn, "BBCA", None).unwrap(); + write_release( + &conn, + "feb", + "2026-02-27", + 1, + bbca, + &[(None, "ALPHA", 1200)], + ); + + let err = query_changes(&conn, "2026-01-30", "2026-02-27").unwrap_err(); + + assert!( + matches!(err, IdxError::InvalidInput(ref m) if m.contains("2026-01-30")), + "{err:?}" + ); + } + #[test] fn test_compute_concentration_pure_function() { let metrics = compute_concentration(&[4000, 2500, 1000, 500]); diff --git a/crates/idx-core/src/ownership/entities.rs b/crates/idx-core/src/ownership/entities.rs index f2a9873..bea4ede 100644 --- a/crates/idx-core/src/ownership/entities.rs +++ b/crates/idx-core/src/ownership/entities.rs @@ -109,34 +109,25 @@ pub fn normalize_name(raw: &str) -> String { return upper; } - let mut tokens: Vec<&str> = upper.split_whitespace().collect(); - - loop { - let mut changed = false; - - if let Some(first) = tokens.first().copied() - && matches!(first, "PT" | "PT.") - { - tokens.remove(0); - changed = true; - } - - if let Some(last) = tokens.last().copied() - && matches!( - last, - "TBK" | "Tbk" | "(PERSERO)" | "LIMITED" | "PTE" | "LTD" - ) - { - let _ = tokens.pop(); - changed = true; - } - - if !changed { - break; - } + let tokens: Vec<&str> = upper.split_whitespace().collect(); + // Trim prefix and suffix tokens by narrowing a slice: linear, where + // removing from the front of the Vec was quadratic in hostile names. + let mut kept = tokens.as_slice(); + while let [first, rest @ ..] = kept + && matches!(*first, "PT" | "PT.") + { + kept = rest; + } + while let [rest @ .., last] = kept + && matches!( + *last, + "TBK" | "Tbk" | "(PERSERO)" | "LIMITED" | "PTE" | "LTD" + ) + { + kept = rest; } - collapse_whitespace(&tokens.join(" ")) + collapse_whitespace(&kept.join(" ")) } /// Convert a KseiRawRow into a normalized KseiHoldingDraft. @@ -357,6 +348,14 @@ mod tests { ); } + #[test] + fn normalize_name_strips_many_prefixes_in_linear_time() { + // Removing from the front of a Vec made this quadratic (~5e9 shifts). + let hostile = format!("{}ACME TBK", "PT ".repeat(100_000)); + assert_eq!(normalize_name(&hostile), "ACME"); + assert_eq!(normalize_name("PT TBK"), ""); + } + #[test] fn test_normalize_name() { assert_eq!( diff --git a/crates/idx-core/src/ownership/graph.rs b/crates/idx-core/src/ownership/graph.rs index 981e5cc..274ec81 100644 --- a/crates/idx-core/src/ownership/graph.rs +++ b/crates/idx-core/src/ownership/graph.rs @@ -14,82 +14,38 @@ pub fn query_ownership_graph( ) -> Result<(Vec, Vec), IdxError> { let root = root.trim(); if root.is_empty() { - return Err(IdxError::ParseError( + return Err(IdxError::InvalidInput( "graph root cannot be empty".to_string(), )); } let root_node_id = detect_root_node(conn, root)?; - let mut visited: BTreeSet = BTreeSet::new(); - { - let mut stmt = conn - .prepare( - "WITH RECURSIVE - latest_ksei AS ( - SELECT sha256 - FROM ownership_releases - ORDER BY as_of_date DESC, imported_at DESC - LIMIT 1 - ), - latest_bing AS ( - SELECT ticker_id, MAX(report_date) AS report_date - FROM bing_holdings - GROUP BY ticker_id - ), - all_edges AS ( - SELECT - 'entity:' || k.entity_id AS from_id, - 'ticker:' || t.code AS to_id - FROM ksei_holdings k - JOIN tickers t ON t.id = k.ticker_id - JOIN latest_ksei lk ON lk.sha256 = k.release_sha256 - WHERE k.entity_id IS NOT NULL - - UNION - - SELECT - 'entity:' || b.entity_id AS from_id, - 'ticker:' || t.code AS to_id - FROM bing_holdings b - JOIN latest_bing lb - ON lb.ticker_id = b.ticker_id - AND lb.report_date = b.report_date - JOIN tickers t ON t.id = b.ticker_id - WHERE b.entity_id IS NOT NULL - AND b.signal = 'holder' - ), - neighbors AS ( - SELECT from_id AS a, to_id AS b FROM all_edges - UNION - SELECT to_id AS a, from_id AS b FROM all_edges - ), - walk(node_id, depth, path) AS ( - SELECT ?1 AS node_id, 0 AS depth, ?1 AS path - UNION ALL - SELECT n.b, w.depth + 1, w.path || '>' || n.b - FROM walk w - JOIN neighbors n ON n.a = w.node_id - WHERE w.depth < ?2 - AND instr(w.path, n.b) = 0 - ) - SELECT DISTINCT node_id - FROM walk", - ) - .map_err(|e| IdxError::DatabaseError(e.to_string()))?; - - let rows = stmt - .query_map(params![root_node_id, depth as i64], |row| { - row.get::<_, String>(0) - }) - .map_err(|e| IdxError::DatabaseError(e.to_string()))?; - - for row in rows { - visited.insert(row.map_err(|e| IdxError::DatabaseError(e.to_string()))?); - } - } - + // Breadth-first over the latest edges with an exact visited set: linear + // in the graph size, unlike enumerating every path, and a node is never + // mistaken for one whose id merely contains it (`entity:1` in `entity:10`). let mut edges = query_all_edges(conn)?; + let mut neighbors: HashMap<&str, Vec<&str>> = HashMap::new(); + for edge in &edges { + neighbors.entry(&edge.from).or_default().push(&edge.to); + neighbors.entry(&edge.to).or_default().push(&edge.from); + } + let mut visited: BTreeSet = BTreeSet::from([root_node_id.clone()]); + let mut frontier = vec![root_node_id.as_str()]; + for _ in 0..depth { + let mut next = Vec::new(); + for node in frontier { + for &neighbor in neighbors.get(node).into_iter().flatten() { + if visited.insert(neighbor.to_string()) { + next.push(neighbor); + } + } + } + if next.is_empty() { + break; + } + frontier = next; + } edges.retain(|edge| visited.contains(&edge.from) && visited.contains(&edge.to)); let nodes = build_nodes(conn, &visited)?; @@ -226,7 +182,7 @@ fn query_all_edges(conn: &Connection) -> Result, IdxError> { if let Ok(release_sha) = conn.query_row( "SELECT sha256 FROM ownership_releases - ORDER BY as_of_date DESC, imported_at DESC + ORDER BY as_of_date DESC, imported_at DESC, id DESC LIMIT 1", [], |row| row.get::<_, String>(0), @@ -380,6 +336,58 @@ mod tests { conn } + #[test] + fn graph_reaches_holders_whose_id_is_a_prefix_of_a_visited_one() { + // From entity:10 via ticker:BB to entity:1; "entity:1" is a substring + // of "entity:10", which the old path check treated as visited. + let conn = setup(); + let bb = upsert_ticker(&conn, "BB", None).unwrap(); + conn.execute( + "INSERT INTO entities (id, canonical_name, created_at, updated_at) + VALUES (10, 'HOLDER TEN', 0, 0), (1, 'HOLDER ONE', 0, 0)", + [], + ) + .unwrap(); + let release = OwnershipRelease { + id: 0, + source_url: None, + sha256: "prefix-graph".to_string(), + as_of_date: NaiveDate::from_ymd_opt(2026, 2, 27).unwrap(), + row_count: 2, + imported_at: 1, + }; + let row = |entity_id: i64, name: &str| KseiHolding { + id: 0, + ticker_id: bb, + entity_id: Some(entity_id), + 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: 600, + report_date: release.as_of_date, + release_sha256: release.sha256.clone(), + }; + write_ksei_release( + &conn, + &release, + &[row(10, "HOLDER TEN"), row(1, "HOLDER ONE")], + false, + ) + .unwrap(); + + let (nodes, edges) = query_ownership_graph(&conn, "HOLDER TEN", 2).unwrap(); + + let mut labels: Vec<_> = nodes.iter().map(|node| node.label.as_str()).collect(); + labels.sort(); + assert_eq!(labels, ["BB", "HOLDER ONE", "HOLDER TEN"]); + assert_eq!(edges.len(), 2); + } + #[test] fn graph_uses_latest_ksei_release_only() { let conn = setup(); diff --git a/crates/idx-core/src/ownership/search.rs b/crates/idx-core/src/ownership/search.rs index 6e63532..9b0af1e 100644 --- a/crates/idx-core/src/ownership/search.rs +++ b/crates/idx-core/src/ownership/search.rs @@ -3,47 +3,50 @@ use rusqlite::{Connection, params}; use crate::error::IdxError; use crate::ownership::types::Entity; -pub fn fts_search(conn: &Connection, query: &str, limit: usize) -> Result, IdxError> { +/// Investors whose canonical name or any recorded alias (the raw names KSEI +/// prints, such as `PT ASTRA INTERNATIONAL TBK`) contains `query`, ignoring +/// ASCII case. Ranked by an exact name, then an exact alias, then a name +/// prefix, then shorter names. +pub fn search_entities( + conn: &Connection, + query: &str, + limit: usize, +) -> Result, IdxError> { let q = query.trim(); if q.is_empty() { return Ok(Vec::new()); } - - let limit_i64 = + let limit = i64::try_from(limit).map_err(|e| IdxError::DatabaseError(format!("invalid limit: {e}")))?; + // `%` and `_` in the query are literal characters, not LIKE wildcards. + let escaped = q + .replace('\\', "\\\\") + .replace('%', "\\%") + .replace('_', "\\_"); + let contains = format!("%{escaped}%"); + let prefix = format!("{escaped}%"); - // Ensure entity_fts has content. For external-content FTS table, populate manually. - let fts_count: i64 = conn - .query_row("SELECT COUNT(*) FROM entity_fts", [], |row| row.get(0)) - .unwrap_or(0); - - if fts_count == 0 { - let _ = rebuild_fts(conn); - } - - if let Ok(rows) = fts_query(conn, q, limit_i64) - && !rows.is_empty() - { - return Ok(rows); - } - - like_query(conn, q, limit_i64) -} - -fn fts_query(conn: &Connection, query: &str, limit: i64) -> Result, IdxError> { let mut stmt = conn .prepare( - "SELECT e.id, e.canonical_name, e.entity_type, e.country - FROM entity_fts f - JOIN entities e ON e.id = f.rowid - WHERE entity_fts MATCH ?1 - ORDER BY rank - LIMIT ?2", + "SELECT e.id, e.canonical_name, e.entity_type, e.country, + MIN(CASE + WHEN UPPER(e.canonical_name) = UPPER(?1) THEN 0 + WHEN UPPER(a.raw_name) = UPPER(?1) THEN 1 + WHEN e.canonical_name LIKE ?3 ESCAPE '\\' THEN 2 + ELSE 3 + END) AS match_rank + FROM entities e + LEFT JOIN entity_aliases a ON a.entity_id = e.id + WHERE e.canonical_name LIKE ?2 ESCAPE '\\' + OR a.raw_name LIKE ?2 ESCAPE '\\' + GROUP BY e.id + ORDER BY match_rank, LENGTH(e.canonical_name), e.canonical_name + LIMIT ?4", ) .map_err(|e| IdxError::DatabaseError(e.to_string()))?; let rows = stmt - .query_map(params![query, limit], |row| { + .query_map(params![q, contains, prefix, limit], |row| { Ok(Entity { id: row.get(0)?, canonical_name: row.get(1)?, @@ -53,59 +56,102 @@ fn fts_query(conn: &Connection, query: &str, limit: i64) -> Result, }) .map_err(|e| IdxError::DatabaseError(e.to_string()))?; - let mut out = Vec::new(); - for row in rows { - out.push(row.map_err(|e| IdxError::DatabaseError(e.to_string()))?); - } - Ok(out) + rows.map(|row| row.map_err(|e| IdxError::DatabaseError(e.to_string()))) + .collect() } -fn like_query(conn: &Connection, query: &str, limit: i64) -> Result, IdxError> { - let mut stmt = conn - .prepare( - "SELECT id, canonical_name, entity_type, country - FROM entities - WHERE canonical_name LIKE ?1 COLLATE NOCASE - ORDER BY canonical_name ASC - LIMIT ?2", - ) - .map_err(|e| IdxError::DatabaseError(e.to_string()))?; - - let like = format!("%{query}%"); - let rows = stmt - .query_map(params![like, limit], |row| { - Ok(Entity { - id: row.get(0)?, - canonical_name: row.get(1)?, - entity_type: row.get(2)?, - country: row.get(3)?, - }) - }) - .map_err(|e| IdxError::DatabaseError(e.to_string()))?; - - let mut out = Vec::new(); - for row in rows { - out.push(row.map_err(|e| IdxError::DatabaseError(e.to_string()))?); - } - Ok(out) +/// Former name of [`search_entities`], kept for library users. +#[deprecated(since = "0.3.3", note = "use search_entities")] +pub fn fts_search(conn: &Connection, query: &str, limit: usize) -> Result, IdxError> { + search_entities(conn, query, limit) } -pub fn rebuild_fts(conn: &Connection) -> Result<(), IdxError> { - conn.execute("DELETE FROM entity_fts", []) - .map_err(|e| IdxError::DatabaseError(e.to_string()))?; - - conn.execute( - "INSERT INTO entity_fts(rowid, canonical_name, aliases) - SELECT e.id, e.canonical_name, COALESCE(a.aliases, '') - FROM entities e - LEFT JOIN ( - SELECT entity_id, GROUP_CONCAT(raw_name, ' ') AS aliases - FROM entity_aliases - GROUP BY entity_id - ) a ON a.entity_id = e.id", - [], - ) - .map_err(|e| IdxError::DatabaseError(e.to_string()))?; - +/// Search no longer uses an FTS index, so there is nothing to rebuild; kept +/// for library users. +#[deprecated(since = "0.3.3", note = "search reads entities and aliases directly")] +pub fn rebuild_fts(_conn: &Connection) -> Result<(), IdxError> { Ok(()) } + +#[cfg(test)] +mod tests { + use rusqlite::{Connection, params}; + + use super::search_entities; + use crate::ownership::db::ensure_schema; + + fn conn_with(entities: &[(&str, &[&str])]) -> Connection { + let conn = Connection::open_in_memory().unwrap(); + ensure_schema(&conn).unwrap(); + for (name, aliases) in entities { + conn.execute( + "INSERT INTO entities (canonical_name, created_at, updated_at) VALUES (?1, 0, 0)", + params![name], + ) + .unwrap(); + let id = conn.last_insert_rowid(); + for alias in *aliases { + conn.execute( + "INSERT INTO entity_aliases (entity_id, raw_name, source, method) + VALUES (?1, ?2, 'ksei', 'exact')", + params![id, alias], + ) + .unwrap(); + } + } + conn + } + + fn names(conn: &Connection, query: &str) -> Vec { + search_entities(conn, query, 10) + .unwrap() + .into_iter() + .map(|e| e.canonical_name) + .collect() + } + + #[test] + fn finds_investors_by_alias() { + let conn = conn_with(&[ + ("ASTRA INTERNATIONAL", &["PT ASTRA INTERNATIONAL TBK"]), + ("ASTRA OTOPARTS", &[]), + ]); + + assert_eq!( + names(&conn, "pt astra international tbk"), + ["ASTRA INTERNATIONAL"] + ); + } + + #[test] + fn ranks_exact_then_prefix_then_shorter_and_lists_each_once() { + let conn = conn_with(&[ + ("SALIM GROUP HOLDINGS", &["PT SALIM", "SALIM"]), + ("ANTHONI SALIM", &[]), + ("SALIM", &[]), + ]); + + assert_eq!( + names(&conn, "salim"), + ["SALIM", "SALIM GROUP HOLDINGS", "ANTHONI SALIM"] + ); + } + + #[test] + fn a_name_prefix_ranks_above_a_mid_name_match() { + // Neither is an exact name or alias: only prefix vs substring differs, + // and the substring match is shorter, so length alone would flip them. + let conn = conn_with(&[("BANK MANDIRI TASPEN", &[]), ("PT BANK", &[])]); + + assert_eq!(names(&conn, "bank"), ["BANK MANDIRI TASPEN", "PT BANK"]); + } + + #[test] + fn treats_like_wildcards_literally_and_empty_queries_as_no_match() { + let conn = conn_with(&[("ALPHA_FUND", &[]), ("ALPHAXFUND", &[]), ("BETA 100%", &[])]); + + assert_eq!(names(&conn, "alpha_fund"), ["ALPHA_FUND"]); + assert_eq!(names(&conn, "100%"), ["BETA 100%"]); + assert!(names(&conn, " ").is_empty()); + } +} diff --git a/crates/idx-core/src/ownership/snapshot.rs b/crates/idx-core/src/ownership/snapshot.rs index 9c4e6fb..91e59a5 100644 --- a/crates/idx-core/src/ownership/snapshot.rs +++ b/crates/idx-core/src/ownership/snapshot.rs @@ -161,6 +161,10 @@ pub fn sync_snapshot( let resolved_db_path = db::resolve_db_path(db_path)?; let db_path = resolved_db_path.as_path(); let manifest = fetch_manifest(manifest_source)?; + { + let _lock = lock(db_path, true)?; + recover_interrupted_install(db_path)?; + } let local_state = { let _shared = lock(db_path, false)?; inspect_local_db(db_path)? @@ -189,6 +193,7 @@ pub fn sync_snapshot( // A body larger than the manifest says is rejected while downloading // (ureq fails once a body reaches its limit, hence the +1); the exact size // and SHA-256 are verified afterwards. + check_download_source(manifest_source, &manifest.snapshot.download_url)?; let bytes = read_bytes( &manifest.snapshot.download_url, "ownership snapshot SQLite", @@ -506,17 +511,12 @@ fn build_sync_decision( }; } - if latest.sha256 == snapshot.latest_release_sha256 { - if latest.row_count != snapshot.latest_row_count - && snapshot.release_count >= local.release_count - { - return SyncDecision { - action: OwnershipSyncAction::Updated, - should_download: true, - reason: "local ownership database has the same latest release date but different row_count metadata".to_string(), - }; - } - + // A same-date rebuild replaces the local file only through + // `replaces_sqlite_sha256` above; differing row counts alone would let a + // replayed older build undo a corrected one. + if latest.sha256 == snapshot.latest_release_sha256 + && latest.row_count == snapshot.latest_row_count + { if local.release_count < snapshot.release_count { return SyncDecision { action: OwnershipSyncAction::Updated, @@ -551,6 +551,18 @@ fn build_sync_decision( } } +/// A manifest fetched over HTTP(S) must not name a local file: it could make +/// sync read a device such as /dev/zero, or any local file. A local manifest +/// (the user's own) may. +fn check_download_source(manifest_source: &str, download_url: &str) -> Result<(), IdxError> { + if is_http_source(manifest_source) && !is_http_source(download_url) { + return Err(IdxError::InvalidInput(format!( + "ownership snapshot manifest from {manifest_source} names a non-HTTP(S) download_url: {download_url}" + ))); + } + Ok(()) +} + fn validate_downloaded_bytes( bytes: &[u8], snapshot: &OwnershipSnapshotArtifact, @@ -626,6 +638,21 @@ fn validate_snapshot_db(path: &Path, snapshot: &OwnershipSnapshotArtifact) -> Re snapshot.latest_row_count, latest.row_count ))); } + // Count the stored rows too: release metadata alone would accept a + // snapshot whose holdings table is empty or missing. + let stored_rows: i64 = conn + .query_row( + "SELECT COUNT(*) FROM ksei_holdings WHERE release_sha256 = ?1", + rusqlite::params![latest.sha256], + |row| row.get(0), + ) + .map_err(|e| IdxError::DatabaseError(format!("failed counting snapshot holdings: {e}")))?; + if usize::try_from(stored_rows).ok() != Some(snapshot.latest_row_count) { + return Err(IdxError::ParseError(format!( + "downloaded snapshot holds {stored_rows} rows for its latest release; manifest expected {}", + snapshot.latest_row_count + ))); + } let ticker_count: i64 = conn .query_row("SELECT COUNT(*) FROM tickers", [], |row| row.get(0)) @@ -668,52 +695,44 @@ fn install_snapshot_file(temp_path: &Path, db_path: &Path) -> Result<(), IdxErro })?; } - let backup_path = build_backup_path(db_path); - if backup_path.exists() { - fs::remove_file(&backup_path).map_err(|e| { - IdxError::Io(format!( - "failed to remove stale backup {}: {e}", - backup_path.display() - )) - })?; - } - - let had_existing = db_path.exists(); + // One rename replaces the old database atomically: a crash leaves either + // the old file or the new one, never neither. Queries do not write to a + // synced file, so its sidecars hold no data and go first. remove_sqlite_sidecars(db_path)?; - - if had_existing { - fs::rename(db_path, &backup_path).map_err(|e| { - IdxError::Io(format!( - "failed to move existing ownership database {} to backup {}: {e}", - db_path.display(), - backup_path.display() - )) - })?; - } - - if let Err(err) = fs::rename(temp_path, db_path) { - if had_existing { - let _ = fs::rename(&backup_path, db_path); - } - return Err(IdxError::Io(format!( - "failed to install ownership snapshot into {}: {err}", + fs::rename(temp_path, db_path).map_err(|e| { + IdxError::Io(format!( + "failed to install ownership snapshot into {}: {e}", db_path.display() - ))); - } - - if had_existing && backup_path.exists() { - fs::remove_file(&backup_path).map_err(|e| { - IdxError::Io(format!( - "failed to remove snapshot backup {}: {e}", - backup_path.display() - )) - })?; - } + )) + })?; remove_sqlite_sidecars(db_path)?; Ok(()) } +/// Versions before 0.3.3 moved the database to a backup before installing; +/// a crash between the two renames left only the backup. Restore it, or drop +/// it when the database is there. Runs before the local database is +/// inspected, so the restored one gets the usual newer/modified safeguards +/// and stays in place if the download then fails. +fn recover_interrupted_install(db_path: &Path) -> Result<(), IdxError> { + let backup_path = build_backup_path(db_path); + if !backup_path.exists() { + return Ok(()); + } + let result = if db_path.exists() { + fs::remove_file(&backup_path) + } else { + fs::rename(&backup_path, db_path) + }; + result.map_err(|e| { + IdxError::Io(format!( + "failed to resolve leftover backup {}: {e}", + backup_path.display() + )) + }) +} + fn build_backup_path(db_path: &Path) -> PathBuf { let file_name = db_path .file_name() @@ -772,17 +791,31 @@ fn read_text(source: &str, context: &str) -> Result { .header("Accept", "application/json,text/plain;q=0.9,*/*;q=0.8") .call() .map_err(|e| IdxError::Http(format!("failed to fetch {context}: {e}")))?; - let mut body = response.into_body(); - return body + return response + .into_body() + .with_config() + .limit(MANIFEST_LIMIT_BYTES) .read_to_string() .map_err(|e| IdxError::Http(format!("failed reading {context} body: {e}"))); } let path = local_source_path(source); - fs::read_to_string(&path) - .map_err(|e| IdxError::Io(format!("failed to read {context} {}: {e}", path.display()))) + // One byte over the limit tells an oversized manifest from one that fits. + let bytes = read_local_bounded(&path, context, MANIFEST_LIMIT_BYTES + 1)?; + if bytes.len() as u64 > MANIFEST_LIMIT_BYTES { + return Err(IdxError::InvalidInput(format!( + "{context} {} is larger than {MANIFEST_LIMIT_BYTES} bytes", + path.display() + ))); + } + String::from_utf8(bytes).map_err(|e| { + IdxError::ParseError(format!("{context} {} is not UTF-8: {e}", path.display())) + }) } +/// A manifest is a few KB; anything near this is not one. +const MANIFEST_LIMIT_BYTES: u64 = 1 << 20; + /// Downloads give up rather than hang: a stalled connection must not keep /// sync (and anything waiting on it) blocked forever. fn http_agent() -> ureq::Agent { @@ -816,9 +849,26 @@ fn read_bytes(source: &str, context: &str, limit_bytes: u64) -> Result, }); } - let path = local_source_path(source); - fs::read(&path) - .map_err(|e| IdxError::Io(format!("failed to read {context} {}: {e}", path.display()))) + read_local_bounded(&local_source_path(source), context, limit_bytes) +} + +/// Reads at most `limit_bytes` of a regular file (not a device or FIFO); a +/// larger file comes back truncated and fails the caller's size check. +fn read_local_bounded(path: &Path, context: &str, limit_bytes: u64) -> Result, IdxError> { + let read_error = |e: std::io::Error| { + IdxError::Io(format!("failed to read {context} {}: {e}", path.display())) + }; + let file = fs::File::open(path).map_err(read_error)?; + if !file.metadata().map_err(read_error)?.is_file() { + return Err(IdxError::InvalidInput(format!( + "{context} {} is not a regular file", + path.display() + ))); + } + let mut bytes = Vec::new(); + std::io::Read::read_to_end(&mut std::io::Read::take(file, limit_bytes), &mut bytes) + .map_err(read_error)?; + Ok(bytes) } fn is_http_source(source: &str) -> bool { @@ -841,7 +891,8 @@ mod tests { use super::{ DEFAULT_SNAPSHOT_MANIFEST_URL, OwnershipSnapshotArtifact, OwnershipSnapshotManifest, OwnershipSnapshotSource, OwnershipSyncAction, SNAPSHOT_MANIFEST_SCHEMA_VERSION, - build_sync_decision, parse_manifest, resolve_manifest_source_with, + build_sync_decision, check_download_source, parse_manifest, read_local_bounded, + resolve_manifest_source_with, validate_snapshot_db, }; use crate::ownership::db::{ensure_schema, insert_release}; use crate::ownership::types::OwnershipRelease; @@ -984,6 +1035,102 @@ mod tests { assert!(!decision.should_download, "{}", decision.reason); } + #[test] + fn build_sync_decision_ignores_a_replayed_older_build_for_the_same_date() { + // Local holds corrected build B (7200 rows); an older build A for the + // same release (7154 rows) that does not list B must not undo it. + let snapshot = same_date_snapshot(Vec::new()); + let mut local = same_release_local(&snapshot, &"c".repeat(64)); + if let Some(release) = local.latest_release.as_mut() { + release.row_count = 7200; + } + + let decision = build_sync_decision(Some(&local), &snapshot, false); + assert!(!decision.should_download, "{}", decision.reason); + } + + #[test] + fn validation_rejects_a_snapshot_whose_holdings_are_missing() { + // Release metadata and manifest agree (7154 rows), but no holdings + // are stored: installing it would replace good data with nothing. + let snapshot = same_date_snapshot(Vec::new()); + let path = std::env::temp_dir().join(format!("idx-empty-{}.sqlite", fastrand::u64(..))); + let conn = Connection::open(&path).unwrap(); + ensure_schema(&conn).unwrap(); + for (index, sha256) in ["1", "2"].into_iter().map(|c| c.repeat(64)).enumerate() { + let as_of_date = NaiveDate::from_ymd_opt(2026, 7 + index as u32, 28).unwrap(); + insert_release( + &conn, + &OwnershipRelease { + id: 0, + source_url: None, + sha256, + as_of_date, + row_count: 1, + imported_at: 0, + }, + ) + .unwrap(); + } + insert_release( + &conn, + &OwnershipRelease { + id: 0, + source_url: None, + sha256: snapshot.latest_release_sha256.clone(), + as_of_date: snapshot.latest_as_of_date, + row_count: snapshot.latest_row_count, + imported_at: 0, + }, + ) + .unwrap(); + for code in 0..snapshot.ticker_count { + conn.execute( + "INSERT INTO tickers (code) VALUES (?1)", + [format!("T{code}")], + ) + .unwrap(); + } + drop(conn); + + let err = validate_snapshot_db(&path, &snapshot).expect_err("no holdings stored"); + assert!(err.to_string().contains("holds 0 rows"), "{err}"); + std::fs::remove_file(&path).unwrap(); + } + + #[test] + fn remote_manifests_may_not_name_local_artifacts() { + let remote = "https://example.com/ownership-snapshot-manifest.json"; + assert!(check_download_source(remote, "/dev/zero").is_err()); + assert!(check_download_source(remote, "file:///etc/passwd").is_err()); + assert!(check_download_source(remote, "https://example.com/a.sqlite").is_ok()); + // The user's own local manifest may point at a local file. + assert!(check_download_source("/tmp/manifest.json", "/tmp/a.sqlite").is_ok()); + } + + #[cfg(unix)] + #[test] + fn local_reads_refuse_devices() { + let err = read_local_bounded(std::path::Path::new("/dev/zero"), "snapshot", 16) + .expect_err("a device is not a snapshot file"); + assert!(err.to_string().contains("not a regular file"), "{err}"); + } + + #[test] + fn local_manifests_over_the_limit_are_rejected_not_truncated() { + let path = std::env::temp_dir().join(format!("idx-manifest-{}.json", fastrand::u64(..))); + std::fs::write( + &path, + vec![b' '; (super::MANIFEST_LIMIT_BYTES + 1) as usize], + ) + .unwrap(); + + let err = super::read_text(path.to_str().unwrap(), "manifest").unwrap_err(); + + assert!(err.to_string().contains("larger than"), "{err}"); + std::fs::remove_file(&path).unwrap(); + } + #[test] fn build_sync_decision_keeps_a_locally_modified_database() { // Same release metadata but a file matching no published build (local diff --git a/crates/idx-core/src/ownership/xlsx.rs b/crates/idx-core/src/ownership/xlsx.rs index c8650d9..58751f8 100644 --- a/crates/idx-core/src/ownership/xlsx.rs +++ b/crates/idx-core/src/ownership/xlsx.rs @@ -87,11 +87,13 @@ fn read_zip_entry( } fn first_worksheet_name(zip: &mut zip::ZipArchive>) -> Result { + // Undecodable entry names cannot be worksheets; skip them. let mut sheets: Vec = zip .file_names() + .filter_map(Result::ok) .filter(|name| name.starts_with("xl/worksheets/") && name.ends_with(".xml")) .filter(|name| !name["xl/worksheets/".len()..].contains('/')) - .map(str::to_string) + .map(|name| name.into_owned()) .collect(); // sheet1.xml, sheet2.xml, ... sort by their numeric suffix. sheets.sort_by_key(|name| { diff --git a/crates/idx-core/src/text.rs b/crates/idx-core/src/text.rs new file mode 100644 index 0000000..4421815 --- /dev/null +++ b/crates/idx-core/src/text.rs @@ -0,0 +1,44 @@ +//! Text from providers, KSEI files, and snapshot manifests is untrusted. +//! Before it reaches a terminal, control characters (which can start escape +//! sequences that clear the screen, rewrite output, or set the window title) +//! and bidirectional overrides (which can reorder what is displayed) are +//! replaced. + +/// `text` with control characters other than newline, and bidirectional +/// override characters, replaced by U+FFFD. +pub fn terminal_safe(text: &str) -> String { + text.chars() + .map(|ch| { + let bidi = matches!(ch, '\u{202A}'..='\u{202E}' | '\u{2066}'..='\u{2069}'); + if (ch.is_control() && ch != '\n') || bidi { + '\u{FFFD}' + } else { + ch + } + }) + .collect() +} + +#[cfg(test)] +mod tests { + use super::terminal_safe; + + #[test] + fn escape_sequences_and_bidi_overrides_are_neutralized() { + assert_eq!( + terminal_safe("\u{1b}[2J\u{1b}[Hfake"), + "\u{FFFD}[2J\u{FFFD}[Hfake" + ); + assert_eq!( + terminal_safe("\u{1b}]0;title\u{7}x"), + "\u{FFFD}]0;title\u{FFFD}x" + ); + assert_eq!(terminal_safe("a\u{9b}31mb"), "a\u{FFFD}31mb"); + assert_eq!(terminal_safe("ab\u{202E}cd"), "ab\u{FFFD}cd"); + assert_eq!(terminal_safe("line one\nline two"), "line one\nline two"); + assert_eq!( + terminal_safe("Saham Grup Djarum’s 14,81%"), + "Saham Grup Djarum’s 14,81%" + ); + } +} diff --git a/crates/idx-mcp/tests/mcp.rs b/crates/idx-mcp/tests/mcp.rs index 906c87f..8438bfc 100644 --- a/crates/idx-mcp/tests/mcp.rs +++ b/crates/idx-mcp/tests/mcp.rs @@ -394,6 +394,22 @@ fn ownership_tools_read_the_local_database() { assert!(page["next_offset"].is_null(), "offset {past}: {page}"); } + // Bad, unknown, or reversed dates are errors, not "everyone is new". + for (from, to) in [ + ("", "2026-02-27"), + ("1999-01-01", "2026-02-27"), + ("2026-02-27", "2026-01-30"), + ("2026-01-30", "2026-01-30"), + ] { + let (is_error, body) = + client.call("get_ownership_changes", json!({"from": from, "to": to})); + assert!(is_error, "{from}..{to}: {body}"); + assert_eq!(body["code"], "INVALIDINPUT", "{from}..{to}: {body}"); + } + let (is_error, body) = client.call("get_investor_holdings", json!({"entity": " "})); + assert!(is_error, "{body}"); + assert_eq!(body["code"], "INVALIDINPUT", "{body}"); + let (is_error, body) = client.call( "get_ownership_changes", json!({"from": "2026-01-30", "to": "2026-02-27", "limit": 301}), diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index d5c10ae..bb7d1dd 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -162,7 +162,12 @@ CLI flags > environment variables > config file > defaults ## Error Strategy - `IdxError` enum (thiserror) with structured error codes - Table mode: human-readable error on stderr -- JSON and JSONL modes: `{"error": true, "code": "...", "message": "..."}` on stderr, stdout empty +- JSON and JSONL modes: `{"error": true, "code": "...", "message": "..."}` on stderr, stdout empty. + This includes argument errors (`INVALIDINPUT`, exit 2) when `-o`/`--output`, + `IDX_OUTPUT`, or auto mode with stdout not a terminal selects JSON; `--help` + and `--version` print normally. +- Untrusted text (provider strings, KSEI names) has control characters and + bidirectional overrides replaced before it reaches a terminal table. - Exit codes by class: `0` success, `2` `INVALIDINPUT`/`CONFIGERROR` and clap usage errors, `3` `SYMBOLNOTFOUND`/`NOMARKETDATA`/`CACHEMISS`, `4` `HTTP`/`RATELIMITED`/ `PROVIDERUNAVAILABLE`/`OFFLINE`/`AUTHERROR`, `5` `UNSUPPORTED`, `1` everything else @@ -182,4 +187,6 @@ CLI flags > environment variables > config file > defaults - `stocks quote` and `stocks compare` require at least one symbol. - Offline reads warn on stderr when serving cache entries past their TTL, including the cache fetch timestamp. `--quiet` suppresses these warnings. + In JSON mode warnings and info are JSON lines, + `{"level": "warning", "message": "..."}`, so stderr stays parseable. Staleness is not yet represented by a versioned JSON result envelope. diff --git a/docs/INSTALL.md b/docs/INSTALL.md index fee5306..a093c3c 100644 --- a/docs/INSTALL.md +++ b/docs/INSTALL.md @@ -11,7 +11,7 @@ Each release also carries a copy of the script as a release asset, so a versioned URL stays stable: ```bash -curl -fsSL https://github.com/0xrsydn/idx-cli/releases/download/v0.3.2/install.sh | sh +curl -fsSL https://github.com/0xrsydn/idx-cli/releases/download/v0.3.3/install.sh | sh ``` ## What it does @@ -45,7 +45,7 @@ them yourself. Pass flags through the pipe with `sh -s --`: ```bash -curl -fsSL https://raw.githubusercontent.com/0xrsydn/idx-cli/main/install.sh | sh -s -- --version v0.3.2 --dir ~/bin +curl -fsSL https://raw.githubusercontent.com/0xrsydn/idx-cli/main/install.sh | sh -s -- --version v0.3.3 --dir ~/bin ``` ## Supported platforms diff --git a/docs/NPM_DISTRIBUTION.md b/docs/NPM_DISTRIBUTION.md index e04345b..700e020 100644 --- a/docs/NPM_DISTRIBUTION.md +++ b/docs/NPM_DISTRIBUTION.md @@ -38,7 +38,7 @@ npx idx-cli --help ``` The package version and the release tag must match. For example, package -version `0.3.2` downloads assets from the `v0.3.2` GitHub Release. +version `0.3.3` downloads assets from the `v0.3.3` GitHub Release. The postinstall script downloads `SHA256SUMS` from the same release and refuses to install a binary whose checksum does not match. @@ -105,7 +105,7 @@ URLs must stay aligned with `scripts/install.js`. To attach binaries to a tag that already exists, run the workflow manually: ```bash -gh workflow run release.yml -f tag=v0.3.2 +gh workflow run release.yml -f tag=v0.3.3 ``` Pull requests that touch the workflow or the installer run the build jobs diff --git a/docs/OWNERSHIP_SYNC.md b/docs/OWNERSHIP_SYNC.md index 92215e8..8de6b84 100644 --- a/docs/OWNERSHIP_SYNC.md +++ b/docs/OWNERSHIP_SYNC.md @@ -62,7 +62,8 @@ Semantics: (`ownership-snapshot-YYYY-MM-DD-.sqlite`), so consumers must always follow `download_url` and validate it with `sqlite_sha256` and `size_bytes`. - `sqlite_sha256` and `size_bytes` are validated before install. -- `latest_*` and `release_count` are validated against the downloaded SQLite contents before replacement. +- `latest_*` and `release_count` are validated against the downloaded SQLite contents before replacement, including the number of holdings rows actually stored for the latest release. +- A manifest fetched over `http(s)://` must give an `http(s)://` `download_url`; only a local manifest may point at a local file. Local files must be regular files, and manifests are limited to 1 MiB. - `replaces_sqlite_sha256` (optional, omitted when empty) lists earlier builds published for the same `latest_as_of_date` that this snapshot supersedes, for example after a parser fix. Clients up to v0.2.x ignore it. @@ -78,9 +79,11 @@ Without `--force`, sync behaves conservatively: - local DB file is byte-identical to a build in `replaces_sqlite_sha256`: replace (queries never write to a synced file, so this means it was not modified; committed changes still in the `-wal` file are checkpointed before hashing) -- local latest release/date/count matches snapshot: no-op +- local latest release/date/count matches snapshot: no-op (a same-date build + with a different row count replaces the local file only when it lists that + file in `replaces_sqlite_sha256`) - local latest release matches but has fewer releases than snapshot: replace to fill missing history - local DB newer than snapshot: no-op and keep local data - local DB diverges at the same latest date: no-op and require `--force` -Replacement is staged through a temp file and validated before the local DB is swapped in. +Replacement is staged through a temp file and validated before the local DB is swapped in with a single rename, so an interrupted sync leaves either the old database or the new one. diff --git a/install.sh b/install.sh index af4f8b6..91b90de 100755 --- a/install.sh +++ b/install.sh @@ -97,8 +97,11 @@ resolve_latest_version() { fetch "$api_url/repos/$REPO/releases?per_page=30" "$releases" || err "could not list releases; pass --version to skip the lookup" + # One field per line first: on a single-line (compact) response the greedy + # match would otherwise pick the last tag, not the newest. tag="$( - sed -n 's/.*"tag_name"[[:space:]]*:[[:space:]]*"\(v[0-9][0-9]*\.[0-9][0-9]*\.[0-9][0-9]*\)".*/\1/p' "$releases" | + tr ',' '\n' < "$releases" | + sed -n 's/.*"tag_name"[[:space:]]*:[[:space:]]*"\(v[0-9][0-9]*\.[0-9][0-9]*\.[0-9][0-9]*\)".*/\1/p' | head -n 1 )" [ -n "$tag" ] || err "no vX.Y.Z release found for $REPO" diff --git a/npm/idx-mcp/package.json b/npm/idx-mcp/package.json index 0617695..718d686 100644 --- a/npm/idx-mcp/package.json +++ b/npm/idx-mcp/package.json @@ -1,6 +1,6 @@ { "name": "idx-mcp", - "version": "0.3.2", + "version": "0.3.3", "description": "MCP server for Indonesian stock market (IDX) market, financial, and ownership data", "license": "MIT", "repository": { diff --git a/package.json b/package.json index 45e2df7..bc7967c 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "idx-cli", - "version": "0.3.2", + "version": "0.3.3", "description": "CLI tool for Indonesian stock market (IDX) analysis", "license": "MIT", "repository": { diff --git a/scripts/audit-msn-fundamentals.sh b/scripts/audit-msn-fundamentals.sh index 593c8f1..b36825e 100755 --- a/scripts/audit-msn-fundamentals.sh +++ b/scripts/audit-msn-fundamentals.sh @@ -130,7 +130,9 @@ resolve_tickers() { done IFS="$old_ifs" else - mapfile -t TICKER_LIST < <(cut -f1 "$ROOT_DIR/src/api/msn/symbol_ids.tsv") + symbol_map="$ROOT_DIR/crates/idx-core/src/api/msn/symbol_ids.tsv" + [[ -f "$symbol_map" ]] || { echo "symbol map not found: $symbol_map" >&2; exit 1; } + mapfile -t TICKER_LIST < <(cut -f1 "$symbol_map") fi if [[ ${#TICKER_LIST[@]} -eq 0 ]]; then diff --git a/scripts/build-latest-ownership-snapshot.sh b/scripts/build-latest-ownership-snapshot.sh index 0c5b434..c09fd75 100755 --- a/scripts/build-latest-ownership-snapshot.sh +++ b/scripts/build-latest-ownership-snapshot.sh @@ -126,6 +126,16 @@ if ! command -v jq >/dev/null 2>&1; then echo "jq is required for manifest/source metadata processing" >&2 exit 1 fi +# NUL-separated discovery fields need jq 1.7 (--raw-output0) and bash 4.4 +# (mapfile -d); the packaged publisher provides both. +if ! jq --raw-output0 -n '"x"' >/dev/null 2>&1; then + echo "jq 1.7 or newer is required (for --raw-output0); found $(jq --version 2>&1)" >&2 + exit 1 +fi +if ((BASH_VERSINFO[0] < 4 || (BASH_VERSINFO[0] == 4 && BASH_VERSINFO[1] < 4))); then + echo "bash 4.4 or newer is required (for mapfile -d); found ${BASH_VERSION}" >&2 + exit 1 +fi if ! "$IDX_BIN" version >/dev/null 2>&1; then echo "failed to run idx binary: $IDX_BIN" >&2 @@ -163,8 +173,11 @@ DB_PATH="$XDG_DATA_HOME/idx/ownership.db" printf 'Discovering latest supported IDX/KSEI above-1%% report...\n' "$IDX_BIN" -o json ownership discover --family above1 --limit 50 > "$DISCOVERY_JSON" -discovery_payload="$( - jq -r ' +# NUL-separated, so a field containing newlines (a report title) cannot +# shift the fields after it. Written to a file: command substitution drops +# NULs, and a jq failure must still stop the script under set -e. +DISCOVERY_FIELDS="$WORKDIR/discovery-fields" +jq --raw-output0 ' if type != "array" or length == 0 then error("ownership discover returned no reports") else @@ -184,9 +197,8 @@ discovery_payload="$( ] | .[] end - ' "$DISCOVERY_JSON" -)" -mapfile -t discovery_fields <<< "$discovery_payload" + ' "$DISCOVERY_JSON" > "$DISCOVERY_FIELDS" +mapfile -d '' -t discovery_fields < "$DISCOVERY_FIELDS" DISCOVERED_FAMILY="${discovery_fields[0]:-}" DISCOVERED_LISTING_PAGE_URL="${discovery_fields[1]:-}" @@ -308,6 +320,14 @@ jq \ original_filename: ($original_filename | if . == "" then null else . end) } ' "$MANIFEST_PATH" > "$TMP_MANIFEST_PATH" +# Clients reject a manifest missing any of these, so a bad one would block +# every `idx ownership sync`; refuse to produce it. +if ! jq -e '[.source.pdf_url, .source.title, .source.publish_date, .snapshot.version] + | all(type == "string" and (gsub("^\\s+|\\s+$"; "") | length > 0))' \ + "$TMP_MANIFEST_PATH" > /dev/null; then + echo "built manifest is missing source.pdf_url, source.title, source.publish_date, or snapshot.version" >&2 + exit 1 +fi mv "$TMP_MANIFEST_PATH" "$MANIFEST_PATH" printf 'Prepared release-ready snapshot from %s (%s)\n' \ diff --git a/scripts/build-ownership-snapshot.sh b/scripts/build-ownership-snapshot.sh index 997b70b..d626209 100755 --- a/scripts/build-ownership-snapshot.sh +++ b/scripts/build-ownership-snapshot.sh @@ -79,9 +79,9 @@ fi mkdir -p "$OUTPUT_DIR" -LATEST_AS_OF="$(sqlite3 "$DB_PATH" "SELECT as_of_date FROM ownership_releases ORDER BY as_of_date DESC, imported_at DESC LIMIT 1;")" -LATEST_RELEASE_SHA="$(sqlite3 "$DB_PATH" "SELECT sha256 FROM ownership_releases ORDER BY as_of_date DESC, imported_at DESC LIMIT 1;")" -LATEST_ROW_COUNT="$(sqlite3 "$DB_PATH" "SELECT row_count FROM ownership_releases ORDER BY as_of_date DESC, imported_at DESC LIMIT 1;")" +LATEST_AS_OF="$(sqlite3 "$DB_PATH" "SELECT as_of_date FROM ownership_releases ORDER BY as_of_date DESC, imported_at DESC, rowid DESC LIMIT 1;")" +LATEST_RELEASE_SHA="$(sqlite3 "$DB_PATH" "SELECT sha256 FROM ownership_releases ORDER BY as_of_date DESC, imported_at DESC, rowid DESC LIMIT 1;")" +LATEST_ROW_COUNT="$(sqlite3 "$DB_PATH" "SELECT row_count FROM ownership_releases ORDER BY as_of_date DESC, imported_at DESC, rowid DESC LIMIT 1;")" RELEASE_COUNT="$(sqlite3 "$DB_PATH" "SELECT COUNT(*) FROM ownership_releases;")" TICKER_COUNT="$(sqlite3 "$DB_PATH" "SELECT COUNT(*) FROM tickers;")" @@ -94,6 +94,15 @@ MANIFEST_PATH="$OUTPUT_DIR/ownership-snapshot-manifest.json" TMP_ARTIFACT_PATH="$OUTPUT_DIR/.ownership-snapshot-${LATEST_AS_OF}.sqlite.tmp.$$" # Remove a partial copy if the script fails before the atomic rename below. trap 'rm -f "$TMP_ARTIFACT_PATH"' EXIT +# Committed data can still sit in the -wal file while another connection has +# the database open; checkpoint it into the main file first (a no-op for a +# closed database, so its bytes and hash are unchanged), and refuse to copy +# when that cannot complete. +checkpoint="$(sqlite3 "$DB_PATH" "PRAGMA wal_checkpoint(TRUNCATE);")" +if [[ "${checkpoint%%|*}" != "0" ]]; then + echo "ownership database is busy; could not checkpoint its WAL before copying: $DB_PATH" >&2 + exit 1 +fi cp "$DB_PATH" "$TMP_ARTIFACT_PATH" # Immutable, content-addressed name. The publisher relies on this to upload and diff --git a/scripts/install-sh-test.sh b/scripts/install-sh-test.sh index 4bcff0e..769e91c 100755 --- a/scripts/install-sh-test.sh +++ b/scripts/install-sh-test.sh @@ -191,6 +191,17 @@ else fail "expands quoted ~ and tolerates trailing slashes"; cat "${work}/out9" >&2 fi +# 10. a compact single-line releases response still resolves the newest tag. +printf '%s' '[{"tag_name":"ownership-snapshot-current"},{"tag_name":"v0.2.4","name":"v0.2.4"},{"tag_name":"v0.2.3"}]' \ + >"${api_dir}/releases" +dir="${work}/compact" +if run_installer "$test_shell" "$installer" --dir "$dir" >"${work}/out10" 2>&1 && + [[ "$("${dir}/idx")" == "idx fake v0.2.4" ]]; then + pass "resolves the newest tag from a compact releases response" +else + fail "resolves the newest tag from a compact releases response"; cat "${work}/out10" >&2 +fi + if ((failures > 0)); then echo "install.sh tests: ${failures} failure(s) with ${test_shell}" >&2 exit 1 diff --git a/web/src/content/docs/cli/reference.md b/web/src/content/docs/cli/reference.md index 2236c01..095347b 100644 --- a/web/src/content/docs/cli/reference.md +++ b/web/src/content/docs/cli/reference.md @@ -463,7 +463,9 @@ Examples: ## `idx config` ```text -Manage configuration +Manage configuration in ~/.config/idx/config.toml (or $XDG_CONFIG_HOME/idx/config.toml). + +Results print as plain text, even when piped, so `$(idx config path)` works in scripts. Ask for JSON with `-o json`, `IDX_OUTPUT=json`, or `general.output = "json"`. Usage: idx config [OPTIONS] @@ -475,13 +477,39 @@ Commands: help Print this message or the help of the given subcommand(s) Options: - -o, --output Output format [default: auto = table on a terminal, json when piped] [possible values: auto, table, json, jsonl] - --no-color Disable colored output - -q, --quiet Suppress non-essential output - -v, --verbose... Increase verbosity (-v, -vv) - --offline Run without network requests - --no-cache Bypass local cache - -h, --help Print help + -o, --output + Output format [default: auto = table on a terminal, json when piped] + + [possible values: auto, table, json, jsonl] + + --no-color + Disable colored output + + -q, --quiet + Suppress non-essential output + + -v, --verbose... + Increase verbosity (-v, -vv) + + --offline + Run without network requests + + --no-cache + Bypass local cache + + -h, --help + Print help (see a summary with '-h') + +Keys: + general.provider, general.history_provider, general.exchange, general.output, + general.color, cache.quote_ttl, cache.fundamental_ttl, ownership.db_path, + ownership.snapshot_manifest + +Examples: + idx config init + idx config set general.output json + idx config get cache.quote_ttl + idx config path ``` ### `idx config init` @@ -509,7 +537,7 @@ Get a configuration value by key Usage: idx config get [OPTIONS] Arguments: - + Key such as general.provider (see `idx config --help`) Options: -o, --output Output format [default: auto = table on a terminal, json when piped] [possible values: auto, table, json, jsonl] @@ -529,7 +557,7 @@ Set a configuration value by key Usage: idx config set [OPTIONS] Arguments: - + Key such as general.provider (see `idx config --help`) Options: @@ -633,7 +661,7 @@ Commands: sync Install or refresh a maintained ownership SQLite snapshot ticker Show all holders for a ticker (KSEI + Bing combined) entity Show all holdings for an entity across tickers - search Search entities by name + search Search entities by name or KSEI alias cross-holders Rank entities by cross-ownership breadth concentration Rank tickers by ownership concentration flow Show Bing institutional flow for a ticker @@ -833,7 +861,7 @@ Show all holdings for an entity across tickers Usage: idx ownership entity [OPTIONS] Arguments: - + Entity name or part of one, case-insensitive; the shortest match is shown Options: -o, --output Output format [default: auto = table on a terminal, json when piped] [possible values: auto, table, json, jsonl] @@ -843,27 +871,55 @@ Options: --offline Run without network requests --no-cache Bypass local cache -h, --help Print help + +Examples: + idx ownership entity "ASTRA INTERNATIONAL" + idx ownership search astra # list candidates first ``` ### `idx ownership search` ```text -Search entities by name +Search entities by name or KSEI alias. + +Matches canonical names and the raw names KSEI prints (e.g. `PT ASTRA INTERNATIONAL TBK`), case-insensitively. Ranked: exact name, exact alias, prefix, then substring. Usage: idx ownership search [OPTIONS] Arguments: + Name, KSEI alias, or part of one Options: - --limit [default: 20] - -o, --output Output format [default: auto = table on a terminal, json when piped] [possible values: auto, table, json, jsonl] - --no-color Disable colored output - -q, --quiet Suppress non-essential output - -v, --verbose... Increase verbosity (-v, -vv) - --offline Run without network requests - --no-cache Bypass local cache - -h, --help Print help + --limit + [default: 20] + + -o, --output + Output format [default: auto = table on a terminal, json when piped] + + [possible values: auto, table, json, jsonl] + + --no-color + Disable colored output + + -q, --quiet + Suppress non-essential output + + -v, --verbose... + Increase verbosity (-v, -vv) + + --offline + Run without network requests + + --no-cache + Bypass local cache + + -h, --help + Print help (see a summary with '-h') + +Examples: + idx ownership search astra + idx ownership search "PT ASTRA INTERNATIONAL TBK" --limit 5 ``` ### `idx ownership cross-holders` @@ -927,20 +983,45 @@ Options: ### `idx ownership changes` ```text -Diff two KSEI releases +Diff two KSEI releases: new, exited, increased, and decreased holders. + +Both dates must be imported releases (`idx ownership releases` lists them), with --from before --to. A holder's accounts in a ticker are summed, so an investor with several accounts compares by its total stake. Usage: idx ownership changes [OPTIONS] --from --to Options: --from - -o, --output Output format [default: auto = table on a terminal, json when piped] [possible values: auto, table, json, jsonl] - --no-color Disable colored output + Earlier release date, YYYY-MM-DD + + -o, --output + Output format [default: auto = table on a terminal, json when piped] + + [possible values: auto, table, json, jsonl] + + --no-color + Disable colored output + --to - -q, --quiet Suppress non-essential output - -v, --verbose... Increase verbosity (-v, -vv) - --offline Run without network requests - --no-cache Bypass local cache - -h, --help Print help + Later release date, YYYY-MM-DD + + -q, --quiet + Suppress non-essential output + + -v, --verbose... + Increase verbosity (-v, -vv) + + --offline + Run without network requests + + --no-cache + Bypass local cache + + -h, --help + Print help (see a summary with '-h') + +Examples: + idx ownership releases + idx ownership changes --from 2026-08-31 --to 2026-09-30 ``` ### `idx ownership graph` diff --git a/web/src/content/docs/start/config.md b/web/src/content/docs/start/config.md index 2595074..4304b4d 100644 --- a/web/src/content/docs/start/config.md +++ b/web/src/content/docs/start/config.md @@ -22,6 +22,10 @@ idx config get cache.quote_ttl idx config path ``` +`idx config` prints plain text, even when piped, so `$(idx config path)` works +in scripts. Ask for JSON with `-o json`, `IDX_OUTPUT=json`, or +`output = "json"`. + ```toml [general] provider = "msn" # msn | yahoo diff --git a/web/src/content/docs/start/install.mdx b/web/src/content/docs/start/install.mdx index 6ea07a7..e8da2b4 100644 --- a/web/src/content/docs/start/install.mdx +++ b/web/src/content/docs/start/install.mdx @@ -40,7 +40,7 @@ against the release `SHA256SUMS`, and installs it into `~/.local/bin` without sudo. Pin a version or change the directory: ```bash -curl -fsSL https://raw.githubusercontent.com/0xrsydn/idx-cli/main/install.sh | sh -s -- --version v0.3.2 --dir ~/bin +curl -fsSL https://raw.githubusercontent.com/0xrsydn/idx-cli/main/install.sh | sh -s -- --version v0.3.3 --dir ~/bin ```