From 8fc295260a551958eb2a2e5da12c6f1e4bb52fc5 Mon Sep 17 00:00:00 2001 From: hude Date: Wed, 13 May 2026 07:25:14 +0900 Subject: [PATCH] Fix repo review findings --- rust/commands/ask_ai.rs | 15 ++- rust/commands/balance.rs | 27 +++++- rust/insert_service.rs | 38 +++++++- rust/python.rs | 3 +- tui/crates/tui-kit-host/src/runtime_loop.rs | 21 +++- .../tui-kit-host/src/runtime_loop_tests.rs | 97 ++++++++++--------- .../tui-kit-render/src/ui/app/screens/wiki.rs | 1 - tui/crates/tui-kit-render/src/ui/content.rs | 6 +- 8 files changed, 145 insertions(+), 63 deletions(-) diff --git a/rust/commands/ask_ai.rs b/rust/commands/ask_ai.rs index ad2ab62..c18a46d 100644 --- a/rust/commands/ask_ai.rs +++ b/rust/commands/ask_ai.rs @@ -77,7 +77,7 @@ pub async fn ask_ai_flow( prompt, response: llm_response, context_count: results.len(), - top_k_used: limit.min(results.len()), + top_k_used: effective_top_k_used(limit, results.len()), }) } @@ -169,6 +169,10 @@ fn build_prompt( ask_ai_prompt(&clipped_query, &docs, language) } +fn effective_top_k_used(limit: usize, result_count: usize) -> usize { + limit.min(result_count).min(MAX_RESULTS) +} + fn clip(s: &str, max: usize) -> String { let clipped: String = s.chars().take(max).collect(); if s.chars().count() > max { @@ -273,7 +277,7 @@ Summarize the main points concisely, taking into account their relevance to the #[cfg(test)] mod tests { - use super::{SearchHit, SearchResult, ask_ai_prompt, extract_answer}; + use super::{SearchHit, SearchResult, ask_ai_prompt, effective_top_k_used, extract_answer}; #[test] fn extract_answer_is_unicode_safe() { @@ -310,4 +314,11 @@ mod tests { assert!(prompt.contains("Answer in 日本語 (Japanese) in tag.")); } + + #[test] + fn effective_top_k_used_matches_prompt_result_cap() { + assert_eq!(effective_top_k_used(100, 10), 5); + assert_eq!(effective_top_k_used(3, 10), 3); + assert_eq!(effective_top_k_used(3, 2), 2); + } } diff --git a/rust/commands/balance.rs b/rust/commands/balance.rs index 4144700..d21b18f 100644 --- a/rust/commands/balance.rs +++ b/rust/commands/balance.rs @@ -1,4 +1,5 @@ use anyhow::{Result, anyhow}; +use kinic_core::amount::format_e8s_to_kinic_string_u128; use tracing::info; use crate::{cli::BalanceArgs, ledger::fetch_balance}; @@ -12,7 +13,7 @@ pub async fn handle(_args: BalanceArgs, ctx: &CommandContext) -> Result<()> { .map_err(|e| anyhow!("Failed to derive principal for current identity: {e}"))?; let balance = fetch_balance(&agent).await?; - let kinic = balance as f64 / 100_000_000f64; + let kinic = format_e8s_to_kinic_string_u128(balance); info!( %principal, @@ -20,7 +21,29 @@ pub async fn handle(_args: BalanceArgs, ctx: &CommandContext) -> Result<()> { balance_kinic = kinic, "fetched token balance" ); - println!("Balance for {principal}: {kinic:.7} KINIC (= {balance} e8s)"); + println!("{}", balance_line(&principal.to_string(), balance)); Ok(()) } + +fn balance_line(principal: &str, balance: u128) -> String { + format!( + "Balance for {}: {} KINIC (= {} e8s)", + principal, + format_e8s_to_kinic_string_u128(balance), + balance + ) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn balance_line_preserves_all_eight_fraction_digits() { + assert_eq!( + balance_line("aaaaa-aa", 42), + "Balance for aaaaa-aa: 0.00000042 KINIC (= 42 e8s)" + ); + } +} diff --git a/rust/insert_service.rs b/rust/insert_service.rs index e3116d8..d87921a 100644 --- a/rust/insert_service.rs +++ b/rust/insert_service.rs @@ -187,10 +187,13 @@ fn ensure_prepared_items_match_memory( items: &[PreparedInsertItem], expected_dim: u64, ) -> Result<()> { - let Some(first) = items.first() else { + if items.is_empty() { bail!("Insert content did not produce any chunks."); - }; - ensure_vector_dim_matches(memory_id, first.embedding.len(), expected_dim) + } + for item in items { + ensure_vector_dim_matches(memory_id, item.embedding.len(), expected_dim)?; + } + Ok(()) } pub fn validate_insert_request_for_submit(request: &InsertRequest) -> Result<()> { @@ -576,6 +579,27 @@ mod tests { assert!(error.to_string().contains("Embedding dimension mismatch")); } + #[test] + fn prepared_items_reject_later_dimension_mismatch() { + let error = ensure_prepared_items_match_memory( + "aaaaa-aa", + &[ + PreparedInsertItem { + embedding: vec![0.1, 0.2, 0.3], + payload: "{}".to_string(), + }, + PreparedInsertItem { + embedding: vec![0.1, 0.2], + payload: "{}".to_string(), + }, + ], + 3, + ) + .unwrap_err(); + + assert!(error.to_string().contains("Embedding dimension mismatch")); + } + #[test] fn raw_insert_request_fails_fast_on_dimension_mismatch() { let request = ValidatedInsertRequest::Raw { @@ -874,8 +898,12 @@ mod tests { #[test] fn validate_insert_request_for_submit_derives_tag_from_file_path() { - let path = - write_workspace_markdown_file("target/kinic-insert-tests/docs/spec/api.md", "# title"); + let unique_suffix = SystemTime::now() + .duration_since(UNIX_EPOCH) + .expect("system clock should be after epoch") + .as_nanos(); + let relative_path = format!("target/kinic-insert-tests/{unique_suffix}/docs/spec/api.md"); + let path = write_workspace_markdown_file(&relative_path, "# title"); let validated = validate_and_transform_insert_request(&InsertRequest::Normal { memory_id: "aaaaa-aa".to_string(), tag: String::new(), diff --git a/rust/python.rs b/rust/python.rs index 11958b0..b615c66 100644 --- a/rust/python.rs +++ b/rust/python.rs @@ -23,6 +23,7 @@ use crate::{ }, }; use icrc_ledger_types::icrc1::account::Account; +use kinic_core::amount::E8S_PER_KINIC; pub(crate) async fn create_memory( use_mainnet: bool, @@ -219,7 +220,7 @@ pub(crate) async fn balance(use_mainnet: bool, identity: String) -> Result<(u128 let balance: u128 = candid::decode_one(&response).context("Failed to decode balance response")?; - let kinic = balance as f64 / 10_000_000f64; + let kinic = balance as f64 / E8S_PER_KINIC as f64; Ok((balance, kinic)) } diff --git a/tui/crates/tui-kit-host/src/runtime_loop.rs b/tui/crates/tui-kit-host/src/runtime_loop.rs index 507d66a..97648c1 100644 --- a/tui/crates/tui-kit-host/src/runtime_loop.rs +++ b/tui/crates/tui-kit-host/src/runtime_loop.rs @@ -692,9 +692,9 @@ fn ui_focus_from_pane(focus: PaneFocus) -> Focus { } #[cfg(test)] -fn build_ui<'a>( +struct BuildUiArgs<'a> { theme: &'a Theme, - cfg: &RuntimeLoopConfig, + cfg: &'a RuntimeLoopConfig, state: &'a CoreState, provider_render_state: &'a ProviderRenderState, textareas: &'a FormTextareas, @@ -703,7 +703,22 @@ fn build_ui<'a>( show_help: bool, show_settings: bool, animation: &'a AnimationState, -) -> TuiKitUi<'a> { +} + +#[cfg(test)] +fn build_ui<'a>(args: BuildUiArgs<'a>) -> TuiKitUi<'a> { + let BuildUiArgs { + theme, + cfg, + state, + provider_render_state, + textareas, + list_scroll_offset, + inspector_scroll, + show_help, + show_settings, + animation, + } = args; let focus = ui_focus_from_pane(state.focus); TuiKitUi::new(theme) .ui_config((cfg.ui_config)()) diff --git a/tui/crates/tui-kit-host/src/runtime_loop_tests.rs b/tui/crates/tui-kit-host/src/runtime_loop_tests.rs index 620dca1..5e20d18 100644 --- a/tui/crates/tui-kit-host/src/runtime_loop_tests.rs +++ b/tui/crates/tui-kit-host/src/runtime_loop_tests.rs @@ -261,18 +261,18 @@ fn build_ui_renders_rename_overlay_contents() { let textareas = FormTextareas::default(); let animation = AnimationState::new(); let provider_render_state = ProviderRenderState::default(); - let ui = build_ui( - &theme, - &cfg, - &state, - &provider_render_state, - &textareas, - 0, - 0, - false, - false, - &animation, - ); + let ui = build_ui(BuildUiArgs { + theme: &theme, + cfg: &cfg, + state: &state, + provider_render_state: &provider_render_state, + textareas: &textareas, + list_scroll_offset: 0, + inspector_scroll: 0, + show_help: false, + show_settings: false, + animation: &animation, + }); let area = Rect::new(0, 0, 100, 30); let mut buf = Buffer::empty(area); Widget::render(ui, area, &mut buf); @@ -1109,8 +1109,10 @@ fn chat_textarea_crlf_paste_normalizes_widget_and_state_to_single_line() { #[test] fn chat_textarea_sync_state_preserves_trailing_space() { let mut state = CoreState::default(); - let mut textareas = FormTextareas::default(); - textareas.chat_input = chat_input_from_text("hello "); + let textareas = FormTextareas { + chat_input: chat_input_from_text("hello "), + ..FormTextareas::default() + }; sync_state_from_textareas(&mut state, &textareas); @@ -1129,8 +1131,10 @@ fn chat_textarea_display_value_preserves_trailing_space() { #[test] fn chat_textarea_sync_from_state_preserves_equivalent_trailing_space_in_widget() { - let mut textareas = FormTextareas::default(); - textareas.chat_input = chat_input_from_text("hello "); + let mut textareas = FormTextareas { + chat_input: chat_input_from_text("hello "), + ..FormTextareas::default() + }; let state = CoreState { chat_input: "hello ".to_string(), ..CoreState::default() @@ -1143,8 +1147,10 @@ fn chat_textarea_sync_from_state_preserves_equivalent_trailing_space_in_widget() #[test] fn chat_textarea_sync_from_state_keeps_cursor_when_text_is_unchanged() { - let mut textareas = FormTextareas::default(); - textareas.chat_input = chat_input_from_text("hello "); + let mut textareas = FormTextareas { + chat_input: chat_input_from_text("hello "), + ..FormTextareas::default() + }; textareas.chat_input.input(textarea_input_from_key_event( crossterm::event::KeyEvent::new( crossterm::event::KeyCode::Left, @@ -1446,18 +1452,18 @@ fn build_ui_places_chat_cursor_after_trailing_space() { sync_form_textareas_from_state(&mut textareas, &state); let provider_render_state = ProviderRenderState::default(); - let ui = build_ui( - &theme, - &cfg, - &state, - &provider_render_state, - &textareas, - 0, - 0, - false, - false, - &animation, - ); + let ui = build_ui(BuildUiArgs { + theme: &theme, + cfg: &cfg, + state: &state, + provider_render_state: &provider_render_state, + textareas: &textareas, + list_scroll_offset: 0, + inspector_scroll: 0, + show_help: false, + show_settings: false, + animation: &animation, + }); let cursor = ui.cursor_position_for_area(Rect::new(0, 0, 120, 40)); assert!(cursor.is_some()); @@ -1658,8 +1664,10 @@ fn chat_submit_uses_selected_slash_command_candidate() { chat_scope: tui_kit_runtime::ChatScope::Selected, ..CoreState::default() }; - let mut textareas = FormTextareas::default(); - textareas.chat_command_selected = 1; + let mut textareas = FormTextareas { + chat_command_selected: 1, + ..FormTextareas::default() + }; let handled = handle_chat_submit_or_command( &mut provider, @@ -1937,18 +1945,19 @@ fn build_ui_forwards_insert_validation_fields_to_render_tree() { }; let textareas = FormTextareas::default(); - let ui = build_ui( - &theme, - &test_runtime_config(), - &state, - &provider_render_state, - &textareas, - 0, - 0, - false, - false, - &animation, - ); + let cfg = test_runtime_config(); + let ui = build_ui(BuildUiArgs { + theme: &theme, + cfg: &cfg, + state: &state, + provider_render_state: &provider_render_state, + textareas: &textareas, + list_scroll_offset: 0, + inspector_scroll: 0, + show_help: false, + show_settings: false, + animation: &animation, + }); let area = Rect::new(0, 0, 120, 40); let mut buf = Buffer::empty(area); ui.render(area, &mut buf); diff --git a/tui/crates/tui-kit-render/src/ui/app/screens/wiki.rs b/tui/crates/tui-kit-render/src/ui/app/screens/wiki.rs index 76715ca..635b55e 100644 --- a/tui/crates/tui-kit-render/src/ui/app/screens/wiki.rs +++ b/tui/crates/tui-kit-render/src/ui/app/screens/wiki.rs @@ -579,7 +579,6 @@ mod tests { document: DocumentSnapshot { title: "/Wiki/index.md".to_string(), lines: vec!["readonly".to_string()], - ..DocumentSnapshot::default() }, mode: ThreePaneMode::Browse, ..ThreePaneSnapshot::default() diff --git a/tui/crates/tui-kit-render/src/ui/content.rs b/tui/crates/tui-kit-render/src/ui/content.rs index 8f1794e..31d2bbe 100644 --- a/tui/crates/tui-kit-render/src/ui/content.rs +++ b/tui/crates/tui-kit-render/src/ui/content.rs @@ -453,11 +453,7 @@ mod tests { let notes_cell = (0..20) .flat_map(|y| (0..60).map(move |x| (x, y))) - .find_map(|(x, y)| { - buf.cell((x, y)) - .filter(|cell| cell.symbol() == "N") - .map(|cell| cell) - }) + .find_map(|(x, y)| buf.cell((x, y)).filter(|cell| cell.symbol() == "N")) .expect("notes cell"); assert_eq!(notes_cell.fg, theme.fg);