From 0c7320a343c4d7cbecce73e91870190759e5a0f3 Mon Sep 17 00:00:00 2001 From: 0xrsydn Date: Wed, 4 Nov 2026 17:39:10 +0700 Subject: [PATCH] fix(movements): read a query's pages as one answer and keep a partial one - Each page was cached on its own, so a multi-page answer could join pages read at different times from a feed that gains rows at the top, repeating some rows and missing others. A query's pages are now read together and cached as one entry. - A failed later page threw away the pages already read. They are now returned with incomplete saying why, and not cached. - The library did not cap pages at MAX_PAGES itself; it clamps now. - The hint said to raise --pages even at 10; at 10 it says to narrow the dates. -o jsonl, which streams rows only, gives the hint on stderr. - A duplicate match arm in the CLI was dead; it is gone. Claude-Session: https://claude.ai/code/session_01RqPPVeVDrnLoP7NUZmLRxY --- crates/idx-cli/src/cli/ownership.rs | 4 +- crates/idx-cli/src/output/mod.rs | 13 ++- crates/idx-cli/src/output/table.rs | 22 ++++- crates/idx-cli/tests/cli.rs | 27 ++++++ crates/idx-core/src/api/mock.rs | 4 + crates/idx-core/src/movements/mod.rs | 127 ++++++++++++++++--------- crates/idx-core/src/movements/types.rs | 5 + crates/idx-mcp/src/server.rs | 3 +- crates/idx-mcp/tests/golden/tools.json | 2 +- web/src/content/docs/cli/ownership.md | 4 +- web/src/content/docs/mcp/tools.md | 2 +- 11 files changed, 159 insertions(+), 54 deletions(-) diff --git a/crates/idx-cli/src/cli/ownership.rs b/crates/idx-cli/src/cli/ownership.rs index cd2850f..63cef02 100644 --- a/crates/idx-cli/src/cli/ownership.rs +++ b/crates/idx-cli/src/cli/ownership.rs @@ -252,9 +252,6 @@ pub fn handle( offline: bool, no_cache: bool, ) -> Result<(), IdxError> { - if let OwnershipCommand::Movements(args) = cmd { - return handle_movements(args, config, CachePolicy::from_flags(offline, no_cache)?); - } let investor = Investor::new(&config.core); let inv = &investor; match cmd { @@ -752,6 +749,7 @@ fn handle_movements( crate::output::render_shareholder_movements( symbol.as_deref(), &list.data, + query.pages, &config.output, config.no_color, ) diff --git a/crates/idx-cli/src/output/mod.rs b/crates/idx-cli/src/output/mod.rs index 633acb2..62506f4 100644 --- a/crates/idx-cli/src/output/mod.rs +++ b/crates/idx-cli/src/output/mod.rs @@ -129,16 +129,25 @@ pub fn render_corporate_calendar( /// JSON is `{movements, more}` (`more`: rows remain past the pages read); /// JSONL is one movement per line. +/// `pages` is how many pages the query asked for, for the hint when more +/// rows exist. JSONL streams the rows only, so the hint goes to stderr. pub fn render_shareholder_movements( symbol: Option<&str>, list: &MovementList, + pages: u32, format: &OutputFormat, no_color: bool, ) -> Result<(), IdxError> { match format { - OutputFormat::Table => table::print_shareholder_movements(symbol, list, no_color), + OutputFormat::Table => table::print_shareholder_movements(symbol, list, pages, no_color), OutputFormat::Json => json::print_json(list), - OutputFormat::Jsonl => json::print_json(&list.movements), + OutputFormat::Jsonl => { + json::print_json(&list.movements)?; + if list.more { + idx_core::runtime::info(table::more_movements_hint(pages)); + } + Ok(()) + } } } diff --git a/crates/idx-cli/src/output/table.rs b/crates/idx-cli/src/output/table.rs index d3afec8..216fce0 100644 --- a/crates/idx-cli/src/output/table.rs +++ b/crates/idx-cli/src/output/table.rs @@ -1920,9 +1920,22 @@ const MOVEMENT_NOTE: &str = "IDX rows are insider disclosures (directors, commis other insiders); KSEI rows are changes of holders at 5% and above. ~ marks Stockbit's estimated \ price. Rows appear days after the change."; +/// What to do when rows remain past the pages read. +pub fn more_movements_hint(pages: u32) -> String { + if pages >= idx_core::movements::types::MAX_PAGES { + "More rows exist: narrow --from/--to (10 pages is the most one query reads).".to_string() + } else { + format!( + "More rows exist: raise --pages (up to {}) or narrow --from/--to.", + idx_core::movements::types::MAX_PAGES + ) + } +} + pub fn print_shareholder_movements( symbol: Option<&str>, list: &MovementList, + pages: u32, no_color: bool, ) -> Result<(), IdxError> { let title = match symbol { @@ -1987,8 +2000,13 @@ pub fn print_shareholder_movements( println!("{table}"); println!("{MOVEMENT_NOTE}"); } - if list.more { - println!("More rows exist: raise --pages (up to 10) or narrow --from/--to."); + if let Some(reason) = &list.incomplete { + println!( + "Reading stopped early ({}); these are the rows read before it.", + terminal_safe(reason) + ); + } else if list.more { + println!("{}", more_movements_hint(pages)); } println!("{STOCKBIT_SESSION_NOTE}"); Ok(()) diff --git a/crates/idx-cli/tests/cli.rs b/crates/idx-cli/tests/cli.rs index 490e254..b1e5cbe 100644 --- a/crates/idx-cli/tests/cli.rs +++ b/crates/idx-cli/tests/cli.rs @@ -2239,6 +2239,33 @@ fn auto_history_falls_back_to_yahoo_when_the_session_is_refused() { let _ = fs::remove_dir_all(root); } +/// A page after the first that fails keeps the rows read before it, says +/// why, and is not cached; a query's pages are cached as one answer. +#[test] +fn movements_keep_the_pages_read_when_a_later_one_fails() { + let root = test_env_dir("movements-partial"); + let run = |fail_page: Option<&str>| { + let mut cmd = mock_bin_with_root(&root); + cmd.env("IDX_STOCKBIT_REFRESH_TOKEN", FAKE_STOCKBIT_TOKEN) + .args(["-o", "json", "ownership", "movements", "--pages", "2"]); + if let Some(page) = fail_page { + cmd.env("IDX_MOCK_MOVEMENTS_FAIL_PAGE", page); + } + run_success_json(&mut cmd) + }; + let partial = run(Some("2")); + assert_eq!(partial["movements"].as_array().unwrap().len(), 8); + assert_eq!(partial["more"], true); + assert!(partial["incomplete"].is_string(), "{partial}"); + // Not cached: the next run reads both pages. + let full = run(None); + assert_eq!(full["movements"].as_array().unwrap().len(), 14); + assert!(full.get("incomplete").is_none()); + // Cached as one answer: a failing page 2 now changes nothing. + assert_eq!(run(Some("2")), full); + let _ = fs::remove_dir_all(root); +} + /// A test session, signed by nobody, that expires in 2100. const FAKE_STOCKBIT_TOKEN: &str = "eyJhbGciOiJub25lIn0.eyJleHAiOiA0MTAyNDQ0ODAwLCAibm90ZSI6ICJ0ZXN0IHRva2VuLCBzaWduZWQgYnkgbm9ib2R5In0.c2ln"; diff --git a/crates/idx-core/src/api/mock.rs b/crates/idx-core/src/api/mock.rs index 3e13721..9dbf60a 100644 --- a/crates/idx-core/src/api/mock.rs +++ b/crates/idx-core/src/api/mock.rs @@ -271,6 +271,10 @@ impl ShareholderMovementProvider for MockStockbitProvider { page: u32, ) -> Result { let subject = query.symbol.as_deref().unwrap_or("shareholder movements"); + // `IDX_MOCK_MOVEMENTS_FAIL_PAGE=N` rate-limits page N alone. + if std::env::var("IDX_MOCK_MOVEMENTS_FAIL_PAGE").ok() == Some(page.to_string()) { + return Err(IdxError::RateLimited); + } let mut name = "majorholder".to_string(); if let Some(symbol) = &query.symbol { name.push('_'); diff --git a/crates/idx-core/src/movements/mod.rs b/crates/idx-core/src/movements/mod.rs index f35387e..293d0a7 100644 --- a/crates/idx-core/src/movements/mod.rs +++ b/crates/idx-core/src/movements/mod.rs @@ -1,7 +1,9 @@ //! Shareholder movements from the user's Stockbit session: reported changes //! in insiders' and major holders' stakes (IDX insider disclosures and KSEI's //! 5% holder changes), one stock's or the market's, newest first. Each page -//! is 50 rows and one request (paced 1 s apart), cached for an hour. +//! is 50 rows and one request (paced 1 s apart). A query's pages are read +//! together and cached together for an hour, so they come from one moment +//! of a feed that gains rows at the top. //! //! These are the reports as filed. Whether they predict anything is not //! claimed here: share plans read as a cluster of buys at one price, and a @@ -9,12 +11,15 @@ pub mod types; +use std::cell::RefCell; + use chrono::Utc; -use crate::api::{shareholder_movement_provider, stockbit}; +use crate::api::{ShareholderMovementProvider, shareholder_movement_provider, stockbit}; use crate::client::{CachePolicy, CacheSpec, Fetched, Idx, cached_fetch}; use crate::error::IdxError; -use types::{MovementList, MovementQuery}; +use crate::runtime; +use types::{MAX_PAGES, MovementList, MovementQuery}; /// The feed gains rows through the day; an hour keeps a session's repeated /// queries cheap without hiding a day's filings for long. @@ -26,15 +31,19 @@ pub struct Movements<'a> { } impl Movements<'_> { - /// Up to `query.pages` pages of the feed, newest first; `more` says - /// whether rows remain past them. The result is as old as its oldest page - /// and stale if any page is. + /// Up to `query.pages` pages of the feed (at most `MAX_PAGES`), newest + /// first; `more` says whether rows remain past them. + /// + /// When a page after the first fails and no cached answer is at hand, + /// the rows read before it are returned with `incomplete` saying why, + /// and are not cached. pub fn list( &self, query: &MovementQuery, policy: CachePolicy, ) -> Result, IdxError> { let mut query = query.clone(); + query.pages = query.pages.clamp(1, MAX_PAGES); if let Some(symbol) = &query.symbol { let (resolved, _) = self.idx.idx_code(symbol, "shareholder movements")?; query.symbol = Some(resolved); @@ -44,50 +53,80 @@ impl Movements<'_> { .symbol .clone() .unwrap_or_else(|| "shareholder movements".to_string()); - let mut list = Fetched { - data: MovementList::default(), - source: stockbit::SOURCE, - fetched_at: Utc::now(), - stale: false, - }; - for page in 1..=query.pages.max(1) { - let key = cache_key(&query, page); - let part = cached_fetch( - &self.idx.cache, - CacheSpec { - bucket: BUCKET, - key: &key, - ttl_secs: TTL_SECS, - subject: &subject, + let key = cache_key(&query); + let partial = RefCell::new(None); + let result = cached_fetch( + &self.idx.cache, + CacheSpec { + bucket: BUCKET, + key: &key, + ttl_secs: TTL_SECS, + subject: &subject, + source: stockbit::SOURCE, + }, + policy, + || read_pages(provider.as_ref(), &query, &partial), + Ok, + ); + match (result, partial.into_inner()) { + (Err(err), Some(mut list)) => { + runtime::warn(format!( + "{err}; showing the {} rows read before it", + list.movements.len() + )); + list.incomplete = Some(err.to_string()); + Ok(Fetched { + data: list, source: stockbit::SOURCE, - }, - policy, - || provider.shareholder_movements(&query, page), - Ok, - )?; - list.fetched_at = list.fetched_at.min(part.fetched_at); - list.stale |= part.stale; - list.data.movements.extend(part.data.movements); - list.data.more = part.data.more; - if !part.data.more { - break; + fetched_at: Utc::now(), + stale: false, + }) } + (result, _) => result, } - Ok(list) } } -/// Every filter and the page, so no two queries share an entry. -fn cache_key(query: &MovementQuery, page: u32) -> String { +/// Reads the query's pages in turn. A failure after the first page leaves +/// the rows read so far in `partial` and fails, so they are not cached. +fn read_pages( + provider: &dyn ShareholderMovementProvider, + query: &MovementQuery, + partial: &RefCell>, +) -> Result { + let mut list = MovementList::default(); + for page in 1..=query.pages { + match provider.shareholder_movements(query, page) { + Ok(part) => { + list.movements.extend(part.movements); + list.more = part.more; + if !part.more { + break; + } + } + Err(err) if page > 1 => { + list.more = true; + *partial.borrow_mut() = Some(list); + return Err(err); + } + Err(err) => return Err(err), + } + } + Ok(list) +} + +/// Every filter and the page count, so no two queries share an entry. +fn cache_key(query: &MovementQuery) -> String { let opt = |value: Option| value.unwrap_or_else(|| "-".to_string()); format!( - "{}:{}:{}:{}:{}:{}:p{page}", + "{}:{}:{}:{}:{}:{}:{}p", query.symbol.as_deref().unwrap_or("market"), opt(query.from.map(|d| d.to_string())), opt(query.to.map(|d| d.to_string())), query.holder_id.as_deref().unwrap_or("-"), query.action.map(|a| a.as_str()).unwrap_or("-"), query.source.map(|s| s.as_str()).unwrap_or("-"), + query.pages, ) } @@ -99,25 +138,27 @@ mod tests { #[test] fn cache_keys_differ_by_every_filter() { let base = MovementQuery::from_args(None, None, None, None, 1).unwrap(); - let mut keys = vec![cache_key(&base, 1), cache_key(&base, 2)]; + let mut two = base.clone(); + two.pages = 2; + let mut keys = vec![cache_key(&base), cache_key(&two)]; let mut q = base.clone(); q.symbol = Some("BBCA.JK".to_string()); - keys.push(cache_key(&q, 1)); + keys.push(cache_key(&q)); let mut q = base.clone(); q.from = chrono::NaiveDate::from_ymd_opt(2026, 10, 1); - keys.push(cache_key(&q, 1)); + keys.push(cache_key(&q)); let mut q = base.clone(); q.to = chrono::NaiveDate::from_ymd_opt(2026, 10, 1); - keys.push(cache_key(&q, 1)); + keys.push(cache_key(&q)); let mut q = base.clone(); q.holder_id = Some("14074".to_string()); - keys.push(cache_key(&q, 1)); + keys.push(cache_key(&q)); let mut q = base.clone(); q.action = Some(ActionFilter::Sell); - keys.push(cache_key(&q, 1)); + keys.push(cache_key(&q)); let mut q = base; q.source = Some(MovementSource::Ksei); - keys.push(cache_key(&q, 1)); + keys.push(cache_key(&q)); let unique: std::collections::HashSet<_> = keys.iter().collect(); assert_eq!(unique.len(), keys.len(), "{keys:?}"); } diff --git a/crates/idx-core/src/movements/types.rs b/crates/idx-core/src/movements/types.rs index 1d031ce..6c974b4 100644 --- a/crates/idx-core/src/movements/types.rs +++ b/crates/idx-core/src/movements/types.rs @@ -288,6 +288,11 @@ pub struct MovementList { /// More rows exist past the pages read (raise `--pages`, or narrow the /// dates). pub more: bool, + /// Why reading stopped before the pages asked for: a later page failed, + /// and `movements` holds the rows read before it. Absent when every page + /// asked for was read. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub incomplete: Option, } #[cfg(test)] diff --git a/crates/idx-mcp/src/server.rs b/crates/idx-mcp/src/server.rs index d78708f..7e74026 100644 --- a/crates/idx-mcp/src/server.rs +++ b/crates/idx-mcp/src/server.rs @@ -411,7 +411,8 @@ commissioners and other insiders, any size, with the trade price) and KSEI chang at 5% and above (price estimated: `price_estimated`). One stock's, or the market's without \ `symbol`. Shares are exact and signed (`shares_change` < 0 is a sale); percentages are basis \ points as Stockbit rounds them, so a small insider can read 0. `more` is true when rows remain \ -past `pages`. These are filings, not signals: many insiders buying at one price on one day is \ +past `pages`; `incomplete`, when present, says a later page failed and the rows are those read \ +before it (retry later for the rest). These are filings, not signals: many insiders buying at one price on one day is \ usually a share plan, and rows appear days after the change. AUTHERROR means there is no \ session: call stockbit_status, then stockbit_login." )] diff --git a/crates/idx-mcp/tests/golden/tools.json b/crates/idx-mcp/tests/golden/tools.json index aec2f76..1342bfa 100644 --- a/crates/idx-mcp/tests/golden/tools.json +++ b/crates/idx-mcp/tests/golden/tools.json @@ -556,7 +556,7 @@ "name": "get_quotes" }, { - "description": "Reported changes in insiders' and major holders' stakes in IDX stocks, from the user's own Stockbit session, newest first: IDX insider disclosures (directors, commissioners and other insiders, any size, with the trade price) and KSEI changes of holders at 5% and above (price estimated: `price_estimated`). One stock's, or the market's without `symbol`. Shares are exact and signed (`shares_change` < 0 is a sale); percentages are basis points as Stockbit rounds them, so a small insider can read 0. `more` is true when rows remain past `pages`. These are filings, not signals: many insiders buying at one price on one day is usually a share plan, and rows appear days after the change. AUTHERROR means there is no session: call stockbit_status, then stockbit_login.", + "description": "Reported changes in insiders' and major holders' stakes in IDX stocks, from the user's own Stockbit session, newest first: IDX insider disclosures (directors, commissioners and other insiders, any size, with the trade price) and KSEI changes of holders at 5% and above (price estimated: `price_estimated`). One stock's, or the market's without `symbol`. Shares are exact and signed (`shares_change` < 0 is a sale); percentages are basis points as Stockbit rounds them, so a small insider can read 0. `more` is true when rows remain past `pages`; `incomplete`, when present, says a later page failed and the rows are those read before it (retry later for the rest). These are filings, not signals: many insiders buying at one price on one day is usually a share plan, and rows appear days after the change. AUTHERROR means there is no session: call stockbit_status, then stockbit_login.", "inputSchema": { "$schema": "https://json-schema.org/draft/2020-12/schema", "properties": { diff --git a/web/src/content/docs/cli/ownership.md b/web/src/content/docs/cli/ownership.md index f22f76c..7258b2e 100644 --- a/web/src/content/docs/cli/ownership.md +++ b/web/src/content/docs/cli/ownership.md @@ -60,7 +60,9 @@ small holding reads `<0.01%`. These are filings, not signals. Many insiders buying at one price on one day is usually a share plan, transfers are not trades, and a row appears days after the change. Each page is 50 rows and one request to Stockbit (`--pages` -up to 10); answers are cached for an hour. +up to 10); a query's pages are read and cached together, for an hour. JSON's +`more` says rows remain past them. If a later page fails, you get the rows +read before it, with `incomplete` saying why, and nothing is cached. ## Import reports yourself diff --git a/web/src/content/docs/mcp/tools.md b/web/src/content/docs/mcp/tools.md index ecca1a1..21118de 100644 --- a/web/src/content/docs/mcp/tools.md +++ b/web/src/content/docs/mcp/tools.md @@ -152,7 +152,7 @@ Latest quotes (price, change, volume, 52-week range, market cap) for one or more ## `get_shareholder_movements` -Reported changes in insiders' and major holders' stakes in IDX stocks, from the user's own Stockbit session, newest first: IDX insider disclosures (directors, commissioners and other insiders, any size, with the trade price) and KSEI changes of holders at 5% and above (price estimated: `price_estimated`). One stock's, or the market's without `symbol`. Shares are exact and signed (`shares_change` < 0 is a sale); percentages are basis points as Stockbit rounds them, so a small insider can read 0. `more` is true when rows remain past `pages`. These are filings, not signals: many insiders buying at one price on one day is usually a share plan, and rows appear days after the change. AUTHERROR means there is no session: call stockbit_status, then stockbit_login. +Reported changes in insiders' and major holders' stakes in IDX stocks, from the user's own Stockbit session, newest first: IDX insider disclosures (directors, commissioners and other insiders, any size, with the trade price) and KSEI changes of holders at 5% and above (price estimated: `price_estimated`). One stock's, or the market's without `symbol`. Shares are exact and signed (`shares_change` < 0 is a sale); percentages are basis points as Stockbit rounds them, so a small insider can read 0. `more` is true when rows remain past `pages`; `incomplete`, when present, says a later page failed and the rows are those read before it (retry later for the rest). These are filings, not signals: many insiders buying at one price on one day is usually a share plan, and rows appear days after the change. AUTHERROR means there is no session: call stockbit_status, then stockbit_login. | Input | Type | Required | Description | | --- | --- | --- | --- |