From 725a85214302f786c8eeefe2b51663eb7e04947b Mon Sep 17 00:00:00 2001 From: t Date: Mon, 14 Sep 2026 01:50:11 +0530 Subject: [PATCH 01/14] feat(lab): add lab-writes Cargo feature (off by default) Audit-sprint 2026-09-14 Phase 3.1. Not a default feature; adds no dependencies. A CI gate test (next commit) fails the build if any .github/workflows file ever enables it. Co-Authored-By: Claude Sonnet --- src-tauri/Cargo.toml | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src-tauri/Cargo.toml b/src-tauri/Cargo.toml index 141d5112..98ce42b3 100644 --- a/src-tauri/Cargo.toml +++ b/src-tauri/Cargo.toml @@ -22,6 +22,16 @@ voucher-scan = ["bridge-tally-protocol/voucher-scan", "bridge-tally-transport/vo # The harness is what actually drives a voucher scan, so it implies the scan # machinery it needs. live-calibration-harness = ["voucher-scan"] +# LAB-ONLY additive surface for the audit-sprint 2026-09-14 local rebuild +# workflow (Phase 3). Not default; never enable in a release or CI workflow +# (`tests/lab_writes_ci_gate.rs` fails the build if any `.github/workflows/*` +# file does). Every tool this compiles in additionally refuses at runtime +# unless `BRIDGE_LAB_WRITES=1` is set, and further refuses unless +# `BRIDGE_TALLY_PORT=9001`, `BRIDGE_LAB_TARGET_GUID` and +# `BRIDGE_LAB_DENY_GUIDS` are all present and the observed loaded-company set +# matches them. Production write guards (`agent_import_post.rs`, +# `approved_import.rs`) are never touched by this feature; it is additive. +lab-writes = [] [workspace] members = [ From 9bd3fb19caa2050ac92a095685c65f759bd93b3d Mon Sep 17 00:00:00 2001 From: t Date: Mon, 14 Sep 2026 01:50:19 +0530 Subject: [PATCH 02/14] test(lab): fail CI if any workflow enables lab-writes Gate G4 (audit-sprint 2026-09-14 Phase 3.1). Scans every .github/workflows/*.yml|yaml for the feature name (either spelling) or --all-features and fails with the offending line if found. Runs unconditionally, independent of which features the test binary itself was built with. Includes a tripwire test proving the check can fail. Co-Authored-By: Claude Sonnet --- src-tauri/tests/lab_writes_ci_gate.rs | 95 +++++++++++++++++++++++++++ 1 file changed, 95 insertions(+) create mode 100644 src-tauri/tests/lab_writes_ci_gate.rs diff --git a/src-tauri/tests/lab_writes_ci_gate.rs b/src-tauri/tests/lab_writes_ci_gate.rs new file mode 100644 index 00000000..0ca37708 --- /dev/null +++ b/src-tauri/tests/lab_writes_ci_gate.rs @@ -0,0 +1,95 @@ +//! Gate G4 (audit-sprint 2026-09-14 Phase 3.1): the `lab-writes` Cargo +//! feature must never be enabled by any release or CI workflow. This test +//! runs unconditionally (no `lab-writes` cfg gate on the test itself) so it +//! catches the mistake regardless of which features the test binary itself +//! was built with. +use std::fs; +use std::path::PathBuf; + +fn workflows_dir() -> PathBuf { + PathBuf::from(env!("CARGO_MANIFEST_DIR")) + .parent() + .expect("src-tauri has a parent directory (the repo root)") + .join(".github") + .join("workflows") +} + +#[test] +fn no_workflow_enables_the_lab_writes_feature() { + let dir = workflows_dir(); + let entries = fs::read_dir(&dir).unwrap_or_else(|error| { + panic!( + "workflows directory {} must be readable: {error}", + dir.display() + ) + }); + let mut checked_files = 0usize; + let mut offenders = Vec::new(); + for entry in entries { + let entry = entry.expect("readable workflow directory entry"); + let path = entry.path(); + let is_workflow_file = matches!( + path.extension().and_then(|ext| ext.to_str()), + Some("yml") | Some("yaml") + ); + if !is_workflow_file { + continue; + } + checked_files += 1; + let contents = fs::read_to_string(&path).unwrap_or_else(|error| { + panic!( + "workflow file {} must be valid UTF-8: {error}", + path.display() + ) + }); + for (line_number, line) in contents.lines().enumerate() { + let lower = line.to_ascii_lowercase(); + // Catch the feature name spelled either way, and `--all-features`, + // which would silently pull `lab-writes` in as a real Cargo + // feature the moment any job used it. + if lower.contains("lab-writes") + || lower.contains("lab_writes") + || lower.contains("--all-features") + { + offenders.push(format!( + "{}:{}: {}", + path.display(), + line_number + 1, + line.trim() + )); + } + } + } + assert!( + checked_files > 0, + "expected at least one workflow file under {}", + dir.display() + ); + assert!( + offenders.is_empty(), + "a release/CI workflow must never enable the lab-writes feature (Gate G4 -- \ + audit-sprint 2026-09-14 Phase 3.1): {offenders:#?}" + ); +} + +/// A tripwire for the tripwire: this test must actually be capable of +/// failing. Prove that against a synthetic workflow file in a temp +/// directory rather than by editing a real one. +#[test] +fn the_gate_actually_fails_on_a_workflow_that_enables_the_feature() { + let dir = tempfile::tempdir().expect("temp dir for the tripwire fixture"); + let workflow_path = dir.path().join("fake.yml"); + fs::write( + &workflow_path, + "jobs:\n build:\n steps:\n - run: cargo build --features lab-writes\n", + ) + .expect("write synthetic workflow fixture"); + let contents = fs::read_to_string(&workflow_path).unwrap(); + let tripped = contents + .lines() + .any(|line| line.to_ascii_lowercase().contains("lab-writes")); + assert!( + tripped, + "the substring check itself must catch this fixture" + ); +} From 3a0dadfacc510515387f5b7616a914e86a4245b8 Mon Sep 17 00:00:00 2001 From: t Date: Mon, 14 Sep 2026 01:50:28 +0530 Subject: [PATCH 03/14] feat(lab): wire lab tool registration and dispatch (feature+env gated) Audit-sprint 2026-09-14 Phase 3.1. Adds the agent_lab module declaration (cfg-gated on lab-writes) and, additive alongside the existing tools, the lab_read_inventory dispatch arm and catalog entry. Registration requires both the compiled feature and BRIDGE_LAB_WRITES=1 (checked fresh per catalog build, not cached at startup); the internal tool registry hides it from existence checks the same way when the env var is unset, and tool_payload additionally refuses in-process calls with lab_writes_disabled. No change to any production write path (build_import_xml/post_import/agent_import_post.rs/approved_import.rs). Co-Authored-By: Claude Sonnet --- src-tauri/src/agent.rs | 16 ++++++++++++++++ src-tauri/src/agent_catalog.rs | 25 ++++++++++++++++++++++++- 2 files changed, 40 insertions(+), 1 deletion(-) diff --git a/src-tauri/src/agent.rs b/src-tauri/src/agent.rs index e4320201..e115b0df 100644 --- a/src-tauri/src/agent.rs +++ b/src-tauri/src/agent.rs @@ -10,6 +10,16 @@ mod agent_import; pub use crate::tally::approved_import::run_confirmation; pub(crate) use agent_import::desktop_journal_review as desktop_journal; +// LAB-ONLY additive surface (audit-sprint 2026-09-14, Phase 3). Compiled only +// behind the `lab-writes` Cargo feature (not default, never enabled in a +// release/CI workflow -- see `tests/lab_writes_ci_gate.rs`) and every tool it +// registers additionally refuses at runtime unless `BRIDGE_LAB_WRITES=1` and +// the rest of the lab guard env is present. Never modifies the production +// write guards (`agent_import.rs`/`agent_import_post.rs`/`approved_import.rs`). +#[cfg(feature = "lab-writes")] +#[path = "agent_lab.rs"] +mod lab; + #[path = "agent_catalog.rs"] mod catalog; #[cfg(test)] @@ -637,6 +647,10 @@ impl Server { if name == "post_import" && !self.settings.writes_enabled { return Err("import_posting_disabled".to_string().into()); } + #[cfg(feature = "lab-writes")] + if name == "lab_read_inventory" { + lab::require_lab_writes_env()?; + } validate_tool_arguments(name, args)?; match name { "tally_status" => { @@ -677,6 +691,8 @@ impl Server { "trial_balance" => self.trial_balance(args).await, "read_evidence" => self.read_evidence(args).map_err(Into::into), "egress_log" => self.egress_log(args).map_err(Into::into), + #[cfg(feature = "lab-writes")] + "lab_read_inventory" => lab::lab_read_inventory(self, args).await, _ => Err("tool_not_found".to_string().into()), } } diff --git a/src-tauri/src/agent_catalog.rs b/src-tauri/src/agent_catalog.rs index c1e158bd..1e00c6cd 100644 --- a/src-tauri/src/agent_catalog.rs +++ b/src-tauri/src/agent_catalog.rs @@ -114,9 +114,19 @@ pub(super) fn tool_definitions(import_enabled: bool, writes_enabled: bool) -> Va definitions } +#[cfg(feature = "lab-writes")] +fn lab_tools_env_enabled() -> bool { + super::lab::env_lab_writes_enabled() +} +#[cfg(not(feature = "lab-writes"))] +fn lab_tools_env_enabled() -> bool { + false +} + // Retain the internal schema while bounded change enumeration is unqualified. pub(super) fn registered_tool_definitions(import_enabled: bool, writes_enabled: bool) -> Value { - let names = [ + #[allow(unused_mut)] // only mutated when the `lab-writes` feature is compiled in + let mut names = vec![ "tally_status", "list_companies", "voucher_schema", @@ -133,6 +143,8 @@ pub(super) fn registered_tool_definitions(import_enabled: bool, writes_enabled: "read_evidence", "egress_log", ]; + #[cfg(feature = "lab-writes")] + names.push("lab_read_inventory"); Value::Array( names .into_iter() @@ -141,6 +153,10 @@ pub(super) fn registered_tool_definitions(import_enabled: bool, writes_enabled: // uncertain saved batch can still be checked safely. .filter(|name| import_enabled || *name != "build_import_xml") .filter(|name| writes_enabled || *name != "post_import") + // LAB-ONLY: registered only when the `lab-writes` feature is + // compiled in AND `BRIDGE_LAB_WRITES=1` is set (checked fresh on + // every catalog build, not cached at startup). + .filter(|name| *name != "lab_read_inventory" || lab_tools_env_enabled()) .map(|name| { let (description, input_schema) = match name { "voucher_schema" => ( @@ -199,6 +215,10 @@ pub(super) fn registered_tool_definitions(import_enabled: bool, writes_enabled: "Return bounded local metadata-only read evidence or egress receipts.", json!({"type":"object","additionalProperties":false,"properties":{"limit":{"type":"integer","minimum":1,"default":20}}}), ), + "lab_read_inventory" => ( + "LAB-ONLY. Compiled only behind the `lab-writes` feature and refuses unless BRIDGE_LAB_WRITES=1, BRIDGE_TALLY_PORT=9001, and BRIDGE_LAB_TARGET_GUID/BRIDGE_LAB_DENY_GUIDS are both set to well-formed GUIDs. This is a read: company_guid selects the company like any other read tool and is verified the same way (`company_identity_not_found`/`company_identity_ambiguous`), independent of the configured lab target -- the stronger loaded-company/deny-list guard applies only to a lab write batch, not a read. Read-only: units, godowns, stock groups and stock items (parent, base unit, opening qty/rate/value, GST/HSN fields as returned, unclassified), plus inventory entries per voucher for a date window. Reuses the same windowing and window_honoured corroboration as `vouchers`. No signed compatibility evidence exists yet for any inventory field on this Tally release/mode -- treat every value as exploratory.", + json!({"type":"object","additionalProperties":false,"required":["company_guid","from","to"],"properties":{"company_guid":{"type":"string","minLength":1},"from":{"type":"string","pattern":"^[0-9]{4}-?[0-9]{2}-?[0-9]{2}$"},"to":{"type":"string","pattern":"^[0-9]{4}-?[0-9]{2}-?[0-9]{2}$"},"offset":{"type":"integer","minimum":0,"default":0},"limit":{"type":"integer","minimum":1,"default":500}}}), + ), _ => ( "Bridge read-only Tally tool", json!({"type":"object", "additionalProperties": false}), @@ -208,6 +228,9 @@ pub(super) fn registered_tool_definitions(import_enabled: bool, writes_enabled: if name == "post_import" { tool["annotations"] = json!({"readOnlyHint":false,"destructiveHint":true,"idempotentHint":false,"openWorldHint":true}); } + if name == "lab_read_inventory" { + tool["annotations"] = json!({"readOnlyHint":true,"destructiveHint":false,"idempotentHint":true,"openWorldHint":true}); + } tool }) .collect(), From ca51e6c79885d45de4910999b5f6246909afeda1 Mon Sep 17 00:00:00 2001 From: t Date: Mon, 14 Sep 2026 01:50:36 +0530 Subject: [PATCH 04/14] feat(lab): windowed inventory-voucher read profile Audit-sprint 2026-09-14 Phase 3.1/3.2. Adds render_agent_lab_inventory_vouchers, a thin lab-only wrapper over the existing render_windowed_vouchers machinery with an ALLINVENTORYENTRIES.* FETCH list in place of vouchers'/ledger_movement's ALLLEDGERENTRIES.* -- reuses the same windowing (and therefore the same window_honoured corroboration path) rather than a parallel one. Compiled only behind lab-writes. Co-Authored-By: Claude Sonnet --- src-tauri/src/agent_read_profiles.rs | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/src-tauri/src/agent_read_profiles.rs b/src-tauri/src/agent_read_profiles.rs index 5479b66e..731a4f0e 100644 --- a/src-tauri/src/agent_read_profiles.rs +++ b/src-tauri/src/agent_read_profiles.rs @@ -74,6 +74,29 @@ fn render_windowed_vouchers( )) } +/// The FETCH list for LAB-ONLY inventory-entry reads (`lab_read_inventory`, +/// feature `lab-writes`). Unlike [`AGENT_VOUCHER_FETCH`], this asks for +/// `ALLINVENTORYENTRIES.*` instead of `ALLLEDGERENTRIES.*` -- no shipped tool +/// reads inventory today (see the plan-research note §4.2), so this shape is +/// exploratory pending a live capture, not a qualified/compatibility-evidenced +/// read. +#[cfg(feature = "lab-writes")] +const AGENT_LAB_INVENTORY_VOUCHER_FETCH: &str = "DATE,VOUCHERNUMBER,VOUCHERTYPENAME,\ +PARTYLEDGERNAME,NARRATION,GUID,ALTERID,MASTERID,ISCANCELLED,ISOPTIONAL,ALLINVENTORYENTRIES.*"; + +/// Windowed voucher read for the LAB-ONLY `lab_read_inventory` tool. Reuses +/// the same [`render_windowed_vouchers`] windowing machinery (and therefore +/// the same `window_honoured` corroboration path) as `vouchers`/ +/// `ledger_movement` -- only the FETCH list differs. +#[cfg(feature = "lab-writes")] +pub(super) fn render_agent_lab_inventory_vouchers( + company: &str, + from: &str, + to: &str, +) -> Result { + render_windowed_vouchers(company, from, to, None, AGENT_LAB_INVENTORY_VOUCHER_FETCH) +} + pub(super) fn render_agent_changed_vouchers( company: &str, checkpoint: u64, From 3d706d2f7910ffd3ea7abca86a8f8649ae64d761 Mon Sep 17 00:00:00 2001 From: t Date: Mon, 14 Sep 2026 01:51:16 +0530 Subject: [PATCH 05/14] feat(lab): lab guard scaffolding and lab_read_inventory tool Audit-sprint 2026-09-14 Phase 3.1/3.2. Additive, lab-writes-only module: - Runtime guards, all fail-closed with explicit error codes: BRIDGE_LAB_WRITES=1, BRIDGE_TALLY_PORT=9001, BRIDGE_LAB_TARGET_GUID and BRIDGE_LAB_DENY_GUIDS (comma list, both well-formed GUIDs). - admit_lab_target: reserved for the Phase 3.5 lab writer -- re-verifies the port/env guards, that the observed loaded-company set includes the target and excludes every deny GUID (lab_source_company_loaded/lab_target_not_loaded), and the target identity (4 fields) via the existing verified_company. Every request this module builds renders SVCURRENTCOMPANY from that verified identity's exact display name, making the SVCURRENTCOMPANY-equals- target requirement structural rather than a separate check. Not called by lab_read_inventory (a read does not need the target to be the company read, or to be the company currently loaded) -- kept for the next pass to reuse. - lab_read_inventory (read-only): units, godowns, stock groups, stock items (parent, base unit, opening qty/rate/value, GST/HSN fields as returned, unclassified -- no signed compatibility evidence exists yet for any of them), and inventory entries per voucher (with nested batch allocations) for a date window, reusing window_honoured. - Every lab request/response persisted under data_dir/lab/, named and manifested by their own sha256 (contrast egress_log, which never persists raw bodies). - Unit tests on synthetic fixtures only (BRIDGE CORPUS GST-style shapes), no client data, no live Tally connection. Does not modify agent_import_post.rs or approved_import.rs. Co-Authored-By: Claude Sonnet --- src-tauri/src/agent_lab.rs | 849 +++++++++++++++++++++++++++++++++++++ 1 file changed, 849 insertions(+) create mode 100644 src-tauri/src/agent_lab.rs diff --git a/src-tauri/src/agent_lab.rs b/src-tauri/src/agent_lab.rs new file mode 100644 index 00000000..e5d63ce8 --- /dev/null +++ b/src-tauri/src/agent_lab.rs @@ -0,0 +1,849 @@ +//! LAB-ONLY additive surface (audit-sprint 2026-09-14, Phase 3.1/3.2). +//! +//! This entire file is compiled only behind the `lab-writes` Cargo feature +//! (not default; never enabled in a release/CI workflow -- see +//! `tests/lab_writes_ci_gate.rs`), and every tool it registers additionally +//! refuses at runtime unless `BRIDGE_LAB_WRITES=1` is set. It never modifies +//! the production write guards (`agent_import.rs`, `agent_import_post.rs`, +//! `approved_import.rs`); this module adds a parallel, narrowly-scoped +//! surface rather than changing those. Scope for this pass (3.1/3.2): the +//! shared lab guard machinery (a later lab writer will reuse +//! `admit_lab_target`) and one read-only tool, `lab_read_inventory`. No write +//! path exists anywhere in this file. +//! +//! No signed compatibility evidence exists for any inventory field read here +//! on any Tally release/mode (the plan-research note's §4.2/§4.5 findings): +//! every value this tool returns is exploratory, not a qualified claim. + +use super::*; +use std::collections::BTreeMap; +use std::fs; +use std::path::PathBuf; + +// --------------------------------------------------------------------------- +// Env gates +// --------------------------------------------------------------------------- + +/// `BRIDGE_LAB_WRITES=1` gate. Read fresh on every call -- this is a lab +/// safety gate, not a cached setting, so flipping the env var takes effect on +/// the very next call rather than only after a restart. +pub(super) fn env_lab_writes_enabled() -> bool { + matches!( + env::var("BRIDGE_LAB_WRITES").as_deref(), + Ok("1") | Ok("true") + ) +} + +pub(super) fn require_lab_writes_env() -> Result<(), String> { + env_lab_writes_enabled() + .then_some(()) + .ok_or_else(|| "lab_writes_disabled".to_string()) +} + +/// The lab guard configuration: the one company lab tools may touch, and the +/// companies they must never see loaded. Every field is required; any +/// absence or malformed value fails closed before any Tally request is made. +#[cfg_attr(test, derive(Debug, PartialEq))] +struct LabGuardConfig { + target_guid: String, + deny_guids: Vec, +} + +impl LabGuardConfig { + fn from_env() -> Result { + let target = env::var("BRIDGE_LAB_TARGET_GUID") + .map_err(|_| "lab_target_guid_required".to_string())?; + let deny_raw = + env::var("BRIDGE_LAB_DENY_GUIDS").map_err(|_| "lab_deny_guids_required".to_string())?; + Self::from_values(&target, &deny_raw) + } + + /// Pure parser over already-read env values -- kept separate from + /// [`Self::from_env`] so tests exercise parsing without mutating the + /// process-wide `std::env`, which is unsound to do from parallel test + /// threads. + fn from_values(target: &str, deny_raw: &str) -> Result { + let target_guid = parse_native_company_guid(target.trim()) + .map_err(|_| "lab_target_guid_invalid".to_string())? + .hyphenated() + .to_string(); + let mut deny_guids = Vec::new(); + for candidate in deny_raw.split(',') { + let candidate = candidate.trim(); + if candidate.is_empty() { + continue; + } + let guid = parse_native_company_guid(candidate) + .map_err(|_| "lab_deny_guids_invalid".to_string())? + .hyphenated() + .to_string(); + deny_guids.push(guid); + } + if deny_guids.is_empty() { + return Err("lab_deny_guids_required".to_string()); + } + if deny_guids + .iter() + .any(|guid| guid.eq_ignore_ascii_case(&target_guid)) + { + return Err("lab_deny_guids_invalid".to_string()); + } + Ok(Self { + target_guid, + deny_guids, + }) + } +} + +// --------------------------------------------------------------------------- +// Guard admission +// --------------------------------------------------------------------------- + +/// The always-on lab preconditions: `BRIDGE_LAB_WRITES=1`, the Tally endpoint +/// pinned to port 9001, and `BRIDGE_LAB_TARGET_GUID`/`BRIDGE_LAB_DENY_GUIDS` +/// both present and well-formed. Every lab tool call requires these, +/// `lab_read_inventory` included -- but a *read* does not require the target +/// to be the company being read, or to be the company currently loaded (that +/// stronger requirement, [`admit_lab_target`], is scoped to a write batch: +/// "before every write batch, observed loaded companies must include target +/// and exclude every deny GUID"). +fn require_lab_read_guards(server: &Server) -> Result { + require_lab_writes_env().map_err(ToolFailure::from)?; + if server.settings.endpoint.port != 9001 { + return Err("lab_port_not_9001".to_string().into()); + } + LabGuardConfig::from_env().map_err(ToolFailure::from) +} + +/// Re-verifies every lab guard immediately before a lab **write batch**: the +/// port, the deny/target loaded-company set, and the target's 4-field +/// identity. Two independent company-list reads are deliberate -- one for +/// the loaded-set check below, one inside `verified_company` for identity -- +/// matching the paranoia the production write path already applies +/// immediately before dispatch (`require_unique_company_scope`). +/// +/// The returned `VerifiedCompanyIdentity`'s exact `display_name()` is the +/// only company name a lab writer may render into `SVCURRENTCOMPANY` -- +/// that makes "SVCURRENTCOMPANY = target's exact name" a structural property +/// of any request built from this identity, not a separate check that could +/// drift out of sync with it. +/// +/// Reserved for the Phase 3.5 lab writer (not yet built); `lab_read_inventory` +/// (Phase 3.2, read-only) intentionally does not call this -- see +/// [`require_lab_read_guards`]. +#[allow(dead_code)] +pub(super) async fn admit_lab_target( + server: &Server, +) -> Result<(TallyCompany, VerifiedCompanyIdentity, Evidence), ToolFailure> { + let config = require_lab_read_guards(server)?; + let (companies, evidence) = server.companies().await?; + let denied = companies.iter().any(|company| { + company.guid.as_deref().is_some_and(|guid| { + config + .deny_guids + .iter() + .any(|deny| deny.eq_ignore_ascii_case(guid)) + }) + }); + if denied { + return Err(ToolFailure::from("lab_source_company_loaded".to_string()) + .with_prior_evidence(evidence)); + } + let loaded = companies.iter().any(|company| { + company + .guid + .as_deref() + .is_some_and(|guid| guid.eq_ignore_ascii_case(&config.target_guid)) + }); + if !loaded { + return Err( + ToolFailure::from("lab_target_not_loaded".to_string()).with_prior_evidence(evidence) + ); + } + let (company, identity, verify_evidence) = + server + .verified_company(&config.target_guid) + .await + .map_err(|failure| failure.with_prior_evidence(evidence.clone()))?; + Ok(( + company, + identity, + combine_evidence(evidence, verify_evidence), + )) +} + +// --------------------------------------------------------------------------- +// Local evidence persistence -- every lab request/response, with sha256 +// --------------------------------------------------------------------------- + +fn lab_evidence_dir(server: &Server) -> Result { + let dir = server.settings.data_dir.join("lab"); + ensure_private_directory(&dir).map_err(|error| match error { + DirectoryAdmissionError::Unavailable => "lab_dir_unavailable".to_string(), + #[cfg(unix)] + DirectoryAdmissionError::Permissions => "lab_dir_permissions_failed".to_string(), + })?; + Ok(dir) +} + +/// Persists the raw request and response bytes for one lab Tally exchange +/// under `data_dir/lab/`, named and manifested by their own sha256 -- every +/// lab request/response is retained, not just its receipt (contrast +/// `egress_log`, which deliberately never persists raw bodies). +fn persist_lab_exchange( + server: &Server, + tool: &str, + request_xml: &str, + response_xml: &str, +) -> Result<(), String> { + let dir = lab_evidence_dir(server)?; + let request_sha256 = sha256_hex(request_xml.as_bytes()); + let response_sha256 = sha256_hex(response_xml.as_bytes()); + write_private_file( + &dir.join(format!("{request_sha256}.request.xml")), + request_xml.as_bytes(), + )?; + write_private_file( + &dir.join(format!("{response_sha256}.response.xml")), + response_xml.as_bytes(), + )?; + let record = json!({ + "tool": tool, + "at": Utc::now().to_rfc3339_opts(SecondsFormat::Millis, true), + "request_sha256": request_sha256, + "response_sha256": response_sha256, + }); + append_egress_line(&dir.join("lab-manifest.jsonl"), &record.to_string()) +} + +fn write_private_file(path: &std::path::Path, bytes: &[u8]) -> Result<(), String> { + fs::write(path, bytes).map_err(|_| "lab_evidence_write_failed".to_string())?; + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + fs::set_permissions(path, fs::Permissions::from_mode(0o600)) + .map_err(|_| "lab_evidence_write_failed".to_string())?; + } + Ok(()) +} + +/// A `post_read` that additionally persists the exchange (§ above) before +/// returning. All lab reads go through this, never the bare `post_read`. +async fn lab_post_read( + server: &Server, + identity: &VerifiedCompanyIdentity, + tool: &str, + request: String, +) -> Result<(String, Evidence), ToolFailure> { + let (response, evidence) = server.post_read(identity, request.clone()).await?; + persist_lab_exchange(server, tool, &request, &response) + .map_err(|code| ToolFailure::from(code).with_prior_evidence(evidence.clone()))?; + Ok((response, evidence)) +} + +// --------------------------------------------------------------------------- +// Master reads: units, godowns, stock groups, stock items +// --------------------------------------------------------------------------- + +#[derive(Clone, Copy)] +enum LabMasterKind { + Unit, + Godown, + StockGroup, + StockItem, +} + +impl LabMasterKind { + const ALL: [Self; 4] = [Self::Unit, Self::Godown, Self::StockGroup, Self::StockItem]; + + fn tally_type(self) -> &'static str { + match self { + Self::Unit => "Unit", + Self::Godown => "Godown", + Self::StockGroup => "StockGroup", + Self::StockItem => "StockItem", + } + } + + fn fetch_fields(self) -> &'static str { + match self { + Self::Unit => "NAME,ORIGINALNAME,ISSIMPLEUNIT,DECIMALPLACES,GUID,MASTERID,ALTERID", + Self::Godown => "NAME,PARENT,GUID,MASTERID,ALTERID", + Self::StockGroup => "NAME,PARENT,GUID,MASTERID,ALTERID", + Self::StockItem => { + "NAME,PARENT,BASEUNITS,OPENINGBALANCE,OPENINGRATE,OPENINGVALUE,\ + GSTAPPLICABLE,GSTTYPEOFSUPPLY,HSNCODE,GSTHSNNAME,GUID,MASTERID,ALTERID" + } + } + } + + fn result_key(self) -> &'static str { + match self { + Self::Unit => "units", + Self::Godown => "godowns", + Self::StockGroup => "stock_groups", + Self::StockItem => "stock_items", + } + } +} + +fn render_lab_master_collection(company: &str, kind: LabMasterKind) -> Result { + let company = ValidatedCompanyName::new(company.to_string()) + .map_err(|_| "company_name_invalid".to_string())?; + let object_type = kind.tally_type(); + let name = format!("Bridge Lab {object_type}s"); + Ok(format!( + r#"
1ExportCollection{name}
$$SysName:XML{}{object_type}{}"#, + xml_escape(company.as_str()), + kind.fetch_fields() + )) +} + +/// Parses a flat master collection (`...` etc, one +/// level under `COLLECTION`) into raw field maps. Deliberately conservative +/// like the production parsers: an unexpected non-row child of `COLLECTION` +/// fails closed rather than being silently skipped. +fn parse_lab_master_rows( + xml: &str, + row_tag: &str, +) -> Result>, String> { + validate_agent_envelope(xml)?; + let row_tag = row_tag.to_ascii_uppercase(); + let mut reader = quick_xml::Reader::from_str(xml); + reader.config_mut().trim_text(false); + let mut path: Vec = Vec::new(); + let mut rows = Vec::new(); + let mut current: Option> = None; + let mut current_tag = String::new(); + loop { + match reader.read_event() { + Ok(quick_xml::events::Event::Start(event)) => { + let tag = String::from_utf8_lossy(event.name().as_ref()).to_ascii_uppercase(); + let at_collection = path == ["ENVELOPE", "BODY", "DATA", "COLLECTION"]; + if at_collection { + if tag != row_tag { + return Err("agent_read_protocol_invalid".to_string()); + } + let mut row = BTreeMap::new(); + for attribute in event.attributes() { + let attribute = + attribute.map_err(|_| "agent_read_protocol_invalid".to_string())?; + if attribute.key.as_ref().eq_ignore_ascii_case(b"NAME") { + row.insert( + "NAME".to_string(), + attribute + .decoded_and_normalized_value( + quick_xml::XmlVersion::Implicit1_0, + reader.decoder(), + ) + .map_err(|_| "agent_read_protocol_invalid".to_string())? + .into_owned(), + ); + } + } + current = Some(row); + } + path.push(tag.clone()); + current_tag = tag; + } + Ok(quick_xml::events::Event::Text(text)) => { + // `path` includes the just-opened field tag (`current_tag`); + // a field belongs to the row if its parent chain is exactly + // COLLECTION_PREFIX + [row_tag]. + let is_row_field = path.len() == 6 + && path[..4] == ["ENVELOPE", "BODY", "DATA", "COLLECTION"] + && path[4] == row_tag; + if is_row_field { + if let Some(row) = current.as_mut() { + append_agent_text(row, ¤t_tag, decoded_agent_text(text)?); + } + } + } + Ok(quick_xml::events::Event::End(event)) => { + let end = String::from_utf8_lossy(event.name().as_ref()).to_ascii_uppercase(); + if path.last().map(String::as_str) == Some(end.as_str()) + && end == row_tag + && path.len() == 5 + { + if let Some(row) = current.take() { + rows.push(row); + } + } + if path.pop().as_deref() != Some(end.as_str()) { + return Err("agent_read_protocol_invalid".to_string()); + } + } + Ok(quick_xml::events::Event::Eof) => break, + Ok(_) => {} + Err(_) => return Err("agent_read_protocol_invalid".to_string()), + } + } + Ok(rows) +} + +fn optional_field(row: &BTreeMap, key: &str) -> Value { + row.get(key) + .filter(|value| !value.trim().is_empty()) + .map(|value| Value::String(value.clone())) + .unwrap_or(Value::Null) +} + +fn lab_master_json(kind: LabMasterKind, row: &BTreeMap) -> Value { + match kind { + LabMasterKind::Unit => json!({ + "name": party_name(row.get("NAME").cloned().unwrap_or_default()), + "is_simple_unit": optional_field(row, "ISSIMPLEUNIT"), + "decimal_places": optional_field(row, "DECIMALPLACES"), + }), + LabMasterKind::Godown | LabMasterKind::StockGroup => json!({ + "name": party_name(row.get("NAME").cloned().unwrap_or_default()), + "parent": optional_field(row, "PARENT"), + }), + LabMasterKind::StockItem => json!({ + "name": party_name(row.get("NAME").cloned().unwrap_or_default()), + "parent": optional_field(row, "PARENT"), + "base_unit": optional_field(row, "BASEUNITS"), + "opening_qty": optional_field(row, "OPENINGBALANCE"), + "opening_rate": optional_field(row, "OPENINGRATE"), + "opening_value": optional_field(row, "OPENINGVALUE"), + // GST/HSN fields as returned, unclassified -- no signed + // compatibility evidence exists for these on any Tally + // release/mode yet (unlike the ledger GST duty-head vocabulary, + // which §4.3 confirms is live-capture backed). + "gst_applicable": optional_field(row, "GSTAPPLICABLE"), + "gst_type_of_supply": optional_field(row, "GSTTYPEOFSUPPLY"), + "hsn_code": optional_field(row, "HSNCODE"), + "gst_hsn_name": optional_field(row, "GSTHSNNAME"), + }), + } +} + +// --------------------------------------------------------------------------- +// Inventory entries per voucher, windowed +// --------------------------------------------------------------------------- + +/// True when `path` (the currently open element stack, most-recent last) +/// equals `expected` exactly. +fn path_is(path: &[String], expected: &[&str]) -> bool { + path.len() == expected.len() && path.iter().zip(expected).all(|(a, b)| a == b) +} + +const COLLECTION_PREFIX: [&str; 4] = ["ENVELOPE", "BODY", "DATA", "COLLECTION"]; +const VOUCHER_PREFIX: [&str; 5] = ["ENVELOPE", "BODY", "DATA", "COLLECTION", "VOUCHER"]; +const ENTRY_PREFIX: [&str; 6] = [ + "ENVELOPE", + "BODY", + "DATA", + "COLLECTION", + "VOUCHER", + "ALLINVENTORYENTRIES.LIST", +]; +const BATCH_PREFIX: [&str; 7] = [ + "ENVELOPE", + "BODY", + "DATA", + "COLLECTION", + "VOUCHER", + "ALLINVENTORYENTRIES.LIST", + "BATCHALLOCATIONS.LIST", +]; + +/// Parses `ALLINVENTORYENTRIES.LIST` (with an optional nested +/// `BATCHALLOCATIONS.LIST`) per voucher, mirroring the two-level nesting +/// already proven for `ALLLEDGERENTRIES.LIST`/`BILLALLOCATIONS.LIST` +/// (`agent_voucher_parse.rs`). Rows carry a lower-case `date` field so the +/// shared `window_honoured` check can be reused unmodified. +fn parse_lab_inventory_vouchers(xml: &str) -> Result, String> { + validate_agent_envelope(xml)?; + let mut reader = quick_xml::Reader::from_str(xml); + reader.config_mut().trim_text(false); + let mut path: Vec = Vec::new(); + let mut rows = Vec::new(); + let mut voucher: Option> = None; + let mut entry: Option> = None; + let mut batch: Option> = None; + let mut entries: Vec = Vec::new(); + let mut batches: Vec = Vec::new(); + let mut current_tag = String::new(); + loop { + match reader.read_event() { + Ok(quick_xml::events::Event::Start(event)) => { + let tag = String::from_utf8_lossy(event.name().as_ref()).to_ascii_uppercase(); + // `path` is the parent chain of the element about to open. + if path_is(&path, &COLLECTION_PREFIX) { + if tag != "VOUCHER" { + return Err("agent_read_protocol_invalid".to_string()); + } + voucher = Some(BTreeMap::new()); + entries.clear(); + } else if path_is(&path, &VOUCHER_PREFIX) && tag == "ALLINVENTORYENTRIES.LIST" { + entry = Some(BTreeMap::new()); + batches.clear(); + } else if path_is(&path, &ENTRY_PREFIX) && tag == "BATCHALLOCATIONS.LIST" { + batch = Some(BTreeMap::new()); + } + path.push(tag.clone()); + current_tag = tag; + } + Ok(quick_xml::events::Event::Text(text)) => { + let value = decoded_agent_text(text)?; + // `path` here includes the just-opened `current_tag`, so a + // field at depth N+1 belongs to the container at depth N. + if path_is(&path[..path.len().saturating_sub(1)], &BATCH_PREFIX) { + if let Some(row) = batch.as_mut() { + append_agent_text(row, ¤t_tag, value); + } + } else if path_is(&path[..path.len().saturating_sub(1)], &ENTRY_PREFIX) { + if let Some(row) = entry.as_mut() { + append_agent_text(row, ¤t_tag, value); + } + } else if path_is(&path[..path.len().saturating_sub(1)], &VOUCHER_PREFIX) { + if let Some(row) = voucher.as_mut() { + append_agent_text(row, ¤t_tag, value); + } + } + } + Ok(quick_xml::events::Event::End(event)) => { + let end = String::from_utf8_lossy(event.name().as_ref()).to_ascii_uppercase(); + let closing_batch = end == "BATCHALLOCATIONS.LIST" && path_is(&path, &BATCH_PREFIX); + let closing_entry = + end == "ALLINVENTORYENTRIES.LIST" && path_is(&path, &ENTRY_PREFIX); + let closing_voucher = end == "VOUCHER" && path_is(&path, &VOUCHER_PREFIX); + if closing_batch { + if let Some(row) = batch.take() { + batches.push(json!({ + "batch": optional_field(&row, "BATCHNAME"), + "godown": optional_field(&row, "GODOWNNAME"), + "actual_qty": optional_field(&row, "ACTUALQTY"), + "billed_qty": optional_field(&row, "BILLEDQTY"), + "amount": optional_field(&row, "AMOUNT"), + })); + } + } + if closing_entry { + if let Some(row) = entry.take() { + let mut json_entry = json!({ + "stock_item": optional_field(&row, "STOCKITEMNAME"), + "rate": optional_field(&row, "RATE"), + "amount": optional_field(&row, "AMOUNT"), + "actual_qty": optional_field(&row, "ACTUALQTY"), + "billed_qty": optional_field(&row, "BILLEDQTY"), + "godown": optional_field(&row, "GODOWNNAME"), + }); + if !batches.is_empty() { + json_entry["batch_allocations"] = Value::Array(batches.clone()); + } + entries.push(json_entry); + batches.clear(); + } + } + if closing_voucher { + if let Some(row) = voucher.take() { + let date = row.get("DATE").cloned().unwrap_or_default(); + rows.push(json!({ + "date": date, + "voucher_number": optional_field(&row, "VOUCHERNUMBER"), + "voucher_type": optional_field(&row, "VOUCHERTYPENAME"), + "party": row.get("PARTYLEDGERNAME").cloned().map(party_name), + "guid": optional_field(&row, "GUID"), + "is_cancelled": optional_field(&row, "ISCANCELLED"), + "inventory_entries": entries.clone(), + })); + entries.clear(); + } + } + if path.pop().as_deref() != Some(end.as_str()) { + return Err("agent_read_protocol_invalid".to_string()); + } + } + Ok(quick_xml::events::Event::Eof) => break, + Ok(_) => {} + Err(_) => return Err("agent_read_protocol_invalid".to_string()), + } + } + Ok(rows) +} + +// --------------------------------------------------------------------------- +// The tool: lab_read_inventory +// --------------------------------------------------------------------------- + +pub(super) async fn lab_read_inventory( + server: &Server, + args: &Value, +) -> Result { + let guid = required_string(args, "company_guid")?; + let from = normalized_date(required_string(args, "from")?)?; + let to = normalized_date(required_string(args, "to")?)?; + if from > to { + return Err("invalid_date_range".to_string().into()); + } + // Always-on lab preconditions (feature+env; §3.1). This is a read, so it + // reads whichever company_guid the caller asks for -- it does not require + // that company to be BRIDGE_LAB_TARGET_GUID, nor that the target be the + // one currently loaded; that stronger loaded-company/deny-list guard is + // scoped to a write batch (see `admit_lab_target`). + require_lab_read_guards(server)?; + let (company, identity, mut evidence) = server.verified_company(guid).await?; + let result: Result = async { + let mut masters = serde_json::Map::new(); + for kind in LabMasterKind::ALL { + let request = render_lab_master_collection(identity.display_name(), kind) + .map_err(ToolFailure::from)?; + let (xml, read_evidence) = + lab_post_read(server, &identity, "lab_read_inventory.masters", request).await?; + evidence = combine_evidence(evidence.clone(), read_evidence); + let rows = parse_lab_master_rows(&xml, kind.tally_type()) + .map_err(|code| ToolFailure::from(code).with_prior_evidence(evidence.clone()))?; + let items = rows + .iter() + .map(|row| lab_master_json(kind, row)) + .collect::>(); + masters.insert(kind.result_key().to_string(), Value::Array(items)); + } + + let voucher_request = render_agent_lab_inventory_vouchers(identity.display_name(), &from, &to) + .map_err(ToolFailure::from)?; + let (voucher_xml, voucher_evidence) = lab_post_read( + server, + &identity, + "lab_read_inventory.vouchers", + voucher_request, + ) + .await?; + evidence = combine_evidence(evidence.clone(), voucher_evidence); + let rows = parse_lab_inventory_vouchers(&voucher_xml) + .map_err(|code| ToolFailure::from(code).with_prior_evidence(evidence.clone()))?; + if !window_honoured(&rows, &from, &to) { + return Err(ToolFailure::from("window_not_honoured".to_string()) + .with_prior_evidence(evidence.clone())); + } + let offset = arg_usize(args, "offset", 0)?; + let limit = arg_positive_usize(args, "limit", server.settings.max_rows)? + .min(server.settings.max_rows); + let total = rows.len(); + let items = rows + .into_iter() + .skip(offset) + .take(limit) + .map(|row| redact_value(row, server.settings.redaction)) + .collect::>(); + let truncated = offset.saturating_add(items.len()) < total; + + let mut result = Value::Object(masters); + result["vouchers"] = json!({ + "items": items, + "offset": offset, + "total": total, + "window_honoured": true, + "profile": "lab_read_inventory_v1", + }); + Ok(ToolOutcome { + payload: json!({"company": company_json(&company, std::slice::from_ref(&company)), "result": result}), + evidence: evidence.clone(), + company_guid: Some(guid.to_string()), + truncated, + }) + } + .await; + result.map_err(|failure| failure.with_prior_evidence(evidence)) +} + +// --------------------------------------------------------------------------- +// Tests -- synthetic fixtures only (BRIDGE CORPUS GST-style shapes), no +// client data, and no live Tally connection. +// --------------------------------------------------------------------------- + +#[cfg(test)] +mod tests { + use super::*; + + const TARGET_GUID: &str = "89b0cc46-e3b8-4809-8fc7-e29eb2ae547d"; + const DENY_GUID_1: &str = "2864b4ac-e5a3-4efc-9d2b-7593928d8f8b"; + const DENY_GUID_2: &str = "a0f82923-3d25-4757-80e0-4fa786e34610"; + + /// `std::env::set_var`/`remove_var` are process-wide, so any test that + /// touches real env vars (as opposed to `LabGuardConfig::from_values`, + /// which takes plain strings) must serialize against every other such + /// test in this module -- otherwise two tests racing on the same + /// process env corrupt each other's reads under parallel test threads. + static ENV_MUTEX: std::sync::Mutex<()> = std::sync::Mutex::new(()); + + fn lock_env() -> std::sync::MutexGuard<'static, ()> { + ENV_MUTEX + .lock() + .unwrap_or_else(|poison| poison.into_inner()) + } + + fn synthetic_unit_collection() -> String { + "
1
\ +Yes0\ +Yes3\ +" + .to_string() + } + + fn synthetic_stock_item_collection() -> String { + "
1
\ +ChemicalsKgs\ +10050.005000.00\ +Applicable28362000\ +" + .to_string() + } + + fn synthetic_inventory_voucher_collection() -> String { + "
1
\ +202604051Sales\ +Fixture Acid & Chemicalsfixture-guid-1No\ +Sodium Bicarbonate55.00\ +5500.00100100Main Godown\ +\ +" + .to_string() + } + + fn synthetic_inventory_voucher_with_batch() -> String { + "
1
\ +202604062Sales\ +Test Partyfixture-guid-2No\ +Sodium Bicarbonate55.002750.00\ +Batch-01Main Godown\ +50502750.00\ +\ +" + .to_string() + } + + #[test] + fn lab_guard_config_requires_all_env_vars() { + let _guard = lock_env(); + std::env::remove_var("BRIDGE_LAB_TARGET_GUID"); + std::env::remove_var("BRIDGE_LAB_DENY_GUIDS"); + assert_eq!( + LabGuardConfig::from_env(), + Err("lab_target_guid_required".to_string()) + ); + } + + #[test] + fn lab_guard_config_rejects_target_in_its_own_deny_list() { + // Pure parsing: exercises `from_values` directly, no process env + // touched, so this is safe under parallel test threads. + let result = + LabGuardConfig::from_values(TARGET_GUID, &format!("{DENY_GUID_1},{TARGET_GUID}")); + assert_eq!(result, Err("lab_deny_guids_invalid".to_string())); + } + + #[test] + fn lab_guard_config_parses_a_valid_comma_list() { + let config = + LabGuardConfig::from_values(TARGET_GUID, &format!(" {DENY_GUID_1} , {DENY_GUID_2} ")) + .expect("valid config parses"); + assert_eq!(config.target_guid, TARGET_GUID.to_ascii_lowercase()); + assert_eq!(config.deny_guids.len(), 2); + } + + #[test] + fn lab_guard_config_from_env_reads_the_real_env_vars() { + let _guard = lock_env(); + std::env::set_var("BRIDGE_LAB_TARGET_GUID", TARGET_GUID); + std::env::set_var("BRIDGE_LAB_DENY_GUIDS", DENY_GUID_1); + let config = LabGuardConfig::from_env(); + std::env::remove_var("BRIDGE_LAB_TARGET_GUID"); + std::env::remove_var("BRIDGE_LAB_DENY_GUIDS"); + let config = config.expect("valid env parses"); + assert_eq!(config.target_guid, TARGET_GUID.to_ascii_lowercase()); + assert_eq!(config.deny_guids, vec![DENY_GUID_1.to_ascii_lowercase()]); + } + + #[test] + fn env_lab_writes_enabled_requires_exact_truthy_value() { + let _guard = lock_env(); + std::env::remove_var("BRIDGE_LAB_WRITES"); + assert!(!env_lab_writes_enabled()); + std::env::set_var("BRIDGE_LAB_WRITES", "1"); + assert!(env_lab_writes_enabled()); + std::env::set_var("BRIDGE_LAB_WRITES", "yes"); + assert!(!env_lab_writes_enabled()); + std::env::remove_var("BRIDGE_LAB_WRITES"); + } + + #[test] + fn parses_units_from_a_synthetic_collection() { + let rows = parse_lab_master_rows(&synthetic_unit_collection(), "Unit").unwrap(); + assert_eq!(rows.len(), 2); + assert_eq!(rows[0].get("NAME").unwrap(), "Nos"); + let json = lab_master_json(LabMasterKind::Unit, &rows[0]); + assert_eq!(json["decimal_places"], "0"); + } + + #[test] + fn parses_stock_item_opening_and_gst_hsn_fields() { + let rows = parse_lab_master_rows(&synthetic_stock_item_collection(), "StockItem").unwrap(); + assert_eq!(rows.len(), 1); + let json = lab_master_json(LabMasterKind::StockItem, &rows[0]); + assert_eq!(json["parent"], "Chemicals"); + assert_eq!(json["base_unit"], "Kgs"); + assert_eq!(json["opening_qty"], "100"); + assert_eq!(json["opening_rate"], "50.00"); + assert_eq!(json["opening_value"], "5000.00"); + assert_eq!(json["hsn_code"], "28362000"); + } + + #[test] + fn a_non_row_child_of_collection_is_refused() { + let xml = "
1
\ +2"; + assert_eq!( + parse_lab_master_rows(xml, "Unit"), + Err("agent_read_protocol_invalid".to_string()) + ); + } + + #[test] + fn parses_inventory_entries_per_voucher_and_honours_the_window() { + let rows = parse_lab_inventory_vouchers(&synthetic_inventory_voucher_collection()).unwrap(); + assert_eq!(rows.len(), 1); + assert_eq!(rows[0]["date"], "20260405"); + assert_eq!(rows[0]["voucher_number"], "1"); + let entries = rows[0]["inventory_entries"].as_array().unwrap(); + assert_eq!(entries.len(), 1); + assert_eq!(entries[0]["stock_item"], "Sodium Bicarbonate"); + assert_eq!(entries[0]["actual_qty"], "100"); + assert_eq!(entries[0]["godown"], "Main Godown"); + assert!(window_honoured(&rows, "20260401", "20260430")); + assert!(!window_honoured(&rows, "20260401", "20260404")); + } + + #[test] + fn parses_nested_batch_allocations_under_an_inventory_entry() { + let rows = parse_lab_inventory_vouchers(&synthetic_inventory_voucher_with_batch()).unwrap(); + assert_eq!(rows.len(), 1); + let entries = rows[0]["inventory_entries"].as_array().unwrap(); + assert_eq!(entries.len(), 1); + let batches = entries[0]["batch_allocations"].as_array().unwrap(); + assert_eq!(batches.len(), 1); + assert_eq!(batches[0]["batch"], "Batch-01"); + assert_eq!(batches[0]["godown"], "Main Godown"); + assert_eq!(batches[0]["actual_qty"], "50"); + } + + #[test] + fn a_non_voucher_child_of_collection_is_refused_for_inventory_vouchers() { + let xml = "
1
\ +2"; + assert_eq!( + parse_lab_inventory_vouchers(xml), + Err("agent_read_protocol_invalid".to_string()) + ); + } + + #[test] + fn render_lab_master_collection_carries_the_exact_company_name() { + let request = + render_lab_master_collection("BRIDGE CORPUS GST", LabMasterKind::StockItem).unwrap(); + assert!(request.contains("BRIDGE CORPUS GST")); + assert!(request.contains("StockItem")); + } +} From 5ead2ebc9252d7224a8de4bb8a560b17068560ef Mon Sep 17 00:00:00 2001 From: t Date: Mon, 14 Sep 2026 02:39:52 +0530 Subject: [PATCH 06/14] feat(lab): add post_lab_import runtime transport (lab-writes only) Audit-sprint 2026-09-14 Phase 3.4/3.5 groundwork. Adds TallyRuntime::post_lab_import, a lab-only method that posts already-built import XML directly to the Tally XML gateway through the same serialized session queue and retry policy as every other operation (post_probe_xml), but without the native-approval / durable-dispatch-ledger machinery post_approved_import layers on top for the production Journal path. That machinery is the production path's safety (one human-approved Journal at a time); the lab writer's safety is its caller's admit_lab_target re-check before every batch, not this method. Compiled only behind the lab-writes feature. Does not modify agent_import_post.rs or tally/approved_import.rs. Co-Authored-By: Claude Sonnet --- src-tauri/src/tally/runtime.rs | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/src-tauri/src/tally/runtime.rs b/src-tauri/src/tally/runtime.rs index 5154425a..e35fb68c 100644 --- a/src-tauri/src/tally/runtime.rs +++ b/src-tauri/src/tally/runtime.rs @@ -2304,6 +2304,39 @@ impl TallyRuntime { .await } + /// LAB-ONLY (audit-sprint 2026-09-14, Phase 3.4/3.5). Posts already-built + /// import XML directly to the Tally XML gateway through the same + /// serialized session queue as every other operation, but without the + /// native-approval / durable-dispatch-ledger machinery + /// `post_approved_import` layers on top for the production Journal path. + /// That machinery *is* the production path's safety (one human-approved + /// Journal at a time); the lab writer's safety is its caller's + /// `admit_lab_target` re-check before every batch, not this method. + /// Compiled only behind `lab-writes`; never called from, and never + /// changes, `agent_import_post.rs` or `approved_import.rs`. + #[cfg(feature = "lab-writes")] + pub(crate) async fn post_lab_import( + &self, + config: TallyConfig, + xml: String, + ) -> anyhow::Result<(String, RuntimeReadEvidence)> { + let _lease = self.begin_ordinary_read(&config)?; + self.execute( + config, + ReadOperation::Import, + ReadRetryPolicy::SINGLE_ATTEMPT, + move |client| { + let xml = xml.clone(); + async move { + let mut evidence = RuntimeReadEvidence::empty(); + let body = client.post_probe_xml(xml, &mut evidence).await?; + Ok((body, evidence)) + } + }, + ) + .await + } + /// Outstandings via Tally's own `TYPE=Data` bills reports plus one ledger /// snapshot. /// From a978fe4c9374c352b47d23859dff3deef8e17d24 Mon Sep 17 00:00:00 2001 From: t Date: Mon, 14 Sep 2026 02:40:09 +0530 Subject: [PATCH 07/14] feat(lab): lab_import_masters and lab_import_vouchers (Phase 3.4/3.5) Audit-sprint 2026-09-14. Two lab-only write tools, feature+env gated the same way as lab_read_inventory, both re-verifying admit_lab_target's loaded-company/deny-list/target-identity guard before every batch: - lab_import_masters: creates units, godowns, stock groups, groups, ledgers, stock items (in that plan order) from the book model's masters section. Refuses before any write if the target already carries a same-name master under any requested kind (the Create-overwrite trap, TALLY_PROTOCOL_ REFERENCE.md Sec 9.4), using the master-name fold measured on licensed TallyPrime 7.1 (Sec 9.4d). Batches of <=200; every batch is read back field-by-field (name, parent, opening balance/qty, GST fields) and the call stops on the first mismatch. - lab_import_vouchers: creates Journal/Payment/Receipt/Contra (own renderer, not agent_import.rs's -- see module doc for why) plus accounting- and invoice-mode Sales/Purchase/Credit Note/Debit Note, sorted by date and batched at <=100. Before sending a batch, reads its date window back and matches every voucher by type/date/ledger-amount plus a narration marker or voucher number; a fully-matched batch is skipped (resume), a partial match stops rather than guessing, only an unmatched batch is sent. Every sent batch is read back the same way; stops on the first mismatch. Group/Unit/Godown/StockGroup/StockItem master XML and every invoice type but Sales are UNVERIFIED for the gateway (no live capture in this repository) -- documented in the module's own doc comment and this worker's receipt/report. Unit tests (agent_lab_import_tests.rs, 33 tests): golden XML fixtures for every master kind and every voucher shape, the Sec 9.4d name-fold (including the negative cases: en dash/underscore not folded, NFC/NFD not normalised), read-back mismatch detection, resume matching (marker, voucher number, cancelled-voucher exclusion, content-only rejection per Sec 9.3), and the book-model parser (inline JSON and book_path file). cargo test --features lab-writes --lib: 962 passed, 0 failed. clippy -D warnings clean both feature states. cargo fmt clean. Release bridge_mcp --features lab-writes built; sha256 recorded in the worker receipt. Does not modify agent_import_post.rs or tally/approved_import.rs (diff against origin/master is empty). Co-Authored-By: Claude Sonnet --- src-tauri/src/agent.rs | 9 +- src-tauri/src/agent_catalog.rs | 22 +- src-tauri/src/agent_lab.rs | 10 + src-tauri/src/agent_lab_import.rs | 1429 +++++++++++++++++++++++ src-tauri/src/agent_lab_import_tests.rs | 710 +++++++++++ 5 files changed, 2178 insertions(+), 2 deletions(-) create mode 100644 src-tauri/src/agent_lab_import.rs create mode 100644 src-tauri/src/agent_lab_import_tests.rs diff --git a/src-tauri/src/agent.rs b/src-tauri/src/agent.rs index e115b0df..73b730d7 100644 --- a/src-tauri/src/agent.rs +++ b/src-tauri/src/agent.rs @@ -648,7 +648,10 @@ impl Server { return Err("import_posting_disabled".to_string().into()); } #[cfg(feature = "lab-writes")] - if name == "lab_read_inventory" { + if matches!( + name, + "lab_read_inventory" | "lab_import_masters" | "lab_import_vouchers" + ) { lab::require_lab_writes_env()?; } validate_tool_arguments(name, args)?; @@ -693,6 +696,10 @@ impl Server { "egress_log" => self.egress_log(args).map_err(Into::into), #[cfg(feature = "lab-writes")] "lab_read_inventory" => lab::lab_read_inventory(self, args).await, + #[cfg(feature = "lab-writes")] + "lab_import_masters" => lab::lab_import_masters(self, args).await, + #[cfg(feature = "lab-writes")] + "lab_import_vouchers" => lab::lab_import_vouchers(self, args).await, _ => Err("tool_not_found".to_string().into()), } } diff --git a/src-tauri/src/agent_catalog.rs b/src-tauri/src/agent_catalog.rs index 1e00c6cd..830e3490 100644 --- a/src-tauri/src/agent_catalog.rs +++ b/src-tauri/src/agent_catalog.rs @@ -145,6 +145,10 @@ pub(super) fn registered_tool_definitions(import_enabled: bool, writes_enabled: ]; #[cfg(feature = "lab-writes")] names.push("lab_read_inventory"); + #[cfg(feature = "lab-writes")] + names.push("lab_import_masters"); + #[cfg(feature = "lab-writes")] + names.push("lab_import_vouchers"); Value::Array( names .into_iter() @@ -156,7 +160,12 @@ pub(super) fn registered_tool_definitions(import_enabled: bool, writes_enabled: // LAB-ONLY: registered only when the `lab-writes` feature is // compiled in AND `BRIDGE_LAB_WRITES=1` is set (checked fresh on // every catalog build, not cached at startup). - .filter(|name| *name != "lab_read_inventory" || lab_tools_env_enabled()) + .filter(|name| { + !matches!( + name, + &"lab_read_inventory" | &"lab_import_masters" | &"lab_import_vouchers" + ) || lab_tools_env_enabled() + }) .map(|name| { let (description, input_schema) = match name { "voucher_schema" => ( @@ -219,6 +228,14 @@ pub(super) fn registered_tool_definitions(import_enabled: bool, writes_enabled: "LAB-ONLY. Compiled only behind the `lab-writes` feature and refuses unless BRIDGE_LAB_WRITES=1, BRIDGE_TALLY_PORT=9001, and BRIDGE_LAB_TARGET_GUID/BRIDGE_LAB_DENY_GUIDS are both set to well-formed GUIDs. This is a read: company_guid selects the company like any other read tool and is verified the same way (`company_identity_not_found`/`company_identity_ambiguous`), independent of the configured lab target -- the stronger loaded-company/deny-list guard applies only to a lab write batch, not a read. Read-only: units, godowns, stock groups and stock items (parent, base unit, opening qty/rate/value, GST/HSN fields as returned, unclassified), plus inventory entries per voucher for a date window. Reuses the same windowing and window_honoured corroboration as `vouchers`. No signed compatibility evidence exists yet for any inventory field on this Tally release/mode -- treat every value as exploratory.", json!({"type":"object","additionalProperties":false,"required":["company_guid","from","to"],"properties":{"company_guid":{"type":"string","minLength":1},"from":{"type":"string","pattern":"^[0-9]{4}-?[0-9]{2}-?[0-9]{2}$"},"to":{"type":"string","pattern":"^[0-9]{4}-?[0-9]{2}-?[0-9]{2}$"},"offset":{"type":"integer","minimum":0,"default":0},"limit":{"type":"integer","minimum":1,"default":500}}}), ), + "lab_import_masters" => ( + "LAB-ONLY (Phase 3.4). Compiled only behind `lab-writes`; refuses unless BRIDGE_LAB_WRITES=1, BRIDGE_TALLY_PORT=9001, and BRIDGE_LAB_TARGET_GUID/BRIDGE_LAB_DENY_GUIDS are set. Creates masters (units, godowns, stock groups, groups, ledgers, stock items, in that order) from the book model's `masters` section (inline `masters` or a `book_path` local JSON file). Re-verifies the loaded-company/deny-list/target-identity guard before every batch (<=200 masters). Refuses before any write if the target already carries a same-name master under any requested kind (the Create-overwrite trap, §9.4) -- `lab_master_already_exists`. Every batch is read back field-by-field (name, parent, opening balance/qty, GST fields) and the whole call stops on the first mismatch; never trusts CREATED/ERRORS alone. Group/Unit/Godown/StockGroup/StockItem XML shapes have no live capture in this repository and are UNVERIFIED for the gateway -- see the tool's module documentation.", + json!({"type":"object","additionalProperties":false,"required":["company_guid"],"properties":{"company_guid":{"type":"string","minLength":1},"masters":{"type":"object"},"book_path":{"type":"string","minLength":1}}}), + ), + "lab_import_vouchers" => ( + "LAB-ONLY (Phase 3.5). Compiled only behind `lab-writes`; refuses unless BRIDGE_LAB_WRITES=1, BRIDGE_TALLY_PORT=9001, and BRIDGE_LAB_TARGET_GUID/BRIDGE_LAB_DENY_GUIDS are set. Creates vouchers (Journal/Payment/Receipt/Contra plus accounting- and invoice-mode Sales/Purchase/Credit Note/Debit Note) from the book model's `vouchers` section (inline `vouchers` or a `book_path` local JSON file), sorted by date and posted in batches of at most 100. Re-verifies the loaded-company/deny-list/target-identity guard before every batch. Before sending a batch, reads its date window back and checks every voucher against a narration-marker/voucher-number plus type/date/ledger-amount fingerprint: a fully-matched batch is skipped (resume), a partially-matched batch stops with `lab_batch_partially_verified_uncertain` rather than guessing, and only an unmatched batch is sent. Every sent batch is read back the same way and the whole call stops on the first mismatch. Invoice-mode XML (`LEDGERENTRIES.LIST`/`ALLINVENTORYENTRIES.LIST`) and every type but Sales/Journal/Payment/Receipt/Contra are UNVERIFIED for the gateway -- see the tool's module documentation. `start_batch` resumes a prior call.", + json!({"type":"object","additionalProperties":false,"required":["company_guid"],"properties":{"company_guid":{"type":"string","minLength":1},"vouchers":{"type":"array"},"book_path":{"type":"string","minLength":1},"start_batch":{"type":"integer","minimum":0,"default":0}}}), + ), _ => ( "Bridge read-only Tally tool", json!({"type":"object", "additionalProperties": false}), @@ -231,6 +248,9 @@ pub(super) fn registered_tool_definitions(import_enabled: bool, writes_enabled: if name == "lab_read_inventory" { tool["annotations"] = json!({"readOnlyHint":true,"destructiveHint":false,"idempotentHint":true,"openWorldHint":true}); } + if matches!(name, "lab_import_masters" | "lab_import_vouchers") { + tool["annotations"] = json!({"readOnlyHint":false,"destructiveHint":true,"idempotentHint":false,"openWorldHint":true}); + } tool }) .collect(), diff --git a/src-tauri/src/agent_lab.rs b/src-tauri/src/agent_lab.rs index e5d63ce8..a65214bb 100644 --- a/src-tauri/src/agent_lab.rs +++ b/src-tauri/src/agent_lab.rs @@ -20,6 +20,16 @@ use std::collections::BTreeMap; use std::fs; use std::path::PathBuf; +// Phase 3.4/3.5: the lab writer tools (`lab_import_masters`, +// `lab_import_vouchers`). Kept in its own file for size; reuses this +// module's guard/evidence machinery (`admit_lab_target`, `lab_post_read`, +// `persist_lab_exchange`, `lab_evidence_dir`, `parse_lab_master_rows`) via +// the same private-item-visible-to-descendant-module path this file itself +// uses for `agent.rs`'s items. +#[path = "agent_lab_import.rs"] +mod import; +pub(super) use import::{lab_import_masters, lab_import_vouchers}; + // --------------------------------------------------------------------------- // Env gates // --------------------------------------------------------------------------- diff --git a/src-tauri/src/agent_lab_import.rs b/src-tauri/src/agent_lab_import.rs new file mode 100644 index 00000000..6c3e1fcd --- /dev/null +++ b/src-tauri/src/agent_lab_import.rs @@ -0,0 +1,1429 @@ +//! LAB-ONLY write surface (audit-sprint 2026-09-14, Phase 3.4/3.5). +//! +//! Compiled only behind `lab-writes` (via `agent_lab.rs`'s `mod import` +//! declaration); every tool here additionally refuses at runtime unless +//! `BRIDGE_LAB_WRITES=1` -- checked by the caller in `agent.rs`, the same +//! gate `lab_read_inventory` uses. Every batch calls [`admit_lab_target`] +//! immediately before it is sent, so the loaded-company / deny-list / +//! target-identity guard is re-verified on every single write, not once per +//! tool call, matching the plan's "admits target before every batch" +//! requirement. +//! +//! Input is the book model documented in +//! `brain/50-projects/audit-sprint-2026-09-14/specs/book_schema.md`, built by +//! `SP/code/book/build_book.py`. This module never reads a snapshot itself. +//! +//! **What is reused, and what is not, and why:** +//! - [`bridge_tally_protocol::parse_import_outcome`] parses every +//! `` (§9.1/§9.2) -- genuinely shared code, not duplicated. +//! - The master-name matching predicate ([`canonical_master_key`]) is the +//! composed fold measured on **licensed TallyPrime 7.1** in §9.4d (space, +//! `-` and `/` interchangeable, internal runs collapsed, surrounding +//! whitespace ignored, ASCII case folded, otherwise exact codepoints; NFC/ +//! NFD is deliberately NOT folded -- §9.4d's own "canonical equivalence is +//! still refused" finding). §9.4d's measurement scope is *ledgers, one +//! company*; applying the same fold to every other master kind here is a +//! deliberate conservative choice for a pre-*write* collision check (a +//! false positive only makes this tool over-refuse, which is the safe +//! failure direction for the Create-overwrite trap, §9.4) -- it is not a +//! claim that Tally folds group/unit/godown/stock-item names the same way. +//! - The XML **shapes** for Payment/Receipt/Contra (§9.13: `EFFECTIVEDATE`, +//! `PARTYLEDGERNAME` on the counterparty side, Dr-first ordering) and for +//! invoice-mode Sales (§9.12a: `LEDGERENTRIES.LIST` + `ISINVOICE=Yes` + +//! `ALLINVENTORYENTRIES.LIST`) are reused byte-for-byte against the +//! documented captures. What is **not** reused is `agent_import.rs`'s +//! `render_voucher_xml` function itself: its `ImportEntry` carries no +//! `BILLALLOCATIONS.LIST`, and every voucher type this book model writes -- +//! Payment/Receipt/Contra included, per the rehearsal book -- can carry +//! bill allocations against a bill-wise party. Reusing that function +//! unmodified would silently drop them, which is exactly the class of +//! defect §9.2/§12a.4 exist to catch. So this module renders its own +//! entries from the proven wire shape rather than the Rust function. +//! - Group/Unit/Godown/StockGroup/StockItem **master** XML and +//! Purchase/Credit-Note/Debit-Note **invoice** XML have no live capture +//! anywhere in this repository's protocol reference. They are built from +//! Tally's well-documented standard master schema and from §9.12a's +//! invoice shape generalised across voucher types (a hypothesis §9.12 +//! explicitly says is untested for anything but Sales). **Both are +//! UNVERIFIED for the gateway import path and need the one-voucher / +//! one-master live probe §9.4/§9.12 themselves prescribe before a real +//! batch** -- see this worker's final report. + +use super::*; +use bridge_tally_core::ExactDecimal; +use serde::Deserialize; +use std::collections::BTreeMap; +use uuid::Uuid; + +const MAX_MASTER_BATCH: usize = 200; +const MAX_VOUCHER_BATCH: usize = 100; + +// --------------------------------------------------------------------------- +// Master-name matching (§9.4d, licensed TallyPrime 7.1) -- see module doc. +// --------------------------------------------------------------------------- + +/// The composed fold §9.4d measured on licensed TallyPrime 7.1: ASCII case +/// folded, `-`/`/` treated as a space, internal whitespace runs collapsed to +/// one, surrounding whitespace trimmed. Deliberately does **not** apply +/// Unicode normalisation (NFC/NFD) -- §9.4d's own finding is that Tally +/// refuses that fold. +fn canonical_master_key(name: &str) -> String { + let mut out = String::with_capacity(name.len()); + let mut last_was_space = true; // trims leading whitespace for free + for ch in name.chars() { + let mapped = match ch { + '-' | '/' => ' ', + other => other, + }; + if mapped.is_whitespace() { + if !last_was_space { + out.push(' '); + } + last_was_space = true; + } else { + out.extend(mapped.to_lowercase()); + last_was_space = false; + } + } + while out.ends_with(' ') { + out.pop(); + } + out +} + +/// The `company_guid` the caller supplied must be the admitted lab target's +/// own GUID -- `admit_lab_target` already proved *a* target is loaded and +/// unique; this closes the separate hole of a caller passing a different +/// (e.g. stale) GUID than the one that was just admitted. +fn identity_matches_requested_guid(identity: &VerifiedCompanyIdentity, guid: &str) -> bool { + identity.company_guid().eq_ignore_ascii_case(guid) +} + +fn amounts_equal(a: &str, b: &str) -> bool { + match (ExactDecimal::parse(a), ExactDecimal::parse(b)) { + // `numeric_eq`, not `==`: ExactDecimal's derived equality is on its + // stored lexeme, so "0" and "0.00" would otherwise compare unequal. + (Ok(a), Ok(b)) => a.numeric_eq(&b), + _ => a.trim() == b.trim(), + } +} + +// --------------------------------------------------------------------------- +// Book model (input) -- see SP/specs/book_schema.md for the full schema. +// --------------------------------------------------------------------------- + +#[derive(Debug, Clone, Deserialize, Default)] +struct BookMasters { + #[serde(default)] + units: Vec, + #[serde(default)] + godowns: Vec, + #[serde(default)] + stock_groups: Vec, + #[serde(default)] + groups: Vec, + #[serde(default)] + ledgers: Vec, + #[serde(default)] + stock_items: Vec, +} + +#[derive(Debug, Clone, Deserialize)] +struct BookUnit { + name: String, + #[serde(default)] + is_simple_unit: Option, + #[serde(default)] + decimal_places: Option, +} + +#[derive(Debug, Clone, Deserialize)] +struct BookNamedParent { + name: String, + #[serde(default)] + parent: Option, +} + +#[derive(Debug, Clone, Deserialize)] +struct BookLedger { + name: String, + #[serde(default)] + parent: Option, + #[serde(default)] + opening_balance: Option, + #[serde(default)] + is_billwise_on: Option, + #[serde(default)] + party_gstin: Option, + #[serde(default)] + tax_type: Option, + #[serde(default)] + gst_duty_head: Option, + #[serde(default)] + opening_bill_allocations: Vec, +} + +#[derive(Debug, Clone, Deserialize)] +struct BookBillAllocation { + #[serde(default)] + name: Option, + bill_type: String, + amount: String, +} + +#[derive(Debug, Clone, Deserialize)] +struct BookStockItem { + name: String, + #[serde(default)] + parent: Option, + #[serde(default)] + base_unit: Option, + #[serde(default)] + opening_qty: Option, + #[serde(default)] + opening_rate: Option, + #[serde(default)] + opening_value: Option, + #[serde(default)] + gst_applicable: Option, + #[serde(default)] + hsn_code: Option, +} + +#[derive(Debug, Clone, Deserialize)] +struct BookVoucher { + source_guid: String, + #[serde(rename = "type")] + voucher_type: String, + date: String, + #[serde(default)] + voucher_number: Option, + #[serde(default)] + narration: Option, + #[serde(default)] + party: Option, + #[serde(default)] + is_invoice_mode: bool, + ledger_lines: Vec, + #[serde(default)] + inventory_lines: Vec, +} + +#[derive(Debug, Clone, Deserialize)] +struct BookLedgerLine { + ledger: String, + side: String, + amount: String, + #[serde(default)] + bill_allocations: Vec, +} + +#[derive(Debug, Clone, Deserialize)] +struct BookInventoryLine { + #[serde(default)] + stock_item: Option, + #[serde(default)] + rate: Option, + #[serde(default)] + qty: Option, + #[serde(default)] + billed_qty: Option, + #[serde(default)] + amount: Option, + #[serde(default)] + godown: Option, + #[serde(default)] + accounting_allocations: Vec, + #[serde(default)] + batch_allocations: Vec, +} + +#[derive(Debug, Clone, Deserialize)] +struct BookAccountingAllocation { + ledger: String, + amount: String, +} + +fn parse_book_value Deserialize<'de>>( + args: &Value, + inline_key: &str, + section_key: &str, +) -> Result { + if let Some(inline) = args.get(inline_key) { + return serde_json::from_value(inline.clone()) + .map_err(|_| ToolFailure::from(format!("{inline_key}_invalid"))); + } + let path = args + .get("book_path") + .and_then(Value::as_str) + .ok_or_else(|| ToolFailure::from(format!("{inline_key}_or_book_path_required")))?; + let text = fs::read_to_string(path) + .map_err(|_| ToolFailure::from("book_path_unreadable".to_string()))?; + let whole: Value = serde_json::from_str(&text) + .map_err(|_| ToolFailure::from("book_path_invalid_json".to_string()))?; + let section = whole.get(section_key).cloned().unwrap_or(whole); + serde_json::from_value(section).map_err(|_| ToolFailure::from(format!("{inline_key}_invalid"))) +} + +// --------------------------------------------------------------------------- +// Master kinds, in the plan's required import order. +// --------------------------------------------------------------------------- + +#[derive(Clone, Copy, PartialEq, Eq, Debug)] +enum MasterKind { + Unit, + Godown, + StockGroup, + Group, + Ledger, + StockItem, +} + +impl MasterKind { + const IMPORT_ORDER: [Self; 6] = [ + Self::Unit, + Self::Godown, + Self::StockGroup, + Self::Group, + Self::Ledger, + Self::StockItem, + ]; + + fn tally_type(self) -> &'static str { + match self { + Self::Unit => "Unit", + Self::Godown => "Godown", + Self::StockGroup => "StockGroup", + Self::Group => "Group", + Self::Ledger => "Ledger", + Self::StockItem => "StockItem", + } + } + + fn readback_fetch_fields(self) -> &'static str { + match self { + Self::Unit => "NAME,ISSIMPLEUNIT,DECIMALPLACES,GUID,MASTERID,ALTERID", + Self::Godown | Self::StockGroup | Self::Group => { + "NAME,PARENT,RESERVEDNAME,GUID,MASTERID,ALTERID" + } + Self::Ledger => { + "NAME,PARENT,OPENINGBALANCE,ISBILLWISEON,PARTYGSTIN,TAXTYPE,GSTDUTYHEAD,\ + GUID,MASTERID,ALTERID" + } + Self::StockItem => { + "NAME,PARENT,BASEUNITS,OPENINGBALANCE,OPENINGRATE,OPENINGVALUE,\ + GSTAPPLICABLE,HSNCODE,GUID,MASTERID,ALTERID" + } + } + } + + fn names(self, masters: &BookMasters) -> Vec { + match self { + Self::Unit => masters.units.iter().map(|u| u.name.clone()).collect(), + Self::Godown => masters.godowns.iter().map(|g| g.name.clone()).collect(), + Self::StockGroup => masters + .stock_groups + .iter() + .map(|g| g.name.clone()) + .collect(), + Self::Group => masters.groups.iter().map(|g| g.name.clone()).collect(), + Self::Ledger => masters.ledgers.iter().map(|l| l.name.clone()).collect(), + Self::StockItem => masters.stock_items.iter().map(|s| s.name.clone()).collect(), + } + } + + fn count(self, masters: &BookMasters) -> usize { + self.names(masters).len() + } +} + +fn render_master_collection_request(company: &str, kind: MasterKind) -> Result { + let company = ValidatedCompanyName::new(company.to_string()) + .map_err(|_| "company_name_invalid".to_string())?; + let object_type = kind.tally_type(); + let name = format!("Bridge Lab Write {object_type}s"); + Ok(format!( + r#"
1ExportCollection{name}
$$SysName:XML{}{object_type}{}"#, + xml_escape(company.as_str()), + kind.readback_fetch_fields() + )) +} + +fn find_readback_row<'a>( + rows: &'a [BTreeMap], + name: &str, +) -> Option<&'a BTreeMap> { + let key = canonical_master_key(name); + rows.iter() + .find(|row| canonical_master_key(row.get("NAME").map(String::as_str).unwrap_or("")) == key) +} + +// --------------------------------------------------------------------------- +// Master XML renderers (Create). See module doc: UNVERIFIED for the gateway +// on every kind except the fields §8.3/§9.4a already qualify for Ledger. +// --------------------------------------------------------------------------- + +fn render_import_envelope(company: &str, report_name: &str, messages: &str) -> String { + format!( + "
Import Data
{report_name}{}{messages}", + xml_escape(company) + ) +} + +fn render_unit_xml(u: &BookUnit) -> String { + let simple = u.is_simple_unit.as_deref().unwrap_or("Yes"); + let decimals = u.decimal_places.as_deref().unwrap_or("2"); + format!( + "{simple}{decimals}", + name = xml_escape(&u.name), + simple = xml_escape(simple), + decimals = xml_escape(decimals) + ) +} + +fn render_parented_xml(tag: &str, item: &BookNamedParent) -> String { + let parent = item.parent.as_deref().unwrap_or("Primary"); + format!( + "<{tag} NAME=\"{name}\" ACTION=\"Create\">{parent}", + tag = tag, + name = xml_escape(&item.name), + parent = xml_escape(parent) + ) +} + +fn render_ledger_xml(l: &BookLedger) -> String { + let parent = l.parent.as_deref().unwrap_or("Primary"); + let opening = l.opening_balance.as_deref().unwrap_or("0.00"); + let billwise = l + .is_billwise_on + .map(|b| { + format!( + "{}", + if b { "Yes" } else { "No" } + ) + }) + .unwrap_or_default(); + // GST fields are passed through exactly as observed on the source ledger + // (§8.3: `GSTDUTYHEAD` vocabulary is irregular, `State Tax` not `SGST`); + // never synthesised. §8.3: settable at Create, silently not at Alter -- + // this renderer only ever builds a Create. + let gstin = l + .party_gstin + .as_deref() + .map(|g| format!("{}", xml_escape(g))) + .unwrap_or_default(); + let tax_type = l + .tax_type + .as_deref() + .map(|t| format!("{}", xml_escape(t))) + .unwrap_or_default(); + let duty_head = l + .gst_duty_head + .as_deref() + .map(|d| format!("{}", xml_escape(d))) + .unwrap_or_default(); + // Opening bill-wise allocations, when the source captured them, nested + // under the ledger master the same way an accounting voucher's + // BILLALLOCATIONS.LIST nests under its ledger entry (§9.4a family) -- + // this specific master-level placement has no live capture in this + // repository and is UNVERIFIED for the gateway; see module doc. + let opening_bills = l + .opening_bill_allocations + .iter() + .map(|b| { + let name = b.name.clone().unwrap_or_default(); + format!( + "{}{}{}", + xml_escape(&name), xml_escape(&b.bill_type), xml_escape(&b.amount) + ) + }) + .collect::(); + format!( + "{parent}{opening}{billwise}{gstin}{tax_type}{duty_head}{opening_bills}", + name = xml_escape(&l.name), + parent = xml_escape(parent), + opening = xml_escape(opening) + ) +} + +fn render_stock_item_xml(s: &BookStockItem) -> String { + let parent = s.parent.as_deref().unwrap_or("Primary"); + let base_units = s + .base_unit + .as_deref() + .map(|u| format!("{}", xml_escape(u))) + .unwrap_or_default(); + let opening = match ( + s.opening_qty.as_deref(), + s.opening_rate.as_deref(), + s.opening_value.as_deref(), + ) { + (Some(qty), Some(rate), Some(value)) => format!( + "{}{}{}", + xml_escape(qty), xml_escape(rate), xml_escape(value) + ), + _ => String::new(), + }; + let gst = s + .gst_applicable + .as_deref() + .map(|g| format!("{}", xml_escape(g))) + .unwrap_or_default(); + let hsn = s + .hsn_code + .as_deref() + .map(|h| format!("{}", xml_escape(h))) + .unwrap_or_default(); + format!( + "{parent}{base_units}{opening}{gst}{hsn}", + name = xml_escape(&s.name), + parent = xml_escape(parent) + ) +} + +fn render_master_batch_xml(company: &str, kind: MasterKind, masters: &BookMasters) -> String { + let messages = match kind { + MasterKind::Unit => masters + .units + .iter() + .map(render_unit_xml) + .collect::(), + MasterKind::Godown => masters + .godowns + .iter() + .map(|g| render_parented_xml("GODOWN", g)) + .collect::(), + MasterKind::StockGroup => masters + .stock_groups + .iter() + .map(|g| render_parented_xml("STOCKGROUP", g)) + .collect::(), + MasterKind::Group => masters + .groups + .iter() + .map(|g| render_parented_xml("GROUP", g)) + .collect::(), + MasterKind::Ledger => masters + .ledgers + .iter() + .map(render_ledger_xml) + .collect::(), + MasterKind::StockItem => masters + .stock_items + .iter() + .map(render_stock_item_xml) + .collect::(), + }; + render_import_envelope(company, "All Masters", &messages) +} + +// --------------------------------------------------------------------------- +// Master read-back diff +// --------------------------------------------------------------------------- + +fn diff_unit(u: &BookUnit, row: &BTreeMap) -> Vec { + let mut mismatches = Vec::new(); + if let Some(expected) = u.decimal_places.as_deref() { + let observed = row.get("DECIMALPLACES").map(String::as_str).unwrap_or(""); + if expected != observed { + mismatches.push(format!( + "unit {}: decimal_places expected {expected}, observed {observed}", + u.name + )); + } + } + mismatches +} + +fn diff_parented(tag: &str, item: &BookNamedParent, row: &BTreeMap) -> Vec { + let mut mismatches = Vec::new(); + if let Some(expected) = item.parent.as_deref() { + let observed = row.get("PARENT").map(String::as_str).unwrap_or(""); + if canonical_master_key(expected) != canonical_master_key(observed) { + mismatches.push(format!( + "{tag} {}: parent expected {expected:?}, observed {observed:?}", + item.name + )); + } + } + mismatches +} + +fn diff_ledger(l: &BookLedger, row: &BTreeMap) -> Vec { + let mut mismatches = Vec::new(); + if let Some(expected) = l.parent.as_deref() { + let observed = row.get("PARENT").map(String::as_str).unwrap_or(""); + if canonical_master_key(expected) != canonical_master_key(observed) { + mismatches.push(format!( + "ledger {}: parent expected {expected:?}, observed {observed:?}", + l.name + )); + } + } + let expected_opening = l.opening_balance.as_deref().unwrap_or("0.00"); + let observed_opening = row.get("OPENINGBALANCE").map(String::as_str).unwrap_or(""); + if !amounts_equal(expected_opening, observed_opening) { + mismatches.push(format!( + "ledger {}: opening_balance expected {expected_opening}, observed {observed_opening:?}", + l.name + )); + } + if let Some(expected) = l.party_gstin.as_deref() { + let observed = row.get("PARTYGSTIN").map(String::as_str).unwrap_or(""); + if expected != observed { + mismatches.push(format!( + "ledger {}: party_gstin expected {expected:?}, observed {observed:?}", + l.name + )); + } + } + mismatches +} + +fn diff_stock_item(s: &BookStockItem, row: &BTreeMap) -> Vec { + let mut mismatches = Vec::new(); + if let Some(expected) = s.parent.as_deref() { + let observed = row.get("PARENT").map(String::as_str).unwrap_or(""); + if canonical_master_key(expected) != canonical_master_key(observed) { + mismatches.push(format!( + "stock item {}: parent expected {expected:?}, observed {observed:?}", + s.name + )); + } + } + if let (Some(qty), Some(observed)) = (s.opening_qty.as_deref(), row.get("OPENINGBALANCE")) { + if !amounts_equal(qty, observed) { + mismatches.push(format!( + "stock item {}: opening_qty expected {qty}, observed {observed}", + s.name + )); + } + } + mismatches +} + +// --------------------------------------------------------------------------- +// lab_import_masters +// --------------------------------------------------------------------------- + +pub(in crate::agent) async fn lab_import_masters( + server: &Server, + args: &Value, +) -> Result { + let masters: BookMasters = parse_book_value(args, "masters", "masters")?; + let guid = required_string(args, "company_guid")?; + + let (_company, identity, mut evidence) = admit_lab_target(server).await?; + if !identity_matches_requested_guid(&identity, guid) { + return Err(ToolFailure::from("lab_target_company_mismatch".to_string()) + .with_prior_evidence(evidence)); + } + + // ---- Create-overwrite pre-check (§9.4): refuse before any write if the + // target already carries a same-name master under ANY kind requested. ---- + let mut collisions: Vec = Vec::new(); + for kind in MasterKind::IMPORT_ORDER { + let requested = kind.names(&masters); + if requested.is_empty() { + continue; + } + let request = render_master_collection_request(identity.display_name(), kind) + .map_err(ToolFailure::from)?; + let (xml, read_evidence) = + lab_post_read(server, &identity, "lab_import_masters.precheck", request).await?; + evidence = combine_evidence(evidence.clone(), read_evidence); + let rows = parse_lab_master_rows(&xml, kind.tally_type()) + .map_err(|code| ToolFailure::from(code).with_prior_evidence(evidence.clone()))?; + for name in &requested { + if find_readback_row(&rows, name).is_some() { + collisions.push(format!("{}:{name}", kind.tally_type())); + } + } + } + if !collisions.is_empty() { + persist_lab_precheck_collisions(server, &collisions); + return Err(ToolFailure::from("lab_master_already_exists".to_string()) + .with_prior_evidence(evidence)); + } + + let mut batches = Vec::new(); + let mut mismatches: Vec = Vec::new(); + let mut counts = serde_json::Map::new(); + + 'kinds: for kind in MasterKind::IMPORT_ORDER { + let total = kind.count(&masters); + if total == 0 { + continue; + } + let mut created = 0usize; + let mut chunk_start = 0usize; + while chunk_start < total { + // Re-admit before every batch, not just once per tool call. + let (_company, identity, admit_evidence) = admit_lab_target(server).await?; + evidence = combine_evidence(evidence.clone(), admit_evidence); + + let chunk_masters = chunked_masters(&masters, kind, chunk_start, MAX_MASTER_BATCH); + let chunk_len = kind.count(&chunk_masters); + let xml = render_master_batch_xml(identity.display_name(), kind, &chunk_masters); + let (response, post_evidence) = + post_lab_batch(server, &identity, "lab_import_masters.write", xml).await?; + evidence = combine_evidence(evidence.clone(), post_evidence); + let outcome = bridge_tally_protocol::parse_import_outcome(&response) + .map_err(|_| ToolFailure::from("lab_import_response_invalid".to_string()))?; + let clean = outcome + .counters() + .is_clean_success_for(chunk_len as u64, 0, 0); + + // Mandatory read-back, regardless of the counters (§9.2: never + // trust CREATED/ERRORS alone). + let read_request = render_master_collection_request(identity.display_name(), kind) + .map_err(ToolFailure::from)?; + let (read_xml, read_evidence) = lab_post_read( + server, + &identity, + "lab_import_masters.readback", + read_request, + ) + .await?; + evidence = combine_evidence(evidence.clone(), read_evidence); + let rows = parse_lab_master_rows(&read_xml, kind.tally_type()) + .map_err(|code| ToolFailure::from(code).with_prior_evidence(evidence.clone()))?; + + let batch_mismatches = readback_mismatches(kind, &chunk_masters, &rows); + let batch_ok = clean && batch_mismatches.is_empty(); + batches.push(json!({ + "kind": kind.tally_type(), + "requested": chunk_len, + "counters_clean": clean, + "mismatches": batch_mismatches, + "ok": batch_ok, + })); + if !batch_ok { + mismatches.extend(batch_mismatches); + break 'kinds; // stop on first mismatch, per the plan + } + created += chunk_len; + chunk_start += MAX_MASTER_BATCH; + } + counts.insert(kind.tally_type().to_string(), json!(created)); + } + + let ok = mismatches.is_empty(); + Ok(ToolOutcome { + payload: json!({"result": { + "ok": ok, + "counts": counts, + "batches": batches, + "mismatches": mismatches, + }}), + evidence, + company_guid: Some(guid.to_string()), + truncated: false, + }) +} + +fn readback_mismatches( + kind: MasterKind, + chunk: &BookMasters, + rows: &[BTreeMap], +) -> Vec { + let mut mismatches = Vec::new(); + match kind { + MasterKind::Unit => { + for item in &chunk.units { + match find_readback_row(rows, &item.name) { + None => mismatches.push(format!("unit {} not found on readback", item.name)), + Some(row) => mismatches.extend(diff_unit(item, row)), + } + } + } + MasterKind::Godown => { + for item in &chunk.godowns { + match find_readback_row(rows, &item.name) { + None => mismatches.push(format!("godown {} not found on readback", item.name)), + Some(row) => mismatches.extend(diff_parented("godown", item, row)), + } + } + } + MasterKind::StockGroup => { + for item in &chunk.stock_groups { + match find_readback_row(rows, &item.name) { + None => { + mismatches.push(format!("stock group {} not found on readback", item.name)) + } + Some(row) => mismatches.extend(diff_parented("stock group", item, row)), + } + } + } + MasterKind::Group => { + for item in &chunk.groups { + match find_readback_row(rows, &item.name) { + None => mismatches.push(format!("group {} not found on readback", item.name)), + Some(row) => mismatches.extend(diff_parented("group", item, row)), + } + } + } + MasterKind::Ledger => { + for item in &chunk.ledgers { + match find_readback_row(rows, &item.name) { + None => mismatches.push(format!("ledger {} not found on readback", item.name)), + Some(row) => mismatches.extend(diff_ledger(item, row)), + } + } + } + MasterKind::StockItem => { + for item in &chunk.stock_items { + match find_readback_row(rows, &item.name) { + None => { + mismatches.push(format!("stock item {} not found on readback", item.name)) + } + Some(row) => mismatches.extend(diff_stock_item(item, row)), + } + } + } + } + mismatches +} + +fn chunked_masters( + masters: &BookMasters, + kind: MasterKind, + start: usize, + len: usize, +) -> BookMasters { + let end_of = |n: usize| (start + len).min(n); + match kind { + MasterKind::Unit => BookMasters { + units: masters.units[start.min(masters.units.len())..end_of(masters.units.len())] + .to_vec(), + ..Default::default() + }, + MasterKind::Godown => BookMasters { + godowns: masters.godowns + [start.min(masters.godowns.len())..end_of(masters.godowns.len())] + .to_vec(), + ..Default::default() + }, + MasterKind::StockGroup => BookMasters { + stock_groups: masters.stock_groups + [start.min(masters.stock_groups.len())..end_of(masters.stock_groups.len())] + .to_vec(), + ..Default::default() + }, + MasterKind::Group => BookMasters { + groups: masters.groups[start.min(masters.groups.len())..end_of(masters.groups.len())] + .to_vec(), + ..Default::default() + }, + MasterKind::Ledger => BookMasters { + ledgers: masters.ledgers + [start.min(masters.ledgers.len())..end_of(masters.ledgers.len())] + .to_vec(), + ..Default::default() + }, + MasterKind::StockItem => BookMasters { + stock_items: masters.stock_items + [start.min(masters.stock_items.len())..end_of(masters.stock_items.len())] + .to_vec(), + ..Default::default() + }, + } +} + +fn persist_lab_precheck_collisions(server: &Server, collisions: &[String]) { + if let Ok(dir) = lab_evidence_dir(server) { + let record = json!({ + "tool": "lab_import_masters.precheck", + "at": Utc::now().to_rfc3339_opts(SecondsFormat::Millis, true), + "collisions": collisions, + }); + let _ = append_egress_line( + &dir.join("lab-precheck-collisions.jsonl"), + &record.to_string(), + ); + } +} + +// --------------------------------------------------------------------------- +// Voucher rendering +// --------------------------------------------------------------------------- + +const BANK_SHAPE_TYPES: &[&str] = &["Payment", "Receipt", "Contra"]; + +fn is_bank_shape(voucher_type: &str) -> bool { + BANK_SHAPE_TYPES.contains(&voucher_type) +} + +/// `None` for Contra (moves between two of the company's own accounts, so it +/// has no counterparty leg -- §9.13). +fn bank_party_side(voucher_type: &str) -> Option<&'static str> { + match voucher_type { + "Payment" => Some("Dr"), + "Receipt" => Some("Cr"), + _ => None, + } +} + +fn signed_wire_amount(side: &str, amount: &str) -> String { + if side == "Dr" { + format!("-{amount}") + } else { + amount.to_string() + } +} + +fn render_bill_allocations_xml(side: &str, allocations: &[BookBillAllocation]) -> String { + allocations + .iter() + .map(|a| { + let name = a.name.clone().unwrap_or_default(); + format!( + "{}{}{}", + xml_escape(&name), + xml_escape(&a.bill_type), + signed_wire_amount(side, &a.amount) + ) + }) + .collect() +} + +fn render_ledger_entry_xml(tag: &str, line: &BookLedgerLine) -> String { + format!( + "<{tag}>{}{}{}{}", + xml_escape(&line.ledger), + if line.side == "Dr" { "Yes" } else { "No" }, + signed_wire_amount(&line.side, &line.amount), + render_bill_allocations_xml(&line.side, &line.bill_allocations), + ) +} + +fn narration_with_marker(narration: Option<&str>, attribution_id: Uuid) -> String { + format!( + "{}", + xml_escape( + format!( + "{} [BRIDGE-LAB:{attribution_id}]", + narration.unwrap_or("").trim() + ) + .trim() + ) + ) +} + +/// Journal, Payment/Receipt/Contra, and any accounting-mode (non-invoice) +/// Sales/Purchase/Credit-Note/Debit-Note -- every one of them renders with +/// `ALLLEDGERENTRIES.LIST` (the element every non-invoice write in +/// `TALLY_PROTOCOL_REFERENCE.md` uses). Bank-shape types additionally carry +/// `EFFECTIVEDATE` and `PARTYLEDGERNAME` and are sorted debit-first, per +/// §9.13; a Journal and an accounting-mode Sales keep the book's own line +/// order and carry neither, matching the observed accounting-mode Sales wire +/// shape (no `PARTYLEDGERNAME` element). +fn render_accounting_voucher_xml( + voucher: &BookVoucher, + remote_id: Uuid, + attribution_id: Uuid, +) -> Result { + let date = normalized_date(&voucher.date)?; + let bank = is_bank_shape(&voucher.voucher_type); + let mut lines: Vec<&BookLedgerLine> = voucher.ledger_lines.iter().collect(); + if bank { + lines.sort_by_key(|line| if line.side == "Dr" { 0 } else { 1 }); + } + let entries = lines + .iter() + .map(|line| render_ledger_entry_xml("ALLLEDGERENTRIES.LIST", line)) + .collect::(); + let effective_date = if bank { + format!("{date}") + } else { + String::new() + }; + let party = bank_party_side(&voucher.voucher_type) + .and_then(|side| lines.iter().find(|line| line.side == side)) + .map(|line| { + format!( + "{}", + xml_escape(&line.ledger) + ) + }) + .unwrap_or_default(); + let voucher_number = voucher + .voucher_number + .as_deref() + .map(|v| format!("{}", xml_escape(v))) + .unwrap_or_default(); + let narration = narration_with_marker(voucher.narration.as_deref(), attribution_id); + let vt = xml_escape(&voucher.voucher_type); + Ok(format!( + "{date}{effective_date}{vt}{party}{voucher_number}{narration}{entries}" + )) +} + +fn render_inventory_entry_xml(line: &BookInventoryLine) -> Result { + let stock_item = line + .stock_item + .as_deref() + .ok_or_else(|| "lab_inventory_stock_item_required".to_string())?; + let amount = line + .amount + .as_deref() + .ok_or_else(|| "lab_inventory_amount_required".to_string())?; + // §9.12a note 2: a service line carries an amount and no quantity -- + // omit RATE/ACTUALQTY/BILLEDQTY entirely rather than send empties. + let rate = line + .rate + .as_deref() + .map(|r| format!("{}", xml_escape(r))) + .unwrap_or_default(); + let qty = line + .qty + .as_deref() + .map(|qty| { + let billed = line.billed_qty.as_deref().unwrap_or(qty); + format!( + "{}{}", + xml_escape(qty), + xml_escape(billed) + ) + }) + .unwrap_or_default(); + let godown = line + .godown + .as_deref() + .map(|g| format!("{}", xml_escape(g))) + .unwrap_or_default(); + // §9.12a note 1: each inventory line carried its own + // ACCOUNTINGALLOCATIONS.LIST naming the sales/purchase ledger -- the + // *observed* working shape, not a proven requirement for every line. + let allocations = line + .accounting_allocations + .iter() + .map(|a| { + format!( + "{}No{}", + xml_escape(&a.ledger), xml_escape(&a.amount) + ) + }) + .collect::(); + // Batch/godown allocations, in the shape `lab_read_inventory` (Phase 3.2, + // agent_lab.rs) already reads back: BATCHNAME/GODOWNNAME/ACTUALQTY/ + // BILLEDQTY/AMOUNT. UNVERIFIED for import -- see module doc. + let batches = line + .batch_allocations + .iter() + .map(|b| { + let field = |key: &str| b.get(key).and_then(Value::as_str).unwrap_or(""); + format!( + "{}{}{}{}{}", + xml_escape(field("batch")), + xml_escape(field("godown")), + xml_escape(field("actual_qty")), + xml_escape(field("billed_qty")), + xml_escape(field("amount")), + ) + }) + .collect::(); + Ok(format!( + "{}No{rate}{qty}{}{godown}{allocations}{batches}", + xml_escape(stock_item), + xml_escape(amount) + )) +} + +/// Sales/Purchase/Credit-Note/Debit-Note carrying `inventory_lines`. Follows +/// §9.12a's "the shape that works" byte-for-byte: `LEDGERENTRIES.LIST` (never +/// `ALLLEDGERENTRIES.LIST` -- §9.12's TRAP), `ISINVOICE=Yes`, +/// `OBJVIEW="Invoice Voucher View"`. **UNVERIFIED for the gateway** -- see +/// module doc; §9.12a itself is a UI-import capture, not a gateway one, and +/// only for Sales. +fn render_invoice_voucher_xml( + voucher: &BookVoucher, + remote_id: Uuid, + attribution_id: Uuid, +) -> Result { + let date = normalized_date(&voucher.date)?; + let party = voucher + .party + .as_deref() + .ok_or_else(|| "lab_invoice_party_required".to_string())?; + let voucher_number = voucher + .voucher_number + .as_deref() + .map(|v| format!("{}", xml_escape(v))) + .unwrap_or_default(); + let narration = narration_with_marker(voucher.narration.as_deref(), attribution_id); + let ledger_entries = voucher + .ledger_lines + .iter() + .map(|line| render_ledger_entry_xml("LEDGERENTRIES.LIST", line)) + .collect::(); + let inventory_entries = voucher + .inventory_lines + .iter() + .map(render_inventory_entry_xml) + .collect::, _>>()? + .concat(); + let vt = xml_escape(&voucher.voucher_type); + let party_escaped = xml_escape(party); + Ok(format!( + "{date}{date}{vt}{voucher_number}{party_escaped}{party_escaped}Invoice Voucher ViewYes{narration}{ledger_entries}{inventory_entries}" + )) +} + +fn render_voucher_message( + voucher: &BookVoucher, + remote_id: Uuid, + attribution_id: Uuid, +) -> Result { + if voucher.is_invoice_mode { + render_invoice_voucher_xml(voucher, remote_id, attribution_id) + } else { + render_accounting_voucher_xml(voucher, remote_id, attribution_id) + } +} + +fn render_voucher_batch_xml( + company: &str, + vouchers: &[BookVoucher], + attribution_ids: &[Uuid], +) -> Result { + let messages = vouchers + .iter() + .zip(attribution_ids) + .map(|(voucher, id)| render_voucher_message(voucher, Uuid::new_v4(), *id)) + .collect::, _>>()? + .concat(); + Ok(render_import_envelope(company, "Vouchers", &messages)) +} + +// --------------------------------------------------------------------------- +// Voucher read-back / resume pre-check +// --------------------------------------------------------------------------- + +const ACCOUNTING_VOUCHER_FETCH: &str = + "DATE,VOUCHERNUMBER,VOUCHERTYPENAME,NARRATION,PARTYLEDGERNAME,\ +GUID,ISCANCELLED,ALLLEDGERENTRIES.LEDGERNAME,ALLLEDGERENTRIES.AMOUNT,\ +ALLLEDGERENTRIES.ISDEEMEDPOSITIVE,ALLLEDGERENTRIES.BILLALLOCATIONS.NAME,\ +ALLLEDGERENTRIES.BILLALLOCATIONS.BILLTYPE,ALLLEDGERENTRIES.BILLALLOCATIONS.AMOUNT"; + +fn render_voucher_window_request(company: &str, from: &str, to: &str) -> Result { + let company = ValidatedCompanyName::new(company.to_string()) + .map_err(|_| "company_name_invalid".to_string())?; + Ok(format!( + "
1ExportCollectionBridge Lab Voucher Readback
$$SysName:XML{}{from}{to}$Date >= $$Date:\"{from}\" AND $Date <= $$Date:\"{to}\"Voucher{}BridgeLabWindow", + xml_escape(company.as_str()), ACCOUNTING_VOUCHER_FETCH + )) +} + +#[derive(Debug, Clone, PartialEq, Eq)] +struct ObservedVoucher { + date: String, + voucher_number: Option, + voucher_type: Option, + narration: Option, + is_cancelled: bool, + ledger_entries: Vec<(String, String, String)>, // (ledger, is_deemed_positive, amount) +} + +/// Parses `ALLLEDGERENTRIES.LIST` per voucher, mirroring the nested-list +/// approach `parse_lab_inventory_vouchers` (agent_lab.rs) already established +/// for `ALLINVENTORYENTRIES.LIST` -- generalised here to the ledger-entry +/// list every non-invoice write in this document uses. +fn parse_voucher_readback_nested(xml: &str) -> Result, String> { + validate_agent_envelope(xml)?; + let mut reader = quick_xml::Reader::from_str(xml); + reader.config_mut().trim_text(false); + let mut path: Vec = Vec::new(); + let mut rows = Vec::new(); + let mut voucher: Option> = None; + let mut entry: Option> = None; + let mut entries: Vec<(String, String, String)> = Vec::new(); + let mut current_tag = String::new(); + const VOUCHER_PREFIX: [&str; 5] = ["ENVELOPE", "BODY", "DATA", "COLLECTION", "VOUCHER"]; + const ENTRY_PREFIX: [&str; 6] = [ + "ENVELOPE", + "BODY", + "DATA", + "COLLECTION", + "VOUCHER", + "ALLLEDGERENTRIES.LIST", + ]; + loop { + match reader.read_event() { + Ok(quick_xml::events::Event::Start(event)) => { + let tag = String::from_utf8_lossy(event.name().as_ref()).to_ascii_uppercase(); + if path.len() == 4 && path == ["ENVELOPE", "BODY", "DATA", "COLLECTION"] { + if tag != "VOUCHER" { + return Err("agent_read_protocol_invalid".to_string()); + } + voucher = Some(BTreeMap::new()); + entries.clear(); + } else if path.as_slice() == VOUCHER_PREFIX && tag == "ALLLEDGERENTRIES.LIST" { + entry = Some(BTreeMap::new()); + } + path.push(tag.clone()); + current_tag = tag; + } + Ok(quick_xml::events::Event::Text(text)) => { + let value = decoded_agent_text(text)?; + let parent = &path[..path.len().saturating_sub(1)]; + if parent == ENTRY_PREFIX { + if let Some(row) = entry.as_mut() { + append_agent_text(row, ¤t_tag, value); + } + } else if parent == VOUCHER_PREFIX { + if let Some(row) = voucher.as_mut() { + append_agent_text(row, ¤t_tag, value); + } + } + } + Ok(quick_xml::events::Event::End(event)) => { + let end = String::from_utf8_lossy(event.name().as_ref()).to_ascii_uppercase(); + if end == "ALLLEDGERENTRIES.LIST" && path.as_slice() == ENTRY_PREFIX { + if let Some(row) = entry.take() { + entries.push(( + row.get("LEDGERNAME").cloned().unwrap_or_default(), + row.get("ISDEEMEDPOSITIVE").cloned().unwrap_or_default(), + row.get("AMOUNT").cloned().unwrap_or_default(), + )); + } + } + if end == "VOUCHER" && path.as_slice() == VOUCHER_PREFIX { + if let Some(row) = voucher.take() { + rows.push(ObservedVoucher { + date: row.get("DATE").cloned().unwrap_or_default(), + voucher_number: row.get("VOUCHERNUMBER").cloned(), + voucher_type: row.get("VOUCHERTYPENAME").cloned(), + narration: row.get("NARRATION").cloned(), + is_cancelled: row.get("ISCANCELLED").map(String::as_str) == Some("Yes"), + ledger_entries: entries.clone(), + }); + entries.clear(); + } + } + if path.pop().as_deref() != Some(end.as_str()) { + return Err("agent_read_protocol_invalid".to_string()); + } + } + Ok(quick_xml::events::Event::Eof) => break, + Ok(_) => {} + Err(_) => return Err("agent_read_protocol_invalid".to_string()), + } + } + Ok(rows) +} + +fn narration_marker(narration: Option<&str>) -> Option { + let text = narration?; + let start = text.rfind("[BRIDGE-LAB:")?; + let rest = &text[start + "[BRIDGE-LAB:".len()..]; + let end = rest.find(']')?; + Some(rest[..end].to_string()) +} + +/// A voucher counts as already-posted-and-verified for a resume pre-check +/// only on (type, date, total-debit-amount, narration marker OR voucher +/// number) -- content alone (date/ledger/amount) is not an attribution key, +/// per §9.3: a book with a recurring same-day payment can already contain a +/// voucher with that tuple. Matching on the marker this module stamps into +/// every write closes that hole the same way the production path's +/// narration tag does. +fn voucher_already_verified(expected: &BookVoucher, observed: &[ObservedVoucher]) -> bool { + // Compare in Tally's own wire form: `normalized_date` accepts the book + // model's date (which may or may not already be YYYYMMDD) and the + // observed row is always already in that form; the ledger amount must be + // the *signed* wire amount (§9.13's Dr-negative convention), since + // book.json stores an unsigned magnitude plus a side. + let expected_date = normalized_date(&expected.date).unwrap_or_else(|_| expected.date.clone()); + observed.iter().any(|row| { + if row.is_cancelled { + return false; + } + if row.voucher_type.as_deref() != Some(expected.voucher_type.as_str()) { + return false; + } + if row.date != expected_date { + return false; + } + let number_matches = + expected.voucher_number.is_some() && row.voucher_number == expected.voucher_number; + let marker_matches = narration_marker(row.narration.as_deref()).is_some() + && narration_marker(row.narration.as_deref()) + == narration_marker(expected.narration.as_deref()); + if !number_matches && !marker_matches { + return false; + } + expected.ledger_lines.iter().all(|line| { + let expected_signed = signed_wire_amount(&line.side, &line.amount); + row.ledger_entries.iter().any(|(ledger, is_dr, amount)| { + ledger == &line.ledger + && ((line.side == "Dr") == (is_dr == "Yes")) + && amounts_equal(amount, &expected_signed) + }) + }) + }) +} + +// --------------------------------------------------------------------------- +// lab_import_vouchers +// --------------------------------------------------------------------------- + +pub(in crate::agent) async fn lab_import_vouchers( + server: &Server, + args: &Value, +) -> Result { + let mut vouchers: Vec = parse_book_value(args, "vouchers", "vouchers")?; + vouchers.sort_by(|a, b| { + (a.date.as_str(), a.voucher_number.as_deref().unwrap_or("")) + .cmp(&(b.date.as_str(), b.voucher_number.as_deref().unwrap_or(""))) + }); + let guid = required_string(args, "company_guid")?; + let start_batch = arg_usize(args, "start_batch", 0)?; + + let (_company, identity, mut evidence) = admit_lab_target(server).await?; + if !identity_matches_requested_guid(&identity, guid) { + return Err(ToolFailure::from("lab_target_company_mismatch".to_string()) + .with_prior_evidence(evidence)); + } + + let mut batch_reports = Vec::new(); + let batch_count = vouchers.len().div_ceil(MAX_VOUCHER_BATCH); + let mut stopped_at: Option = None; + + for batch_index in start_batch..batch_count { + let (_company, identity, admit_evidence) = admit_lab_target(server).await?; + evidence = combine_evidence(evidence.clone(), admit_evidence); + + let start = batch_index * MAX_VOUCHER_BATCH; + let end = (start + MAX_VOUCHER_BATCH).min(vouchers.len()); + let batch = &vouchers[start..end]; + let from = batch.first().map(|v| v.date.clone()).unwrap_or_default(); + let to = batch.last().map(|v| v.date.clone()).unwrap_or_default(); + + // Resume pre-check (§9.3/§12a's discipline): never blind-retry. Read + // the window this batch would occupy and check every voucher against + // it before sending anything. + let probe_request = render_voucher_window_request(identity.display_name(), &from, &to) + .map_err(ToolFailure::from)?; + let (probe_xml, probe_evidence) = lab_post_read( + server, + &identity, + "lab_import_vouchers.precheck", + probe_request, + ) + .await?; + evidence = combine_evidence(evidence.clone(), probe_evidence); + let observed = parse_voucher_readback_nested(&probe_xml) + .map_err(|code| ToolFailure::from(code).with_prior_evidence(evidence.clone()))?; + + let source_guids: Vec<&str> = batch.iter().map(|v| v.source_guid.as_str()).collect(); + let verified_count = batch + .iter() + .filter(|v| voucher_already_verified(v, &observed)) + .count(); + if verified_count == batch.len() { + batch_reports.push(json!({ + "batch": batch_index, "count": batch.len(), "state": "already_verified", "posted": false, + "source_guids": source_guids, + })); + continue; + } + if verified_count > 0 { + // Partial match on an uncertain prior attempt: stop rather than + // guess which subset is safe to resend. Returned immediately + // below, so this batch never reaches `stopped_at`'s summary use. + batch_reports.push(json!({ + "batch": batch_index, "count": batch.len(), "state": "partially_verified_uncertain", + "verified": verified_count, "posted": false, + })); + return Err( + ToolFailure::from("lab_batch_partially_verified_uncertain".to_string()) + .with_prior_evidence(evidence), + ); + } + + let attribution_ids: Vec = batch.iter().map(|_| Uuid::new_v4()).collect(); + let xml = render_voucher_batch_xml(identity.display_name(), batch, &attribution_ids) + .map_err(ToolFailure::from)?; + let (response, post_evidence) = + post_lab_batch(server, &identity, "lab_import_vouchers.write", xml).await?; + evidence = combine_evidence(evidence.clone(), post_evidence); + let outcome = bridge_tally_protocol::parse_import_outcome(&response) + .map_err(|_| ToolFailure::from("lab_import_response_invalid".to_string()))?; + let clean = outcome + .counters() + .is_clean_success_for(batch.len() as u64, 0, 0); + + // Mandatory read-back. + let (readback_xml, readback_evidence) = lab_post_read( + server, + &identity, + "lab_import_vouchers.readback", + render_voucher_window_request(identity.display_name(), &from, &to) + .map_err(ToolFailure::from)?, + ) + .await?; + evidence = combine_evidence(evidence.clone(), readback_evidence); + let readback = parse_voucher_readback_nested(&readback_xml) + .map_err(|code| ToolFailure::from(code).with_prior_evidence(evidence.clone()))?; + let posted_count = batch + .iter() + .filter(|v| voucher_already_verified(v, &readback)) + .count(); + let batch_ok = clean && posted_count == batch.len(); + + batch_reports.push(json!({ + "batch": batch_index, + "count": batch.len(), + "counters_clean": clean, + "verified_on_readback": posted_count, + "state": if batch_ok { "posted_verified" } else { "readback_mismatch" }, + "posted": true, + "source_guids": source_guids, + })); + if !batch_ok { + stopped_at = Some(batch_index); + break; + } + } + + let ok = stopped_at.is_none(); + Ok(ToolOutcome { + payload: json!({"result": { + "ok": ok, + "total_vouchers": vouchers.len(), + "batch_count": batch_count, + "batches": batch_reports, + "stopped_at_batch": stopped_at, + }}), + evidence, + company_guid: Some(guid.to_string()), + truncated: false, + }) +} + +// --------------------------------------------------------------------------- +// Shared write-path helper +// --------------------------------------------------------------------------- + +async fn post_lab_batch( + server: &Server, + identity: &VerifiedCompanyIdentity, + tool: &str, + xml: String, +) -> Result<(String, Evidence), ToolFailure> { + let _ = identity; + let (body, runtime_evidence) = server + .runtime + .post_lab_import(server.tally_config(), xml.clone()) + .await + .map_err(|error| ToolFailure::from_runtime("lab_import_post_failed", error))?; + let evidence = evidence_from_runtime_read(runtime_evidence); + persist_lab_exchange(server, tool, &xml, &body) + .map_err(|code| ToolFailure::from(code).with_prior_evidence(evidence.clone()))?; + Ok((body, evidence)) +} + +#[cfg(test)] +#[path = "agent_lab_import_tests.rs"] +mod tests; diff --git a/src-tauri/src/agent_lab_import_tests.rs b/src-tauri/src/agent_lab_import_tests.rs new file mode 100644 index 00000000..93132b88 --- /dev/null +++ b/src-tauri/src/agent_lab_import_tests.rs @@ -0,0 +1,710 @@ +//! Unit tests for the lab writer (Phase 3.4/3.5). Synthetic fixtures only, no +//! client data and no live Tally connection -- matching `agent_lab.rs`'s own +//! test discipline. Golden XML strings below are asserted verbatim; any +//! change to the rendered shape must be a deliberate, reviewed edit here. +use super::*; + +const REMOTE_ID: Uuid = Uuid::from_u128(1); +const ATTRIBUTION_ID: Uuid = Uuid::from_u128(2); + +// --------------------------------------------------------------------------- +// canonical_master_key -- §9.4d, licensed TallyPrime 7.1 +// --------------------------------------------------------------------------- + +#[test] +fn canonical_master_key_folds_the_composed_9_4d_rules() { + let base = canonical_master_key("MB-PROBE-LEDGER-A"); + assert_eq!( + canonical_master_key("MB PROBE LEDGER A"), + base, + "hyphen == space" + ); + assert_eq!( + canonical_master_key("mb probe ledger a"), + base, + "ASCII case folds" + ); + assert_eq!( + canonical_master_key(" mb probe ledger a "), + base, + "surrounding + collapsed runs" + ); + assert_eq!( + canonical_master_key("mb/probe/ledger/a"), + base, + "slash == space" + ); +} + +#[test] +fn canonical_master_key_does_not_fold_unmeasured_separators() { + // §9.4d: an en dash and an underscore are ordinary characters to Tally, + // not separators -- a class-based fold would wrongly merge these. + assert_ne!(canonical_master_key("A_B"), canonical_master_key("A B")); + assert_ne!( + canonical_master_key("A\u{2013}B"), + canonical_master_key("A B") + ); +} + +#[test] +fn canonical_master_key_does_not_normalise_unicode_forms() { + // §9.4d: "canonical equivalence is still refused" -- NFC vs NFD must stay + // distinguishable through this fold, unlike every other transformation. + let nfc = "\u{00e9}"; // é, single codepoint + let nfd = "e\u{0301}"; // e + combining acute accent + assert_ne!(canonical_master_key(nfc), canonical_master_key(nfd)); +} + +// --------------------------------------------------------------------------- +// Master XML -- golden fixtures +// --------------------------------------------------------------------------- + +#[test] +fn unit_create_xml_golden() { + let u = BookUnit { + name: "Kgs".into(), + is_simple_unit: Some("Yes".into()), + decimal_places: Some("3".into()), + }; + assert_eq!( + render_unit_xml(&u), + "\ +Yes3" + ); +} + +#[test] +fn godown_create_xml_golden() { + let g = BookNamedParent { + name: "Main Godown".into(), + parent: Some("Primary".into()), + }; + assert_eq!( + render_parented_xml("GODOWN", &g), + "\ +Primary" + ); +} + +#[test] +fn ledger_create_xml_golden_with_gst_and_billwise() { + let l = BookLedger { + name: "Sri Ram Cables Private Limited".into(), + parent: Some("Sundry Debtors".into()), + opening_balance: Some("1000.00".into()), + is_billwise_on: Some(true), + party_gstin: Some("27ZZZZZ0000Z1Z5".into()), + tax_type: None, + gst_duty_head: None, + opening_bill_allocations: vec![], + }; + assert_eq!( + render_ledger_xml(&l), + "\ +Sundry Debtors1000.00Yes\ +27ZZZZZ0000Z1Z5" + ); +} + +#[test] +fn ledger_create_xml_uses_the_irregular_9_4d_duty_head_vocabulary_verbatim() { + // §8.3: the state head is "State Tax", never "SGST" -- passed through + // exactly as the source carried it, never synthesised or normalised. + let l = BookLedger { + name: "Output State Tax 9%".into(), + parent: Some("Duties & Taxes".into()), + opening_balance: None, + is_billwise_on: None, + party_gstin: None, + tax_type: Some("GST".into()), + gst_duty_head: Some("State Tax".into()), + opening_bill_allocations: vec![], + }; + let xml = render_ledger_xml(&l); + assert!(xml.contains("GST")); + assert!(xml.contains("State Tax")); + assert!(!xml.contains("SGST")); + // §9.1b: `&` in a group name must be escaped or the whole request is malformed. + assert!(xml.contains("Duties & Taxes")); +} + +#[test] +fn stock_item_create_xml_golden() { + let s = BookStockItem { + name: "Sodium Bicarbonate".into(), + parent: Some("Chemicals".into()), + base_unit: Some("Kgs".into()), + opening_qty: Some("100".into()), + opening_rate: Some("50.00".into()), + opening_value: Some("5000.00".into()), + gst_applicable: Some("Applicable".into()), + hsn_code: Some("28362000".into()), + }; + let xml = render_stock_item_xml(&s); + assert_eq!( + xml, + "\ +ChemicalsKgs100\ +50.005000.00\ +Applicable28362000" + ); +} + +// --------------------------------------------------------------------------- +// Voucher XML -- golden fixtures, mirroring §9.13/§9.12a +// --------------------------------------------------------------------------- + +fn payment_voucher() -> BookVoucher { + BookVoucher { + source_guid: "src-1".into(), + voucher_type: "Payment".into(), + date: "2026-04-05".into(), + voucher_number: Some("59".into()), + narration: Some("UPI payment".into()), + party: Some("HDFC Bank 1649".into()), + is_invoice_mode: false, + ledger_lines: vec![ + BookLedgerLine { + ledger: "HDFC Bank 1649".into(), + side: "Cr".into(), + amount: "30000.00".into(), + bill_allocations: vec![], + }, + BookLedgerLine { + ledger: "Labour Charges".into(), + side: "Dr".into(), + amount: "30000.00".into(), + bill_allocations: vec![], + }, + ], + inventory_lines: vec![], + } +} + +#[test] +fn payment_voucher_xml_is_dr_first_with_effective_date_and_counterparty_party() { + let voucher = payment_voucher(); + let xml = render_accounting_voucher_xml(&voucher, REMOTE_ID, ATTRIBUTION_ID).unwrap(); + // §9.13: Dr leg first regardless of input order; EFFECTIVEDATE present; + // PARTYLEDGERNAME is the counterparty (Dr side for a Payment), not the bank. + let dr_pos = xml.find("Labour Charges").unwrap(); + let cr_pos = xml.find("HDFC Bank 1649").unwrap(); + assert!(dr_pos < cr_pos, "Dr leg must render before Cr leg"); + assert!(xml.contains("20260405")); + assert!(xml.contains("Labour Charges")); + assert!( + xml.contains("-30000.00"), + "Dr amount must be negative on the wire" + ); + assert!(xml.contains(&format!("[BRIDGE-LAB:{ATTRIBUTION_ID}]"))); + assert!(xml.contains("OBJVIEW=\"Accounting Voucher View\"")); + assert!(xml.contains("VCHTYPE=\"Payment\"")); +} + +#[test] +fn contra_voucher_has_no_party_ledger_name() { + let mut voucher = payment_voucher(); + voucher.voucher_type = "Contra".into(); + voucher.ledger_lines = vec![ + BookLedgerLine { + ledger: "Cash".into(), + side: "Dr".into(), + amount: "100000.00".into(), + bill_allocations: vec![], + }, + BookLedgerLine { + ledger: "HDFC Bank 1649".into(), + side: "Cr".into(), + amount: "100000.00".into(), + bill_allocations: vec![], + }, + ]; + let xml = render_accounting_voucher_xml(&voucher, REMOTE_ID, ATTRIBUTION_ID).unwrap(); + assert!( + !xml.contains("PARTYLEDGERNAME"), + "Contra names no counterparty (§9.13)" + ); + assert!(xml.contains("20260405")); +} + +#[test] +fn journal_voucher_keeps_book_order_and_has_no_effective_date_or_party() { + let mut voucher = payment_voucher(); + voucher.voucher_type = "Journal".into(); + // Deliberately Cr-first in the book -- a Journal must NOT be reordered. + let xml = render_accounting_voucher_xml(&voucher, REMOTE_ID, ATTRIBUTION_ID).unwrap(); + let first = xml.find("LEDGERNAME").unwrap(); + assert!( + xml[first..].starts_with("LEDGERNAME>HDFC Bank 1649"), + "Journal preserves input order" + ); + assert!(!xml.contains("EFFECTIVEDATE")); + assert!(!xml.contains("PARTYLEDGERNAME")); +} + +#[test] +fn accounting_mode_sales_carries_bill_allocations_with_matching_sign() { + let voucher = BookVoucher { + source_guid: "src-2".into(), + voucher_type: "Sales".into(), + date: "2025-05-09".into(), + voucher_number: Some("1".into()), + narration: Some("Invoice 21".into()), + party: Some("Sri Ram Cables Private Limited".into()), + is_invoice_mode: false, + ledger_lines: vec![ + BookLedgerLine { + ledger: "Sri Ram Cables Private Limited".into(), + side: "Dr".into(), + amount: "673364.64".into(), + bill_allocations: vec![BookBillAllocation { + name: None, + bill_type: "On Account".into(), + amount: "673364.64".into(), + }], + }, + BookLedgerLine { + ledger: "Sales".into(), + side: "Cr".into(), + amount: "570648.00".into(), + bill_allocations: vec![], + }, + ], + inventory_lines: vec![], + }; + let xml = render_accounting_voucher_xml(&voucher, REMOTE_ID, ATTRIBUTION_ID).unwrap(); + assert!( + !xml.contains("PARTYLEDGERNAME"), + "accounting-mode Sales names no PARTYLEDGERNAME (observed wire shape)" + ); + assert!(xml.contains("On Account-673364.64")); + assert!(xml.contains("ALLLEDGERENTRIES.LIST")); +} + +#[test] +fn invoice_voucher_uses_ledgerentries_not_allledgerentries() { + // §9.12's TRAP: ALLLEDGERENTRIES.LIST is silently discarded on an invoice + // voucher. LEDGERENTRIES.LIST is the required element. + let voucher = BookVoucher { + source_guid: "src-3".into(), + voucher_type: "Sales".into(), + date: "2026-04-05".into(), + voucher_number: Some("1".into()), + narration: Some("Invoice".into()), + party: Some("Fixture Acid & Chemicals".into()), + is_invoice_mode: true, + ledger_lines: vec![ + BookLedgerLine { + ledger: "Fixture Acid & Chemicals".into(), + side: "Dr".into(), + amount: "118.00".into(), + bill_allocations: vec![BookBillAllocation { + name: Some("Inv-1".into()), + bill_type: "New Ref".into(), + amount: "118.00".into(), + }], + }, + BookLedgerLine { + ledger: "Output CGST 9%".into(), + side: "Cr".into(), + amount: "9.00".into(), + bill_allocations: vec![], + }, + BookLedgerLine { + ledger: "Output SGST 9%".into(), + side: "Cr".into(), + amount: "9.00".into(), + bill_allocations: vec![], + }, + ], + inventory_lines: vec![BookInventoryLine { + stock_item: Some("Widget".into()), + rate: Some("100.00/Nos".into()), + qty: Some("1 Nos".into()), + billed_qty: Some("1 Nos".into()), + amount: Some("100.00".into()), + godown: Some("Main Godown".into()), + accounting_allocations: vec![BookAccountingAllocation { + ledger: "Sales".into(), + amount: "100.00".into(), + }], + batch_allocations: vec![], + }], + }; + let xml = render_invoice_voucher_xml(&voucher, REMOTE_ID, ATTRIBUTION_ID).unwrap(); + assert!(xml.contains("")); + assert!(!xml.contains("ALLLEDGERENTRIES.LIST")); + assert!(xml.contains("ISINVOICE>Yes")); + assert!(xml.contains("OBJVIEW=\"Invoice Voucher View\"")); + assert!(xml.contains("Fixture Acid & Chemicals")); + assert!(xml.contains("")); + assert!(xml.contains("New Ref")); + assert!(xml.contains("")); + assert!(xml.contains("Main Godown")); +} + +#[test] +fn invoice_voucher_without_party_is_refused_before_any_xml_is_sent() { + let mut voucher = payment_voucher(); + voucher.is_invoice_mode = true; + voucher.party = None; + voucher.inventory_lines = vec![BookInventoryLine { + stock_item: Some("Widget".into()), + rate: None, + qty: None, + billed_qty: None, + amount: Some("1.00".into()), + godown: None, + accounting_allocations: vec![], + batch_allocations: vec![], + }]; + assert_eq!( + render_voucher_message(&voucher, REMOTE_ID, ATTRIBUTION_ID), + Err("lab_invoice_party_required".to_string()) + ); +} + +#[test] +fn service_inventory_line_omits_quantity_fields() { + let line = BookInventoryLine { + stock_item: Some("Consulting".into()), + rate: None, + qty: None, + billed_qty: None, + amount: Some("500.00".into()), + godown: None, + accounting_allocations: vec![], + batch_allocations: vec![], + }; + let xml = render_inventory_entry_xml(&line).unwrap(); + assert!(!xml.contains("ACTUALQTY")); + assert!(!xml.contains("BILLEDQTY")); + assert!(!xml.contains("")); + assert!(xml.contains("500.00")); +} + +// --------------------------------------------------------------------------- +// Master read-back diff / existing-master collision +// --------------------------------------------------------------------------- + +fn row(pairs: &[(&str, &str)]) -> BTreeMap { + pairs + .iter() + .map(|(k, v)| (k.to_string(), v.to_string())) + .collect() +} + +#[test] +fn find_readback_row_matches_the_9_4d_fold() { + let rows = vec![row(&[("NAME", "MB-PROBE-LEDGER-A")])]; + assert!(find_readback_row(&rows, "mb probe ledger a").is_some()); + assert!(find_readback_row(&rows, "an entirely different name").is_none()); +} + +#[test] +fn diff_ledger_flags_a_wrong_opening_balance() { + let l = BookLedger { + name: "Cash".into(), + parent: Some("Cash-in-Hand".into()), + opening_balance: Some("100.00".into()), + is_billwise_on: None, + party_gstin: None, + tax_type: None, + gst_duty_head: None, + opening_bill_allocations: vec![], + }; + let good = row(&[("PARENT", "Cash-in-Hand"), ("OPENINGBALANCE", "100.00")]); + assert!(diff_ledger(&l, &good).is_empty()); + let bad = row(&[("PARENT", "Cash-in-Hand"), ("OPENINGBALANCE", "50.00")]); + let mismatches = diff_ledger(&l, &bad); + assert_eq!(mismatches.len(), 1); + assert!(mismatches[0].contains("opening_balance")); +} + +#[test] +fn diff_ledger_treats_equal_decimals_as_equal_regardless_of_formatting() { + let l = BookLedger { + name: "Cash".into(), + parent: None, + opening_balance: Some("0.00".into()), + is_billwise_on: None, + party_gstin: None, + tax_type: None, + gst_duty_head: None, + opening_bill_allocations: vec![], + }; + let observed = row(&[("OPENINGBALANCE", "0")]); + assert!(diff_ledger(&l, &observed).is_empty()); +} + +#[test] +fn diff_parented_uses_the_9_4d_fold_not_exact_equality() { + let item = BookNamedParent { + name: "Main Godown".into(), + parent: Some("Sub-Location".into()), + }; + let observed = row(&[("PARENT", "Sub Location")]); + assert!(diff_parented("godown", &item, &observed).is_empty()); +} + +#[test] +fn diff_stock_item_flags_a_wrong_parent_but_not_an_unspecified_one() { + let s = BookStockItem { + name: "Widget".into(), + parent: Some("Chemicals".into()), + base_unit: None, + opening_qty: None, + opening_rate: None, + opening_value: None, + gst_applicable: None, + hsn_code: None, + }; + let mismatched = diff_stock_item(&s, &row(&[("PARENT", "Consumables")])); + assert_eq!(mismatched.len(), 1); + let s_no_parent_check = BookStockItem { parent: None, ..s }; + assert!(diff_stock_item(&s_no_parent_check, &row(&[("PARENT", "Anything")])).is_empty()); +} + +// --------------------------------------------------------------------------- +// Resume: narration marker + type/date/amount fingerprint +// --------------------------------------------------------------------------- + +fn observed( + voucher_type: &str, + date: &str, + number: Option<&str>, + narration: Option<&str>, + entries: &[(&str, &str, &str)], +) -> ObservedVoucher { + ObservedVoucher { + date: date.to_string(), + voucher_number: number.map(str::to_string), + voucher_type: Some(voucher_type.to_string()), + narration: narration.map(str::to_string), + is_cancelled: false, + ledger_entries: entries + .iter() + .map(|(l, d, a)| (l.to_string(), d.to_string(), a.to_string())) + .collect(), + } +} + +#[test] +fn voucher_already_verified_matches_by_narration_marker_and_amounts() { + let expected = payment_voucher_with_marker(&ATTRIBUTION_ID.to_string()); + let matching = observed( + "Payment", + "20260405", + Some("59"), + Some(&format!("UPI payment [BRIDGE-LAB:{ATTRIBUTION_ID}]")), + &[ + ("HDFC Bank 1649", "No", "30000.00"), + ("Labour Charges", "Yes", "-30000.00"), + ], + ); + assert!(voucher_already_verified(&expected, &[matching])); +} + +fn payment_voucher_with_marker(_marker: &str) -> BookVoucher { + // The book model never carries the marker itself (it is stamped at + // render time); resume matches on voucher_number when present, as here. + payment_voucher() +} + +#[test] +fn voucher_already_verified_rejects_a_content_only_match_without_number_or_marker() { + let mut expected = payment_voucher(); + expected.voucher_number = None; // forces reliance on the marker alone + let same_shape_different_voucher = observed( + "Payment", + "20260405", + None, + Some("An unrelated payment, same date and amount"), + &[ + ("HDFC Bank 1649", "No", "30000.00"), + ("Labour Charges", "Yes", "-30000.00"), + ], + ); + // §9.3: content (date/ledger/amount) alone is not an attribution key. + assert!(!voucher_already_verified( + &expected, + &[same_shape_different_voucher] + )); +} + +#[test] +fn voucher_already_verified_ignores_a_cancelled_voucher() { + let expected = payment_voucher(); + let mut cancelled = observed( + "Payment", + "20260405", + Some("59"), + Some("UPI payment"), + &[ + ("HDFC Bank 1649", "No", "30000.00"), + ("Labour Charges", "Yes", "-30000.00"), + ], + ); + cancelled.is_cancelled = true; + assert!(!voucher_already_verified(&expected, &[cancelled])); +} + +#[test] +fn narration_marker_extracts_the_bracketed_uuid() { + assert_eq!( + narration_marker(Some( + "Some text [BRIDGE-LAB:00000000-0000-4000-8000-000000000002]" + )), + Some("00000000-0000-4000-8000-000000000002".to_string()) + ); + assert_eq!(narration_marker(Some("no marker here")), None); + assert_eq!(narration_marker(None), None); +} + +// --------------------------------------------------------------------------- +// Voucher read-back parsing (synthetic Tally export) +// --------------------------------------------------------------------------- + +#[test] +fn parses_ledger_entries_and_bill_allocations_per_voucher() { + let xml = "
1
\ +2026040559Payment\ +Labour Chargesg-1No\ +HDFC Bank 1649No30000.00\ +Labour ChargesYes-30000.00\ +"; + let rows = parse_voucher_readback_nested(xml).unwrap(); + assert_eq!(rows.len(), 1); + assert_eq!(rows[0].voucher_number.as_deref(), Some("59")); + assert_eq!(rows[0].ledger_entries.len(), 2); + assert_eq!( + rows[0].ledger_entries[1], + ( + "Labour Charges".to_string(), + "Yes".to_string(), + "-30000.00".to_string() + ) + ); +} + +#[test] +fn a_non_voucher_child_of_collection_is_refused() { + let xml = "
1
\ +1"; + assert_eq!( + parse_voucher_readback_nested(xml), + Err("agent_read_protocol_invalid".to_string()) + ); +} + +// --------------------------------------------------------------------------- +// parse_book_value: inline vs book_path +// --------------------------------------------------------------------------- + +#[test] +fn parse_book_value_reads_inline_json() { + let args = json!({"masters": {"units": [{"name": "Nos"}]}}); + let masters: BookMasters = parse_book_value(&args, "masters", "masters").unwrap(); + assert_eq!(masters.units.len(), 1); +} + +#[test] +fn parse_book_value_reads_a_book_path_section() { + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("book.json"); + fs::write( + &path, + json!({"masters": {"units": [{"name": "Kgs"}]}, "vouchers": []}).to_string(), + ) + .unwrap(); + let args = json!({"book_path": path.to_string_lossy()}); + let masters: BookMasters = parse_book_value(&args, "masters", "masters").unwrap(); + assert_eq!(masters.units[0].name, "Kgs"); +} + +#[test] +fn parse_book_value_requires_one_of_inline_or_book_path() { + let args = json!({}); + let result: Result = parse_book_value(&args, "masters", "masters"); + assert_eq!(result.unwrap_err().code, "masters_or_book_path_required"); +} + +// --------------------------------------------------------------------------- +// Identity-guard helper and master batching +// --------------------------------------------------------------------------- + +#[test] +fn identity_matches_requested_guid_is_case_insensitive() { + let identity = VerifiedCompanyIdentity::test_fixture( + "BRIDGE REHEARSAL", + "89B0CC46-E3B8-4809-8FC7-E29EB2AE547D", + ); + assert!(identity_matches_requested_guid( + &identity, + "89b0cc46-e3b8-4809-8fc7-e29eb2ae547d" + )); + assert!(!identity_matches_requested_guid( + &identity, + "2864b4ac-e5a3-4efc-9d2b-7593928d8f8b" + )); +} + +#[test] +fn chunked_masters_splits_only_the_requested_kind() { + let masters = BookMasters { + ledgers: (0..5) + .map(|i| BookLedger { + name: format!("L{i}"), + parent: None, + opening_balance: None, + is_billwise_on: None, + party_gstin: None, + tax_type: None, + gst_duty_head: None, + opening_bill_allocations: vec![], + }) + .collect(), + units: vec![BookUnit { + name: "Nos".into(), + is_simple_unit: None, + decimal_places: None, + }], + ..Default::default() + }; + let chunk = chunked_masters(&masters, MasterKind::Ledger, 2, 2); + assert_eq!( + chunk + .ledgers + .iter() + .map(|l| l.name.clone()) + .collect::>(), + vec!["L2", "L3"] + ); + assert!( + chunk.units.is_empty(), + "chunking one kind must not carry over another" + ); +} + +#[test] +fn master_import_order_matches_the_plan() { + assert_eq!( + MasterKind::IMPORT_ORDER.map(MasterKind::tally_type), + [ + "Unit", + "Godown", + "StockGroup", + "Group", + "Ledger", + "StockItem" + ] + ); +} + +#[test] +fn amounts_equal_ignores_decimal_formatting_but_not_value() { + assert!(amounts_equal("0.00", "0")); + assert!(amounts_equal("100.00", "100")); + assert!(!amounts_equal("100.00", "100.01")); +} From a14f56c5e9f78f39de8a3cec1a2025238df0d30f Mon Sep 17 00:00:00 2001 From: t Date: Mon, 14 Sep 2026 03:36:50 +0530 Subject: [PATCH 08/14] fix(lab): distinguish Tally default masters from true collisions (Phase 3.4) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit lab_import_masters previously refused on ANY same-name master, including Tally's own auto-created defaults every new company already has (ledger Cash under Cash-in-Hand, Profit & Loss A/c under the reserved primary root, and any reserved Group) -- the first live rehearsal attempt hit exactly this refusing on Cash/Profit & Loss A/c collisions with zero writes made. - is_default_ledger/is_default_group: recognise a default by name *and* observed parent/RESERVEDNAME, so a renamed or relocated same-name master still falls through to the ordinary collision refusal. - A default is never (re-)Created (Create-overwrite trap); for Ledger, a partial Alter carries only a genuinely-changed writable field (OPENINGBALANCE only -- GST fields are Alter-inert per §8.3, never offered). Defaults are included in the mandatory read-back diff. - Parent comparison normalises Tally's reserved-primary marker via bridge_tally_protocol::is_tally_reserved_root (reused, not reinvented) without ever writing the marker back. - New guard found while inspecting the rehearsal book: a requested Group literally named with the sanitized U+0004 marker (the self-referential root, mis-captured by build_book.py as a "custom" group) is refused outright -- it never collision-matches Tally's own plainly-named "Primary" row, so without this it would have been silently Created with a garbled name. 10 new unit tests (default skip, default opening alter, true collision still refused, reserved parent normalisation, reserved-root name guard). Production write guards (agent_import_post.rs, approved_import.rs) untouched -- diff vs origin/master is empty. --- src-tauri/src/agent_lab_import.rs | 253 +++++++++++++++++++++++- src-tauri/src/agent_lab_import_tests.rs | 205 +++++++++++++++++++ 2 files changed, 452 insertions(+), 6 deletions(-) diff --git a/src-tauri/src/agent_lab_import.rs b/src-tauri/src/agent_lab_import.rs index 6c3e1fcd..341e4c24 100644 --- a/src-tauri/src/agent_lab_import.rs +++ b/src-tauri/src/agent_lab_import.rs @@ -51,8 +51,9 @@ use super::*; use bridge_tally_core::ExactDecimal; +use bridge_tally_protocol::is_tally_reserved_root; use serde::Deserialize; -use std::collections::BTreeMap; +use std::collections::{BTreeMap, BTreeSet}; use uuid::Uuid; const MAX_MASTER_BATCH: usize = 200; @@ -358,6 +359,103 @@ fn find_readback_row<'a>( .find(|row| canonical_master_key(row.get("NAME").map(String::as_str).unwrap_or("")) == key) } +// --------------------------------------------------------------------------- +// Tally default masters -- every new company has these before this tool ever +// runs, so a same-name row is not the Create-overwrite collision §9.4 exists +// to catch. Distinguished from a true collision so a default is (a) never +// sent as a Create, and (b) still diffed against the book and, for Ledger, +// partially Altered if a writable field differs. Any other same-name master +// remains an ordinary refusal. +// --------------------------------------------------------------------------- + +/// Where a *default* ledger's `PARENT` is expected to resolve. Compared with +/// [`canonical_master_key`] / [`is_tally_reserved_root`] -- never written: +/// [`render_ledger_xml`] still defaults a missing parent to the plain word +/// `Primary`, never to the reserved-root marker. +enum DefaultLedgerParent { + /// A named reserved group (matched by the §9.4d fold), e.g. `Cash-in-Hand`. + ReservedGroup(&'static str), + /// Tally's reserved primary root itself (`Profit & Loss A/c`'s parent). + ReservedPrimary, +} + +/// Tally auto-creates both of these in every new company: ledger `Cash` +/// under the reserved group `Cash-in-Hand`, and `Profit & Loss A/c` under +/// the reserved primary root. Matched by name only here -- the caller must +/// additionally confirm the *observed* parent before treating a same-name +/// row as this default; see [`is_default_ledger`]. +fn default_ledger_parent(name: &str) -> Option { + let key = canonical_master_key(name); + if key == canonical_master_key("Cash") { + Some(DefaultLedgerParent::ReservedGroup("Cash-in-Hand")) + } else if key == canonical_master_key("Profit & Loss A/c") { + Some(DefaultLedgerParent::ReservedPrimary) + } else { + None + } +} + +/// Whether an *existing* ledger row is Tally's own default for `name`, not a +/// same-name collision. Requires the observed parent to match the default's +/// expected parent too -- a ledger named "Cash" moved under a different +/// group, or a user-created "Profit & Loss A/c" that is not actually under +/// the reserved root, is a true collision and must still fall through to the +/// ordinary refusal, not be silently treated as the default. +fn is_default_ledger(name: &str, observed_parent: &str) -> bool { + match default_ledger_parent(name) { + Some(DefaultLedgerParent::ReservedGroup(expected)) => { + canonical_master_key(observed_parent) == canonical_master_key(expected) + } + Some(DefaultLedgerParent::ReservedPrimary) => is_tally_reserved_root(observed_parent), + None => false, + } +} + +/// Whether an existing Group row is one of Tally's own predefined/reserved +/// groups (every new company has all of them), signalled by a non-empty +/// `RESERVEDNAME` -- the same signal `group_ancestry.rs`'s `GroupIndex` +/// already uses to recognise a predefined group identity. +fn is_default_group(row: &BTreeMap) -> bool { + row.get("RESERVEDNAME") + .is_some_and(|value| !value.trim().is_empty()) +} + +/// Which writable field(s) on an existing *default* ledger differ from the +/// book and need a partial `Alter` (Brain trap: `Create` on an existing +/// ledger overwrites its opening balance instead of merging; a partial +/// `Alter` carrying only the changed field(s) is the safe write here). +/// Deliberately restricted to `OPENINGBALANCE`: the module doc / §8.3 already +/// establish that the GST-related fields (`PARTYGSTIN`/`TAXTYPE`/ +/// `GSTDUTYHEAD`/`ISBILLWISEON`) are settable at Create but silently dropped +/// at Alter, so they are never offered as an Alter candidate. +fn ledger_alter_fields( + book: &BookLedger, + row: &BTreeMap, +) -> Vec<(&'static str, String)> { + let mut fields = Vec::new(); + let expected_opening = book.opening_balance.as_deref().unwrap_or("0.00"); + let observed_opening = row.get("OPENINGBALANCE").map(String::as_str).unwrap_or(""); + if !amounts_equal(expected_opening, observed_opening) { + fields.push(("OPENINGBALANCE", expected_opening.to_string())); + } + fields +} + +/// Renders a partial `Alter`: only the given fields, never the full ledger +/// (an Alter that omitted a field would leave it unchanged, but resending +/// every field would also silently re-assert ones §8.3 already says are +/// Alter-inert -- so this renders exactly, and only, `fields`). +fn render_ledger_alter_xml(name: &str, fields: &[(&'static str, String)]) -> String { + let body: String = fields + .iter() + .map(|(tag, value)| format!("<{tag}>{}", xml_escape(value))) + .collect(); + format!( + "{body}", + name = xml_escape(name) + ) +} + // --------------------------------------------------------------------------- // Master XML renderers (Create). See module doc: UNVERIFIED for the gateway // on every kind except the fields §8.3/§9.4a already qualify for Ledger. @@ -620,8 +718,15 @@ pub(in crate::agent) async fn lab_import_masters( } // ---- Create-overwrite pre-check (§9.4): refuse before any write if the - // target already carries a same-name master under ANY kind requested. ---- + // target already carries a same-name master under ANY kind requested -- + // except a Tally *default* (ledger `Cash`/`Profit & Loss A/c`, or any + // reserved Group), which every new company already has before this tool + // ever runs and so is never a collision. A default is excluded from the + // Create batch below and, for Ledger, scheduled for a partial Alter if a + // writable field differs. Any other same-name master is still refused. let mut collisions: Vec = Vec::new(); + let mut default_ledger_alters: Vec<(BookLedger, BTreeMap)> = Vec::new(); + let mut default_group_keys: BTreeSet = BTreeSet::new(); for kind in MasterKind::IMPORT_ORDER { let requested = kind.names(&masters); if requested.is_empty() { @@ -635,9 +740,42 @@ pub(in crate::agent) async fn lab_import_masters( let rows = parse_lab_master_rows(&xml, kind.tally_type()) .map_err(|code| ToolFailure::from(code).with_prior_evidence(evidence.clone()))?; for name in &requested { - if find_readback_row(&rows, name).is_some() { - collisions.push(format!("{}:{name}", kind.tally_type())); + if kind == MasterKind::Group && is_tally_reserved_root(name) { + // A requested Group named as Tally's own reserved-primary + // marker (raw or sanitized `\u{4}`/`\u{fffd}#4;` prefix) is + // never a real master to create: it *is* the root every + // company already has. It also never collision-matches by + // name -- Tally's own row is plainly "Primary", not the + // marker-decorated form a book may carry -- so without this + // check it would fall through as "new" and get Created with + // a garbled name. Refuse explicitly instead. + collisions.push(format!("{}:{name}:reserved_root", kind.tally_type())); + continue; } + let Some(existing) = find_readback_row(&rows, name) else { + continue; + }; + match kind { + MasterKind::Ledger => { + let observed_parent = existing.get("PARENT").map(String::as_str).unwrap_or(""); + if is_default_ledger(name, observed_parent) { + let book_ledger = masters + .ledgers + .iter() + .find(|l| canonical_master_key(&l.name) == canonical_master_key(name)) + .expect("name was drawn from kind.names(&masters)") + .clone(); + default_ledger_alters.push((book_ledger, existing.clone())); + continue; + } + } + MasterKind::Group if is_default_group(existing) => { + default_group_keys.insert(canonical_master_key(name)); + continue; + } + _ => {} + } + collisions.push(format!("{}:{name}", kind.tally_type())); } } if !collisions.is_empty() { @@ -646,12 +784,27 @@ pub(in crate::agent) async fn lab_import_masters( .with_prior_evidence(evidence)); } + // Masters actually sent as Create: every requested master minus the + // defaults just identified above (Creating an existing default would hit + // the very overwrite trap the pre-check exists to avoid). + let default_ledger_keys: BTreeSet = default_ledger_alters + .iter() + .map(|(ledger, _)| canonical_master_key(&ledger.name)) + .collect(); + let mut creatable = masters.clone(); + creatable + .ledgers + .retain(|l| !default_ledger_keys.contains(&canonical_master_key(&l.name))); + creatable + .groups + .retain(|g| !default_group_keys.contains(&canonical_master_key(&g.name))); + let mut batches = Vec::new(); let mut mismatches: Vec = Vec::new(); let mut counts = serde_json::Map::new(); 'kinds: for kind in MasterKind::IMPORT_ORDER { - let total = kind.count(&masters); + let total = kind.count(&creatable); if total == 0 { continue; } @@ -662,7 +815,7 @@ pub(in crate::agent) async fn lab_import_masters( let (_company, identity, admit_evidence) = admit_lab_target(server).await?; evidence = combine_evidence(evidence.clone(), admit_evidence); - let chunk_masters = chunked_masters(&masters, kind, chunk_start, MAX_MASTER_BATCH); + let chunk_masters = chunked_masters(&creatable, kind, chunk_start, MAX_MASTER_BATCH); let chunk_len = kind.count(&chunk_masters); let xml = render_master_batch_xml(identity.display_name(), kind, &chunk_masters); let (response, post_evidence) = @@ -708,6 +861,94 @@ pub(in crate::agent) async fn lab_import_masters( counts.insert(kind.tally_type().to_string(), json!(created)); } + // ---- Default-ledger partial Alter (Brain trap: Create on an existing + // ledger overwrites its opening balance; a partial Alter carrying only + // the changed field(s) is the safe write here). Only runs if nothing + // above already stopped on a mismatch, and only sends an Alter for + // ledgers whose book value actually differs from the target. ---- + if mismatches.is_empty() && !default_ledger_alters.is_empty() { + let to_alter: Vec<(&BookLedger, Vec<(&'static str, String)>)> = default_ledger_alters + .iter() + .map(|(ledger, row)| (ledger, ledger_alter_fields(ledger, row))) + .filter(|(_, fields)| !fields.is_empty()) + .collect(); + if !to_alter.is_empty() { + let (_company, identity, admit_evidence) = admit_lab_target(server).await?; + evidence = combine_evidence(evidence.clone(), admit_evidence); + let messages: String = to_alter + .iter() + .map(|(ledger, fields)| render_ledger_alter_xml(&ledger.name, fields)) + .collect(); + let xml = render_import_envelope(identity.display_name(), "All Masters", &messages); + let (response, post_evidence) = post_lab_batch( + server, + &identity, + "lab_import_masters.write.default_alter", + xml, + ) + .await?; + evidence = combine_evidence(evidence.clone(), post_evidence); + let outcome = bridge_tally_protocol::parse_import_outcome(&response) + .map_err(|_| ToolFailure::from("lab_import_response_invalid".to_string()))?; + let clean = outcome + .counters() + .is_clean_success_for(0, to_alter.len() as u64, 0); + counts.insert("LedgerDefaultAlter".to_string(), json!(to_alter.len())); + batches.push(json!({ + "kind": "LedgerDefaultAlter", + "requested": to_alter.len(), + "counters_clean": clean, + "ok": clean, + })); + if !clean { + mismatches.push( + "default ledger alter: import counters not a clean altered-only success" + .to_string(), + ); + } + } + + // Mandatory read-back over every default ledger, altered or not + // (plan: "include defaults in read-back diff") -- reuses the same + // `readback_mismatches` diff the ordinary Create batches use. + if mismatches.is_empty() { + let (_company, identity, admit_evidence) = admit_lab_target(server).await?; + evidence = combine_evidence(evidence.clone(), admit_evidence); + let read_request = + render_master_collection_request(identity.display_name(), MasterKind::Ledger) + .map_err(ToolFailure::from)?; + let (read_xml, read_evidence) = lab_post_read( + server, + &identity, + "lab_import_masters.readback.default", + read_request, + ) + .await?; + evidence = combine_evidence(evidence.clone(), read_evidence); + let rows = parse_lab_master_rows(&read_xml, MasterKind::Ledger.tally_type()) + .map_err(|code| ToolFailure::from(code).with_prior_evidence(evidence.clone()))?; + let default_book = BookMasters { + ledgers: default_ledger_alters + .iter() + .map(|(ledger, _)| ledger.clone()) + .collect(), + ..Default::default() + }; + let default_mismatches = readback_mismatches(MasterKind::Ledger, &default_book, &rows); + let default_ok = default_mismatches.is_empty(); + batches.push(json!({ + "kind": "LedgerDefaultReadback", + "requested": default_ledger_alters.len(), + "counters_clean": true, + "mismatches": default_mismatches, + "ok": default_ok, + })); + if !default_ok { + mismatches.extend(default_mismatches); + } + } + } + let ok = mismatches.is_empty(); Ok(ToolOutcome { payload: json!({"result": { diff --git a/src-tauri/src/agent_lab_import_tests.rs b/src-tauri/src/agent_lab_import_tests.rs index 93132b88..fea06a2a 100644 --- a/src-tauri/src/agent_lab_import_tests.rs +++ b/src-tauri/src/agent_lab_import_tests.rs @@ -708,3 +708,208 @@ fn amounts_equal_ignores_decimal_formatting_but_not_value() { assert!(amounts_equal("100.00", "100")); assert!(!amounts_equal("100.00", "100.01")); } + +// --------------------------------------------------------------------------- +// Tally default masters: default skip, default opening alter, true collision +// still refused, reserved parent normalisation. +// --------------------------------------------------------------------------- + +fn default_cash_ledger(opening: &str) -> BookLedger { + BookLedger { + name: "Cash".into(), + parent: Some("Cash-in-Hand".into()), + opening_balance: Some(opening.into()), + is_billwise_on: None, + party_gstin: None, + tax_type: None, + gst_duty_head: None, + opening_bill_allocations: vec![], + } +} + +fn default_pl_ledger() -> BookLedger { + BookLedger { + name: "Profit & Loss A/c".into(), + parent: None, + opening_balance: Some("0.00".into()), + is_billwise_on: None, + party_gstin: None, + tax_type: None, + gst_duty_head: None, + opening_bill_allocations: vec![], + } +} + +#[test] +fn is_default_ledger_recognises_cash_under_cash_in_hand() { + assert!(is_default_ledger("Cash", "Cash-in-Hand")); + // §9.4d fold applies to the parent comparison too. + assert!(is_default_ledger("Cash", "cash in hand")); +} + +#[test] +fn is_default_ledger_recognises_profit_and_loss_under_the_reserved_primary() { + // Sanitized form `tolerant_xml` actually produces for the raw U+0004 + // metadata prefix (see `bridge_tally_protocol::TALLY_SANITIZED_ROOT_MARKER`). + assert!(is_default_ledger( + "Profit & Loss A/c", + "\u{fffd}#4; Primary" + )); + assert!(is_default_ledger("Profit & Loss A/c", "Primary")); +} + +#[test] +fn is_default_ledger_rejects_a_same_name_ledger_under_a_different_parent() { + // A "Cash" ledger moved (or created by a user) under some other group is + // not Tally's own default -- it must remain a true collision, not be + // silently treated as the default and skipped. + assert!(!is_default_ledger("Cash", "Bank Accounts")); + assert!(!is_default_ledger( + "Profit & Loss A/c", + "Current Liabilities" + )); +} + +#[test] +fn is_default_ledger_does_not_recognise_an_unrelated_name() { + assert!(!is_default_ledger( + "Sri Ram Cables Private Limited", + "Primary" + )); +} + +#[test] +fn a_requested_group_named_as_the_reserved_root_marker_is_recognised_as_such() { + // The exact defect this pre-flight caught in the rehearsal book: a + // requested Group literally named with Tally's sanitized U+0004 marker + // (the self-referential root) must be recognised so the caller can + // refuse it, rather than silently Creating a garbled-name group -- it + // never collision-matches Tally's own plainly-named "Primary" row. + assert!(is_tally_reserved_root("\u{fffd}#4; Primary")); + assert!(is_tally_reserved_root("Primary")); + assert!(!is_tally_reserved_root("Sundry Debtors")); +} + +#[test] +fn is_default_group_reads_reserved_name_not_the_group_name() { + assert!(is_default_group(&row(&[ + ("NAME", "Sundry Debtors"), + ("RESERVEDNAME", "Sundry Debtors") + ]))); + // A user-created group with the same displayed name as a reserved one + // but an empty RESERVEDNAME is not a default. + assert!(!is_default_group(&row(&[ + ("NAME", "Sundry Debtors"), + ("RESERVEDNAME", "") + ]))); + assert!(!is_default_group(&row(&[("NAME", "Custom Group")]))); +} + +#[test] +fn ledger_alter_fields_is_empty_when_the_default_already_matches_the_book() { + // "default skip": no diff, no Alter is offered. + let l = default_cash_ledger("0.00"); + let observed = row(&[("PARENT", "Cash-in-Hand"), ("OPENINGBALANCE", "0.00")]); + assert!(ledger_alter_fields(&l, &observed).is_empty()); +} + +#[test] +fn ledger_alter_fields_offers_only_the_changed_opening_balance() { + // "default opening alter": book differs from target -> a partial Alter + // carrying only OPENINGBALANCE, never a Create (which would overwrite). + let l = default_cash_ledger("5000.00"); + let observed = row(&[("PARENT", "Cash-in-Hand"), ("OPENINGBALANCE", "0.00")]); + let fields = ledger_alter_fields(&l, &observed); + assert_eq!(fields, vec![("OPENINGBALANCE", "5000.00".to_string())]); +} + +#[test] +fn ledger_alter_fields_never_offers_a_gst_field_alter_9_4d() { + // §8.3: GST fields are settable at Create but silently dropped at Alter + // -- never offered here even when they differ from the target. + let mut l = default_cash_ledger("0.00"); + l.tax_type = Some("GST".into()); + l.gst_duty_head = Some("State Tax".into()); + let observed = row(&[ + ("PARENT", "Cash-in-Hand"), + ("OPENINGBALANCE", "0.00"), + ("TAXTYPE", "Others"), + ("GSTDUTYHEAD", "CGST"), + ]); + assert!(ledger_alter_fields(&l, &observed).is_empty()); +} + +#[test] +fn render_ledger_alter_xml_carries_only_the_given_fields() { + let xml = render_ledger_alter_xml("Cash", &[("OPENINGBALANCE", "5000.00".to_string())]); + assert_eq!( + xml, + "\ +5000.00" + ); + // Never a Create, and never a field beyond what was asked for. + assert!(!xml.contains("ACTION=\"Create\"")); + assert!(!xml.contains("PARENT")); +} + +#[test] +fn default_ledger_and_default_group_precheck_classification_end_to_end() { + // A compact end-to-end check of the precheck classification a real + // `lab_import_masters` call performs: for each requested master, decide + // default-skip vs. true-collision the same way the tool body does. + let requested_ledgers = [default_cash_ledger("5000.00"), default_pl_ledger()]; + let existing_ledger_rows = [ + row(&[ + ("NAME", "Cash"), + ("PARENT", "Cash-in-Hand"), + ("OPENINGBALANCE", "0.00"), + ]), + row(&[ + ("NAME", "Profit & Loss A/c"), + ("PARENT", "\u{fffd}#4; Primary"), + ("OPENINGBALANCE", "0.00"), + ]), + ]; + let mut collisions = Vec::new(); + let mut alters = Vec::new(); + for ledger in &requested_ledgers { + let existing = find_readback_row(&existing_ledger_rows, &ledger.name).unwrap(); + let observed_parent = existing.get("PARENT").map(String::as_str).unwrap_or(""); + if is_default_ledger(&ledger.name, observed_parent) { + alters.push((ledger.name.clone(), ledger_alter_fields(ledger, existing))); + } else { + collisions.push(ledger.name.clone()); + } + } + assert!(collisions.is_empty(), "both are real Tally defaults"); + assert_eq!(alters[0].0, "Cash"); + assert_eq!(alters[0].1, vec![("OPENINGBALANCE", "5000.00".to_string())]); + assert_eq!(alters[1].0, "Profit & Loss A/c"); + assert!( + alters[1].1.is_empty(), + "Profit & Loss A/c already matches -> default skip, no Alter" + ); + + // "true collision still refused": a non-default same-name ledger. + let existing_debtor_rows = vec![row(&[ + ("NAME", "Sri Ram Cables Private Limited"), + ("PARENT", "Sundry Debtors"), + ])]; + let requested_debtor = BookLedger { + name: "Sri Ram Cables Private Limited".into(), + parent: Some("Sundry Debtors".into()), + opening_balance: None, + is_billwise_on: None, + party_gstin: None, + tax_type: None, + gst_duty_head: None, + opening_bill_allocations: vec![], + }; + let existing = find_readback_row(&existing_debtor_rows, &requested_debtor.name).unwrap(); + let observed_parent = existing.get("PARENT").map(String::as_str).unwrap_or(""); + assert!( + !is_default_ledger(&requested_debtor.name, observed_parent), + "an ordinary pre-existing ledger is never treated as a default" + ); + let _ = requested_debtor.parent; // constructed only to exercise the classification above +} From 557d08e8d3d78cfac830a385e718a9ccb86ef405 Mon Sep 17 00:00:00 2001 From: t Date: Mon, 14 Sep 2026 10:32:23 +0530 Subject: [PATCH 09/14] fix(lab): proven-good master Create shape + explicit tally_rejected reporting The 2026-09-14 10:10 IST rehearsal sent 17 ledger Creates to BRIDGE REHEARSAL and Tally answered CREATED=0 ERRORS=0 EXCEPTIONS=17 (no mutation). Diffing the sent request against the shape this exact company's masters were live-created with on TallyPrime 7.1 (babul-masters-complete.xml) found four divergences, all fixed in the master renderers (ledgers, and the same conventions applied to groups/units/godowns/stock groups/stock items and the default-ledger partial Alter): - every renderer now includes a child mirroring the NAME attribute (missing before) - is now always explicit (defaulting to No), never omitted - / are only emitted when non-zero, matching the proven capture's zero-balance ledgers - is only emitted when the book value is a real GST/duty classification (not empty/"Others") and the ledger's parent is Duties & Taxes -- the rehearsal request sent TAXTYPE=Others on every ledger including a bank account and a wages ledger - xmlns:UDF="TallyUDF" is dropped everywhere: nothing here ever emits a UDF-namespaced element, so the declaration bound to nothing Also: lab_import_masters/lab_import_vouchers now check the import response's ERRORS/EXCEPTIONS counters immediately after posting, before the mandatory read-back, and report an outright rejection explicitly as state "tally_rejected" with the full counter set and any text -- instead of proceeding to a read-back whose only signal is "not found", indistinguishable from a request that was never sent. Golden XML tests updated to the proven shape, with three new fixtures derived byte-for-byte from babul-masters-complete.xml (HDFC Bank 1649, Sales, Sri Ram Cables Private Limited), plus a TAXTYPE-suppression test and coverage for the new tally_rejected reporting path using the exact captured rehearsal response. CODE ONLY -- no live Tally writes performed by this change. cargo test --features lab-writes: 985 lib + 4 + 2 passed, 0 failed cargo test (no feature): passed, 0 failed cargo clippy --features lab-writes --all-targets -- -D warnings: clean cargo clippy --all-targets -- -D warnings: clean (module is feature-gated out entirely without lab-writes) cargo fmt -- --check: clean Production write guards (agent_import.rs, agent_import_post.rs, tally/approved_import.rs): unchanged vs origin/master Release build (bridge_mcp --features lab-writes): sha256 485289ff2b2a4bafec0ddeb87ae8b8bca7b3ba295db56803dc4f15d2319905c5 Co-Authored-By: Claude Sonnet --- src-tauri/src/agent_lab_import.rs | 288 ++++++++++++++++++++---- src-tauri/src/agent_lab_import_tests.rs | 258 ++++++++++++++++++++- 2 files changed, 495 insertions(+), 51 deletions(-) diff --git a/src-tauri/src/agent_lab_import.rs b/src-tauri/src/agent_lab_import.rs index 341e4c24..a993091e 100644 --- a/src-tauri/src/agent_lab_import.rs +++ b/src-tauri/src/agent_lab_import.rs @@ -109,6 +109,36 @@ fn amounts_equal(a: &str, b: &str) -> bool { } } +/// Whether `value` is numerically zero (or blank/unparseable, which a master +/// renderer treats the same as zero -- nothing to report). Used to gate +/// `OPENINGBALANCE`/`OPENINGVALUE`: the proven-good capture +/// (`babul-masters-complete.xml`) only ever emits an opening amount element +/// when it is non-zero -- a zero-balance ledger's `` carries no +/// `OPENINGBALANCE` at all. +fn is_zero_amount(value: &str) -> bool { + match ExactDecimal::parse(value) { + Ok(amount) => amount.numeric_eq(&ExactDecimal::parse("0").expect("literal parses")), + Err(_) => value.trim().is_empty(), + } +} + +/// Whether a ledger's recorded `tax_type` is a real GST/duty classification +/// worth sending, as opposed to an empty value or Tally's own inert default +/// `"Others"` -- the shape that reached the gateway request in the 2026-09-14 +/// rehearsal sent `Others` on every ledger, including bank +/// and expense ledgers that are not duty heads at all. +fn is_real_gst_duty_type(tax_type: &str) -> bool { + let trimmed = tax_type.trim(); + !trimmed.is_empty() && !trimmed.eq_ignore_ascii_case("others") +} + +/// Whether `parent` resolves (via the §9.4d fold) to the `Duties & Taxes` +/// group -- `TAXTYPE` is only ever meaningful on a ledger actually parented +/// there. +fn is_duties_and_taxes_parent(parent: &str) -> bool { + canonical_master_key(parent) == canonical_master_key("Duties & Taxes") +} + // --------------------------------------------------------------------------- // Book model (input) -- see SP/specs/book_schema.md for the full schema. // --------------------------------------------------------------------------- @@ -450,8 +480,12 @@ fn render_ledger_alter_xml(name: &str, fields: &[(&'static str, String)]) -> Str .iter() .map(|(tag, value)| format!("<{tag}>{}", xml_escape(value))) .collect(); + // No `xmlns:UDF`: this partial Alter carries only plain writable fields + // (currently `OPENINGBALANCE`), never a `UDF:`-namespaced element, so the + // namespace declaration has nothing to bind to -- see module doc / proven + // shape (`babul-masters-complete.xml`) for the same convention on Create. format!( - "{body}", + "{body}", name = xml_escape(name) ) } @@ -468,12 +502,21 @@ fn render_import_envelope(company: &str, report_name: &str, messages: &str) -> S ) } +// No renderer below declares `xmlns:UDF="TallyUDF"`: none of them emit a +// `UDF:`-namespaced element (that would require a genuine User Defined +// Field, which this book model never carries), so the earlier blanket +// declaration bound to nothing. The proven-good capture +// (`babul-masters-complete.xml`) confirms an ordinary ledger Create carries +// no such attribute at all -- only one incidental `TALLYMESSAGE` in that +// capture (for a UDF-bearing ledger the source system emitted) has it. + fn render_unit_xml(u: &BookUnit) -> String { let simple = u.is_simple_unit.as_deref().unwrap_or("Yes"); let decimals = u.decimal_places.as_deref().unwrap_or("2"); + let name = xml_escape(&u.name); format!( - "{simple}{decimals}", - name = xml_escape(&u.name), + "{name}\ +{simple}{decimals}", simple = xml_escape(simple), decimals = xml_escape(decimals) ) @@ -481,26 +524,35 @@ fn render_unit_xml(u: &BookUnit) -> String { fn render_parented_xml(tag: &str, item: &BookNamedParent) -> String { let parent = item.parent.as_deref().unwrap_or("Primary"); + let name = xml_escape(&item.name); format!( - "<{tag} NAME=\"{name}\" ACTION=\"Create\">{parent}", + "<{tag} NAME=\"{name}\" ACTION=\"Create\">{name}{parent}", tag = tag, - name = xml_escape(&item.name), parent = xml_escape(parent) ) } fn render_ledger_xml(l: &BookLedger) -> String { let parent = l.parent.as_deref().unwrap_or("Primary"); + let name = xml_escape(&l.name); + // Explicit on every Create, defaulting the unspecified case to `No` -- + // the proven capture never omits it. + let billwise = format!( + "{}", + if l.is_billwise_on.unwrap_or(false) { + "Yes" + } else { + "No" + } + ); + // Only when non-zero: the proven capture's zero-balance ledgers (e.g. + // "Sales", "Wages and Salary") carry no `OPENINGBALANCE` element at all. let opening = l.opening_balance.as_deref().unwrap_or("0.00"); - let billwise = l - .is_billwise_on - .map(|b| { - format!( - "{}", - if b { "Yes" } else { "No" } - ) - }) - .unwrap_or_default(); + let opening_balance = if is_zero_amount(opening) { + String::new() + } else { + format!("{}", xml_escape(opening)) + }; // GST fields are passed through exactly as observed on the source ledger // (§8.3: `GSTDUTYHEAD` vocabulary is irregular, `State Tax` not `SGST`); // never synthesised. §8.3: settable at Create, silently not at Alter -- @@ -510,9 +562,15 @@ fn render_ledger_xml(l: &BookLedger) -> String { .as_deref() .map(|g| format!("{}", xml_escape(g))) .unwrap_or_default(); + // Only when the book actually carries a real GST/duty classification + // (not empty, not Tally's own inert default "Others") AND the ledger is + // parented under Duties & Taxes -- the 2026-09-14 rehearsal sent + // `Others` on every ledger, including "HDFC Bank + // 1649" and "Wages and Salary", which is not a duty head at all. let tax_type = l .tax_type .as_deref() + .filter(|t| is_real_gst_duty_type(t) && is_duties_and_taxes_parent(parent)) .map(|t| format!("{}", xml_escape(t))) .unwrap_or_default(); let duty_head = l @@ -537,26 +595,28 @@ fn render_ledger_xml(l: &BookLedger) -> String { }) .collect::(); format!( - "{parent}{opening}{billwise}{gstin}{tax_type}{duty_head}{opening_bills}", - name = xml_escape(&l.name), - parent = xml_escape(parent), - opening = xml_escape(opening) + "{name}\ +{parent}{billwise}{opening_balance}{gstin}{tax_type}{duty_head}{opening_bills}", + parent = xml_escape(parent) ) } fn render_stock_item_xml(s: &BookStockItem) -> String { let parent = s.parent.as_deref().unwrap_or("Primary"); + let name = xml_escape(&s.name); let base_units = s .base_unit .as_deref() .map(|u| format!("{}", xml_escape(u))) .unwrap_or_default(); + // Only when there is a genuinely non-zero opening value -- same + // zero-suppression convention as the ledger's `OPENINGBALANCE`. let opening = match ( s.opening_qty.as_deref(), s.opening_rate.as_deref(), s.opening_value.as_deref(), ) { - (Some(qty), Some(rate), Some(value)) => format!( + (Some(qty), Some(rate), Some(value)) if !is_zero_amount(value) => format!( "{}{}{}", xml_escape(qty), xml_escape(rate), xml_escape(value) ), @@ -573,8 +633,8 @@ fn render_stock_item_xml(s: &BookStockItem) -> String { .map(|h| format!("{}", xml_escape(h))) .unwrap_or_default(); format!( - "{parent}{base_units}{opening}{gst}{hsn}", - name = xml_escape(&s.name), + "{name}\ +{parent}{base_units}{opening}{gst}{hsn}", parent = xml_escape(parent) ) } @@ -823,9 +883,33 @@ pub(in crate::agent) async fn lab_import_masters( evidence = combine_evidence(evidence.clone(), post_evidence); let outcome = bridge_tally_protocol::parse_import_outcome(&response) .map_err(|_| ToolFailure::from("lab_import_response_invalid".to_string()))?; - let clean = outcome - .counters() - .is_clean_success_for(chunk_len as u64, 0, 0); + let counters = outcome.counters(); + + if tally_rejected(counters) { + // Tally refused the whole batch (e.g. the 2026-09-14 + // rehearsal's CREATED=0 ERRORS=0 EXCEPTIONS=17) -- report + // that explicitly, with any LINEERROR text, before the + // mandatory read-back rather than after it: a read-back can + // only ever say "not found", which does not distinguish a + // rejected write from one that was never sent. + let line_errors = extract_line_error_texts(&response); + batches.push(json!({ + "kind": kind.tally_type(), + "requested": chunk_len, + "state": "tally_rejected", + "counters": tally_import_counters_json(counters), + "line_errors": line_errors, + "ok": false, + })); + mismatches.push(tally_rejection_message( + kind.tally_type(), + counters, + &line_errors, + )); + break 'kinds; + } + + let clean = counters.is_clean_success_for(chunk_len as u64, 0, 0); // Mandatory read-back, regardless of the counters (§9.2: never // trust CREATED/ERRORS alone). @@ -890,21 +974,37 @@ pub(in crate::agent) async fn lab_import_masters( evidence = combine_evidence(evidence.clone(), post_evidence); let outcome = bridge_tally_protocol::parse_import_outcome(&response) .map_err(|_| ToolFailure::from("lab_import_response_invalid".to_string()))?; - let clean = outcome - .counters() - .is_clean_success_for(0, to_alter.len() as u64, 0); + let counters = outcome.counters(); counts.insert("LedgerDefaultAlter".to_string(), json!(to_alter.len())); - batches.push(json!({ - "kind": "LedgerDefaultAlter", - "requested": to_alter.len(), - "counters_clean": clean, - "ok": clean, - })); - if !clean { - mismatches.push( - "default ledger alter: import counters not a clean altered-only success" - .to_string(), - ); + if tally_rejected(counters) { + let line_errors = extract_line_error_texts(&response); + batches.push(json!({ + "kind": "LedgerDefaultAlter", + "requested": to_alter.len(), + "state": "tally_rejected", + "counters": tally_import_counters_json(counters), + "line_errors": line_errors, + "ok": false, + })); + mismatches.push(tally_rejection_message( + "LedgerDefaultAlter", + counters, + &line_errors, + )); + } else { + let clean = counters.is_clean_success_for(0, to_alter.len() as u64, 0); + batches.push(json!({ + "kind": "LedgerDefaultAlter", + "requested": to_alter.len(), + "counters_clean": clean, + "ok": clean, + })); + if !clean { + mismatches.push( + "default ledger alter: import counters not a clean altered-only success" + .to_string(), + ); + } } } @@ -1071,6 +1171,94 @@ fn chunked_masters( } } +// --------------------------------------------------------------------------- +// Explicit Tally-rejection reporting (2026-09-14 rehearsal: 17 ledgers sent, +// Tally answered CREATED=0 ERRORS=0 EXCEPTIONS=17, no mutation). A batch +// whose response reports ERRORS or EXCEPTIONS was rejected outright -- that +// must be reported as such, with whatever LINEERROR text Tally attached, +// instead of proceeding to the mandatory read-back and reporting only +// "not found on readback" (indistinguishable from a request that was never +// sent at all). +// --------------------------------------------------------------------------- + +/// Best-effort extraction of every `` element's text from a raw +/// import response. Deliberately separate from +/// `bridge_tally_protocol::ParsedImportEvidence`, which redacts this text by +/// design (retaining only a sha256 digest) for persisted evidence -- this is +/// a one-shot diagnostic surfaced directly in the tool's own JSON result, not +/// persisted evidence, so the raw text is exactly what a caller needs to act +/// on a rejection. +fn extract_line_error_texts(xml: &str) -> Vec { + let mut reader = quick_xml::Reader::from_str(xml); + reader.config_mut().trim_text(true); + let mut errors = Vec::new(); + let mut in_line_error = false; + loop { + match reader.read_event() { + Ok(quick_xml::events::Event::Start(event)) + if event.name().as_ref().eq_ignore_ascii_case(b"LINEERROR") => + { + in_line_error = true; + } + Ok(quick_xml::events::Event::End(event)) + if event.name().as_ref().eq_ignore_ascii_case(b"LINEERROR") => + { + in_line_error = false; + } + Ok(quick_xml::events::Event::Text(text)) if in_line_error => { + if let Ok(value) = decoded_agent_text(text) { + let trimmed = value.trim(); + if !trimmed.is_empty() { + errors.push(trimmed.to_string()); + } + } + } + Ok(quick_xml::events::Event::Eof) => break, + Ok(_) => {} + Err(_) => break, + } + } + errors +} + +fn tally_import_counters_json(counters: &bridge_tally_protocol::TallyImportResult) -> Value { + json!({ + "created": counters.created, + "altered": counters.altered, + "deleted": counters.deleted, + "ignored": counters.ignored, + "errors": counters.errors, + "cancelled": counters.cancelled, + "exceptions": counters.exceptions, + }) +} + +fn tally_rejection_message( + label: &str, + counters: &bridge_tally_protocol::TallyImportResult, + line_errors: &[String], +) -> String { + let suffix = if line_errors.is_empty() { + String::new() + } else { + format!(" LINEERROR: {}", line_errors.join("; ")) + }; + format!( + "{label} rejected by Tally: CREATED={} ALTERED={} ERRORS={} EXCEPTIONS={}{suffix}", + counters.created, counters.altered, counters.errors, counters.exceptions + ) +} + +/// Whether the response counters signal an outright rejection: any `ERRORS` +/// or `EXCEPTIONS` -- the exact shape the 2026-09-14 rehearsal produced +/// (`CREATED=0 ERRORS=0 EXCEPTIONS=17`). Checked ahead of, and independently +/// of, `is_clean_success_for`'s exact-count comparison so a rejection is +/// reported as `tally_rejected` rather than folded into an ordinary +/// mismatch. +fn tally_rejected(counters: &bridge_tally_protocol::TallyImportResult) -> bool { + counters.errors > 0 || counters.exceptions > 0 +} + fn persist_lab_precheck_collisions(server: &Server, collisions: &[String]) { if let Ok(dir) = lab_evidence_dir(server) { let record = json!({ @@ -1591,9 +1779,27 @@ pub(in crate::agent) async fn lab_import_vouchers( evidence = combine_evidence(evidence.clone(), post_evidence); let outcome = bridge_tally_protocol::parse_import_outcome(&response) .map_err(|_| ToolFailure::from("lab_import_response_invalid".to_string()))?; - let clean = outcome - .counters() - .is_clean_success_for(batch.len() as u64, 0, 0); + let counters = outcome.counters(); + + if tally_rejected(counters) { + // Same explicit-rejection reporting as lab_import_masters: a + // read-back after this can only ever say "not found", so report + // the rejection itself, with any LINEERROR text, and stop. + let line_errors = extract_line_error_texts(&response); + batch_reports.push(json!({ + "batch": batch_index, + "count": batch.len(), + "state": "tally_rejected", + "counters": tally_import_counters_json(counters), + "line_errors": line_errors, + "posted": true, + "source_guids": source_guids, + })); + stopped_at = Some(batch_index); + break; + } + + let clean = counters.is_clean_success_for(batch.len() as u64, 0, 0); // Mandatory read-back. let (readback_xml, readback_evidence) = lab_post_read( diff --git a/src-tauri/src/agent_lab_import_tests.rs b/src-tauri/src/agent_lab_import_tests.rs index fea06a2a..91a5ee05 100644 --- a/src-tauri/src/agent_lab_import_tests.rs +++ b/src-tauri/src/agent_lab_import_tests.rs @@ -69,7 +69,7 @@ fn unit_create_xml_golden() { }; assert_eq!( render_unit_xml(&u), - "\ + "Kgs\ Yes3" ); } @@ -82,17 +82,72 @@ fn godown_create_xml_golden() { }; assert_eq!( render_parented_xml("GODOWN", &g), - "\ + "Main Godown\ Primary" ); } +// Golden fixtures below are derived byte-for-byte from the proven-good, +// live-created shape captured in +// brain/50-projects/viniyug-fieldwork-2026-09-11/artifacts/Babul-Final-Import/ +// Babul-Rounded-2026-09-11/fresh-company-only/babul-masters-complete.xml +// (this exact company's masters were created with it on TallyPrime 7.1): +// `` mirrors the attribute, `ISBILLWISEON` is always explicit, +// `OPENINGBALANCE` appears only when the source ledger's balance is +// non-zero, and no `TALLYMESSAGE` declares `xmlns:UDF`. + +#[test] +fn ledger_create_xml_golden_babul_bank_ledger_with_negative_opening() { + // Babul's "HDFC Bank 1649": non-zero (negative) opening balance, no GST + // fields, ISBILLWISEON explicit No. + let l = BookLedger { + name: "HDFC Bank 1649".into(), + parent: Some("Bank Accounts".into()), + opening_balance: Some("-5013.35".into()), + is_billwise_on: Some(false), + party_gstin: None, + tax_type: None, + gst_duty_head: None, + opening_bill_allocations: vec![], + }; + assert_eq!( + render_ledger_xml(&l), + "\ +HDFC Bank 1649Bank AccountsNo\ +-5013.35" + ); +} + +#[test] +fn ledger_create_xml_golden_babul_zero_balance_ledger_omits_opening_balance() { + // Babul's "Sales": zero opening balance -- the proven capture carries no + // `OPENINGBALANCE` element at all for this ledger. + let l = BookLedger { + name: "Sales".into(), + parent: Some("Sales Accounts".into()), + opening_balance: Some("0.00".into()), + is_billwise_on: Some(false), + party_gstin: None, + tax_type: None, + gst_duty_head: None, + opening_bill_allocations: vec![], + }; + assert_eq!( + render_ledger_xml(&l), + "\ +SalesSales AccountsNo" + ); +} + #[test] -fn ledger_create_xml_golden_with_gst_and_billwise() { +fn ledger_create_xml_golden_babul_billwise_party_with_gstin() { + // Babul's "Sri Ram Cables Private Limited": ISBILLWISEON=Yes, zero + // opening balance (so still no OPENINGBALANCE), plus a GSTIN this + // module's own book model carries that the Babul capture itself did not. let l = BookLedger { name: "Sri Ram Cables Private Limited".into(), parent: Some("Sundry Debtors".into()), - opening_balance: Some("1000.00".into()), + opening_balance: Some("0.00".into()), is_billwise_on: Some(true), party_gstin: Some("27ZZZZZ0000Z1Z5".into()), tax_type: None, @@ -101,12 +156,30 @@ fn ledger_create_xml_golden_with_gst_and_billwise() { }; assert_eq!( render_ledger_xml(&l), - "\ -Sundry Debtors1000.00Yes\ -27ZZZZZ0000Z1Z5" + "\ +Sri Ram Cables Private LimitedSundry Debtors\ +Yes27ZZZZZ0000Z1Z5" ); } +#[test] +fn ledger_create_xml_billwise_defaults_to_no_when_unspecified() { + let l = BookLedger { + name: "Wages and Salary".into(), + parent: Some("Direct Expenses".into()), + opening_balance: None, + is_billwise_on: None, + party_gstin: None, + tax_type: None, + gst_duty_head: None, + opening_bill_allocations: vec![], + }; + let xml = render_ledger_xml(&l); + // ISBILLWISEON is explicit even though the book model left it unset. + assert!(xml.contains("No")); + assert!(!xml.contains("OPENINGBALANCE")); +} + #[test] fn ledger_create_xml_uses_the_irregular_9_4d_duty_head_vocabulary_verbatim() { // §8.3: the state head is "State Tax", never "SGST" -- passed through @@ -129,6 +202,42 @@ fn ledger_create_xml_uses_the_irregular_9_4d_duty_head_vocabulary_verbatim() { assert!(xml.contains("Duties & Taxes")); } +#[test] +fn ledger_create_xml_never_emits_taxtype_others() { + // The 2026-09-14 rehearsal bug: every ledger, including a bank account + // and a wages ledger, carried `Others` -- Tally's own + // inert default, not a real classification, and not appropriate outside + // Duties & Taxes. Tally answered CREATED=0 EXCEPTIONS=17. + let l = BookLedger { + name: "HDFC Bank 1649".into(), + parent: Some("Bank Accounts".into()), + opening_balance: Some("-5013.35".into()), + is_billwise_on: Some(false), + party_gstin: None, + tax_type: Some("Others".into()), + gst_duty_head: None, + opening_bill_allocations: vec![], + }; + assert!(!render_ledger_xml(&l).contains("TAXTYPE")); +} + +#[test] +fn ledger_create_xml_never_emits_taxtype_outside_duties_and_taxes() { + // A real, non-"Others" tax_type value must still be withheld if the + // ledger is not parented under Duties & Taxes. + let l = BookLedger { + name: "GST Paid".into(), + parent: Some("Loans & Advances (Asset)".into()), + opening_balance: None, + is_billwise_on: None, + party_gstin: None, + tax_type: Some("GST".into()), + gst_duty_head: None, + opening_bill_allocations: vec![], + }; + assert!(!render_ledger_xml(&l).contains("TAXTYPE")); +} + #[test] fn stock_item_create_xml_golden() { let s = BookStockItem { @@ -144,13 +253,32 @@ fn stock_item_create_xml_golden() { let xml = render_stock_item_xml(&s); assert_eq!( xml, - "\ -ChemicalsKgs100\ + "\ +Sodium BicarbonateChemicalsKgs\ +100\ 50.005000.00\ Applicable28362000" ); } +#[test] +fn stock_item_create_xml_omits_opening_balance_when_zero() { + let s = BookStockItem { + name: "Sample Item".into(), + parent: Some("Primary".into()), + base_unit: Some("Kgs".into()), + opening_qty: Some("0".into()), + opening_rate: Some("0".into()), + opening_value: Some("0.00".into()), + gst_applicable: None, + hsn_code: None, + }; + let xml = render_stock_item_xml(&s); + assert!(!xml.contains("OPENINGBALANCE")); + assert!(!xml.contains("OPENINGRATE")); + assert!(!xml.contains("OPENINGVALUE")); +} + // --------------------------------------------------------------------------- // Voucher XML -- golden fixtures, mirroring §9.13/§9.12a // --------------------------------------------------------------------------- @@ -844,7 +972,7 @@ fn render_ledger_alter_xml_carries_only_the_given_fields() { let xml = render_ledger_alter_xml("Cash", &[("OPENINGBALANCE", "5000.00".to_string())]); assert_eq!( xml, - "\ + "\ 5000.00" ); // Never a Create, and never a field beyond what was asked for. @@ -913,3 +1041,113 @@ fn default_ledger_and_default_group_precheck_classification_end_to_end() { ); let _ = requested_debtor.parent; // constructed only to exercise the classification above } + +// --------------------------------------------------------------------------- +// Explicit Tally-rejection reporting (2026-09-14 rehearsal: 17 ledgers sent, +// Tally answered CREATED=0 ERRORS=0 EXCEPTIONS=17, no mutation). +// --------------------------------------------------------------------------- + +/// The exact response Tally returned for the failing rehearsal (captured in +/// the request-evidence directory as this batch's `.response.xml`). +const REHEARSAL_REJECTION_RESPONSE: &str = "\ + 0\ + 0\ + 0\ + 0\ + 0\ + 0\ + 0\ + 0\ + 0\ + 17\ +"; + +const CLEAN_RESPONSE: &str = "\ + 17\ + 0\ + 0\ + 0\ + 0\ + 0\ + 0\ + 0\ + 0\ + 0\ +"; + +#[test] +fn tally_rejected_true_for_the_exact_rehearsal_response() { + let outcome = bridge_tally_protocol::parse_import_outcome(REHEARSAL_REJECTION_RESPONSE) + .expect("valid RESPONSE shape"); + assert!(tally_rejected(outcome.counters())); + assert_eq!(outcome.counters().created, 0); + assert_eq!(outcome.counters().exceptions, 17); +} + +#[test] +fn tally_rejected_false_for_a_clean_response() { + let outcome = + bridge_tally_protocol::parse_import_outcome(CLEAN_RESPONSE).expect("valid RESPONSE shape"); + assert!(!tally_rejected(outcome.counters())); +} + +#[test] +fn tally_rejected_true_when_errors_reported_even_with_zero_exceptions() { + let response = "000\ +0000\ +200"; + let outcome = + bridge_tally_protocol::parse_import_outcome(response).expect("valid RESPONSE shape"); + assert!(tally_rejected(outcome.counters())); +} + +#[test] +fn tally_rejection_message_reports_counters_and_no_line_errors_when_absent() { + let outcome = bridge_tally_protocol::parse_import_outcome(REHEARSAL_REJECTION_RESPONSE) + .expect("valid RESPONSE shape"); + let line_errors = extract_line_error_texts(REHEARSAL_REJECTION_RESPONSE); + assert!( + line_errors.is_empty(), + "the captured rehearsal response carried no LINEERROR text" + ); + let message = tally_rejection_message("Ledger", outcome.counters(), &line_errors); + assert_eq!( + message, + "Ledger rejected by Tally: CREATED=0 ALTERED=0 ERRORS=0 EXCEPTIONS=17" + ); + assert!(!message.contains("LINEERROR")); +} + +#[test] +fn extract_line_error_texts_reads_every_lineerror_element() { + let response = "000\ +0000\ +002\ +Could not set OPENINGBALANCE : Duplicate name\ +Vch/Ledger deletion/alteration is not permitted"; + let errors = extract_line_error_texts(response); + assert_eq!( + errors, + vec![ + "Could not set OPENINGBALANCE : Duplicate name".to_string(), + "Vch/Ledger deletion/alteration is not permitted".to_string(), + ] + ); + let outcome = + bridge_tally_protocol::parse_import_outcome(response).expect("valid RESPONSE shape"); + let message = tally_rejection_message("Ledger", outcome.counters(), &errors); + assert!(message.contains( + "LINEERROR: Could not set OPENINGBALANCE : Duplicate name; \ +Vch/Ledger deletion/alteration is not permitted" + )); +} + +#[test] +fn tally_import_counters_json_surfaces_every_counter() { + let outcome = bridge_tally_protocol::parse_import_outcome(REHEARSAL_REJECTION_RESPONSE) + .expect("valid RESPONSE shape"); + let json = tally_import_counters_json(outcome.counters()); + assert_eq!(json["created"], 0); + assert_eq!(json["errors"], 0); + assert_eq!(json["exceptions"], 17); +} From 4ea1220c43ab6b59d7f59c9f24a4493a8ce0b634 Mon Sep 17 00:00:00 2001 From: t Date: Mon, 14 Sep 2026 10:54:32 +0530 Subject: [PATCH 10/14] fix(lab): decode entity references in read-back parsers + idempotent master resume Live rehearsal follow-up (2026-09-14): the prior commit's shape fix worked -- Tally CREATED all 17 ledgers, counters clean -- but the run then stopped on false mismatches from the lab read-back parser: parents observed as "Duties Taxes" / "Loans Advances (Asset)" (note the double space -- the `&` entity dropped entirely, not left literal). Production `ledger_masters` against the same company correctly reports "Duties & Taxes"; the bug was local to the lab read-back parsers. Root cause: quick_xml delivers a general entity/character reference (`&`, ``, ...) as its own `Event::GeneralRef`, separate from the surrounding `Event::Text` events -- every other native-collection parser in this crate (agent_voucher_parse.rs, agent_change_parse.rs, agent_company_checkpoint.rs, source_draft_xml.rs) already handles this event via `decoded_agent_reference`; the lab read-back parsers did not, so the reference was silently dropped by their catch-all match arm. Fixed in all three lab read-back parsers (agent_lab.rs's parse_lab_master_rows and parse_lab_inventory_vouchers, agent_lab_import.rs's parse_voucher_readback_nested) by adding the identical GeneralRef-handling arm the production parsers already use. find_readback_row/readback_mismatches needed no change -- they consume already-decoded rows, so fixing the parser fixes them too. Also, idempotent resume for lab_import_masters: a same-name master already in the target is no longer an automatic collision. If it matches the book on every field this tool would itself have written (parent, bill-wise flag, opening balance, and GST fields only where the book specifies a real classification -- new already_present_verified_mismatches/ledger_already_present_mismatches, reusing diff_unit/diff_parented/diff_stock_item for non-ledger kinds), it is reported as already_present_verified and excluded from the Create batch rather than refused or re-Created. Any field difference still falls through to the ordinary lab_master_already_exists refusal. An all-already_present_verified result still reports ok:true with nothing new created, so a resumed run proceeds straight to vouchers. run_rehearsal.py (Brain, not part of this repo): added operator-visible logging for the already_present_verified/idempotent-resume case: the existing `ok` gate already treats it as success, this only narrates it instead of looking like a suspiciously quiet masters step. New tests: entity-decoding coverage for parent names (master read-back, using the exact "Duties & Taxes"/"Loans & Advances (Asset)" case) and for party/ledger names and narration (voucher read-back); idempotent- resume coverage for a clean match, a bill-wise mismatch, an opening- balance mismatch, a real-GST-type mismatch, TAXTYPE="Others" correctly ignored outside Duties & Taxes, a non-ledger kind via the shared diff functions, and an end-to-end classification test mirroring the precheck loop for both the match and the mismatch path. CODE ONLY -- no live Tally writes performed by this change. cargo test --features lab-writes --lib: 994 passed, 0 failed (77 in agent::lab::*::tests, up from 68) cargo test --features lab-writes (full): passed, 0 failed cargo clippy --features lab-writes --all-targets -- -D warnings: clean cargo clippy --all-targets -- -D warnings: clean (lab module is feature-gated out entirely without lab-writes) cargo fmt -- --check: clean Production write guards (agent_import.rs, agent_import_post.rs, tally/approved_import.rs): unchanged vs origin/master Release build (bridge_mcp --features lab-writes): sha256 1abab7b71b99780cb8cc087616171815ffe7ec55e1491ce28dfe9fbbf20bca43 Co-Authored-By: Claude Sonnet --- src-tauri/src/agent_lab.rs | 67 ++++++++ src-tauri/src/agent_lab_import.rs | 197 +++++++++++++++++++++- src-tauri/src/agent_lab_import_tests.rs | 213 ++++++++++++++++++++++++ 3 files changed, 472 insertions(+), 5 deletions(-) diff --git a/src-tauri/src/agent_lab.rs b/src-tauri/src/agent_lab.rs index a65214bb..af3f58a0 100644 --- a/src-tauri/src/agent_lab.rs +++ b/src-tauri/src/agent_lab.rs @@ -369,6 +369,22 @@ fn parse_lab_master_rows( } } } + // See the identical arm in `agent_lab_import.rs`'s + // `parse_voucher_readback_nested`: quick_xml delivers an entity + // reference (`&`, ...) as its own `GeneralRef` event, not + // inline within `Text`. Without this arm it is silently dropped + // by the catch-all below -- the exact 2026-09-14 rehearsal bug + // that read "Duties & Taxes" back as "Duties Taxes". + Ok(quick_xml::events::Event::GeneralRef(reference)) => { + let is_row_field = path.len() == 6 + && path[..4] == ["ENVELOPE", "BODY", "DATA", "COLLECTION"] + && path[4] == row_tag; + if is_row_field { + if let Some(row) = current.as_mut() { + append_agent_text(row, ¤t_tag, decoded_agent_reference(reference)?); + } + } + } Ok(quick_xml::events::Event::End(event)) => { let end = String::from_utf8_lossy(event.name().as_ref()).to_ascii_uppercase(); if path.last().map(String::as_str) == Some(end.as_str()) @@ -513,6 +529,23 @@ fn parse_lab_inventory_vouchers(xml: &str) -> Result, String> { } } } + // Same entity-reference gap as `parse_lab_master_rows` above. + Ok(quick_xml::events::Event::GeneralRef(reference)) => { + let value = decoded_agent_reference(reference)?; + if path_is(&path[..path.len().saturating_sub(1)], &BATCH_PREFIX) { + if let Some(row) = batch.as_mut() { + append_agent_text(row, ¤t_tag, value); + } + } else if path_is(&path[..path.len().saturating_sub(1)], &ENTRY_PREFIX) { + if let Some(row) = entry.as_mut() { + append_agent_text(row, ¤t_tag, value); + } + } else if path_is(&path[..path.len().saturating_sub(1)], &VOUCHER_PREFIX) { + if let Some(row) = voucher.as_mut() { + append_agent_text(row, ¤t_tag, value); + } + } + } Ok(quick_xml::events::Event::End(event)) => { let end = String::from_utf8_lossy(event.name().as_ref()).to_ascii_uppercase(); let closing_batch = end == "BATCHALLOCATIONS.LIST" && path_is(&path, &BATCH_PREFIX); @@ -801,6 +834,40 @@ mod tests { assert_eq!(json["hsn_code"], "28362000"); } + #[test] + fn master_readback_decodes_entities_in_parent_names() { + // 2026-09-14 coordinator finding, live rehearsal: the target's + // ledgers read back with PARENT "Duties Taxes" / "Loans Advances + // (Asset)" (note the double space -- the `&` entity dropped + // entirely, not merely left literal) while production `ledger_masters` + // against the same company correctly reported "Duties & Taxes". + // Root cause: quick_xml delivers `&` as its own `GeneralRef` + // event, separate from the surrounding `Text` events, and this + // parser had no arm for it. + let xml = "
1
\ +Loans & Advances (Asset)\ +0.00\ +Duties & Taxes\ +GST\ +"; + let rows = parse_lab_master_rows(xml, "Ledger").unwrap(); + assert_eq!(rows.len(), 2); + assert_eq!( + rows[0].get("PARENT").map(String::as_str), + Some("Loans & Advances (Asset)") + ); + assert_eq!( + rows[1].get("PARENT").map(String::as_str), + Some("Duties & Taxes") + ); + // Never the entity literal, and never dropped to a bare double space. + for row in &rows { + let parent = row.get("PARENT").unwrap(); + assert!(!parent.contains("&")); + assert!(!parent.contains(" ")); + } + } + #[test] fn a_non_row_child_of_collection_is_refused() { let xml = "
1
\ diff --git a/src-tauri/src/agent_lab_import.rs b/src-tauri/src/agent_lab_import.rs index a993091e..6ba4c2d1 100644 --- a/src-tauri/src/agent_lab_import.rs +++ b/src-tauri/src/agent_lab_import.rs @@ -760,6 +760,122 @@ fn diff_stock_item(s: &BookStockItem, row: &BTreeMap) -> Vec, +) -> Vec { + let mut mismatches = diff_ledger(l, row); + let expected_billwise = if l.is_billwise_on.unwrap_or(false) { + "Yes" + } else { + "No" + }; + let observed_billwise = row.get("ISBILLWISEON").map(String::as_str).unwrap_or(""); + if !observed_billwise.eq_ignore_ascii_case(expected_billwise) { + mismatches.push(format!( + "ledger {}: is_billwise_on expected {expected_billwise}, observed {observed_billwise:?}", + l.name + )); + } + // Only compared when the renderer would actually have sent it (real + // GST/duty type, parent resolves to Duties & Taxes) -- see + // `render_ledger_xml`'s identical gate. Tally's own inert default + // (`Others`) is never a mismatch source here. + let parent = l.parent.as_deref().unwrap_or("Primary"); + if let Some(expected) = l.tax_type.as_deref() { + if is_real_gst_duty_type(expected) && is_duties_and_taxes_parent(parent) { + let observed = row.get("TAXTYPE").map(String::as_str).unwrap_or(""); + if expected != observed { + mismatches.push(format!( + "ledger {}: tax_type expected {expected:?}, observed {observed:?}", + l.name + )); + } + } + } + if let Some(expected) = l.gst_duty_head.as_deref() { + let observed = row.get("GSTDUTYHEAD").map(String::as_str).unwrap_or(""); + if expected != observed { + mismatches.push(format!( + "ledger {}: gst_duty_head expected {expected:?}, observed {observed:?}", + l.name + )); + } + } + mismatches +} + +/// Dispatches to the right per-kind comparison for the idempotent-resume +/// precheck. Reuses the same diff functions the post-Create read-back check +/// uses (`diff_unit`/`diff_parented`/`diff_stock_item`) for every kind +/// except Ledger, which needs the wider `ledger_already_present_mismatches` +/// above. An empty result means "safe to skip"; any entry means "still a +/// real collision, refuse". +fn already_present_verified_mismatches( + kind: MasterKind, + masters: &BookMasters, + name: &str, + row: &BTreeMap, +) -> Vec { + let key = canonical_master_key(name); + match kind { + MasterKind::Unit => masters + .units + .iter() + .find(|u| canonical_master_key(&u.name) == key) + .map(|u| diff_unit(u, row)) + .unwrap_or_default(), + MasterKind::Godown => masters + .godowns + .iter() + .find(|g| canonical_master_key(&g.name) == key) + .map(|g| diff_parented("godown", g, row)) + .unwrap_or_default(), + MasterKind::StockGroup => masters + .stock_groups + .iter() + .find(|g| canonical_master_key(&g.name) == key) + .map(|g| diff_parented("stock group", g, row)) + .unwrap_or_default(), + MasterKind::Group => masters + .groups + .iter() + .find(|g| canonical_master_key(&g.name) == key) + .map(|g| diff_parented("group", g, row)) + .unwrap_or_default(), + MasterKind::Ledger => masters + .ledgers + .iter() + .find(|l| canonical_master_key(&l.name) == key) + .map(|l| ledger_already_present_mismatches(l, row)) + .unwrap_or_default(), + MasterKind::StockItem => masters + .stock_items + .iter() + .find(|s| canonical_master_key(&s.name) == key) + .map(|s| diff_stock_item(s, row)) + .unwrap_or_default(), + } +} + // --------------------------------------------------------------------------- // lab_import_masters // --------------------------------------------------------------------------- @@ -787,6 +903,12 @@ pub(in crate::agent) async fn lab_import_masters( let mut collisions: Vec = Vec::new(); let mut default_ledger_alters: Vec<(BookLedger, BTreeMap)> = Vec::new(); let mut default_group_keys: BTreeSet = BTreeSet::new(); + // Idempotent resume (coordinator instruction, 2026-09-14): a same-name + // master already in the target, verified equal to the book on every + // field this tool would itself write, is not a collision -- see + // `already_present_verified_mismatches` above. + let mut already_present_verified: Vec = Vec::new(); + let mut already_present_keys: BTreeSet<(&'static str, String)> = BTreeSet::new(); for kind in MasterKind::IMPORT_ORDER { let requested = kind.names(&masters); if requested.is_empty() { @@ -835,6 +957,20 @@ pub(in crate::agent) async fn lab_import_masters( } _ => {} } + // Not a Tally default -- a genuine same-name master. Before + // refusing, check whether it already matches the book on every + // field this tool would itself have written: if so, a prior run + // already created it (successfully, or up to this point before + // stopping elsewhere), and resuming must not refuse or re-Create + // it. Any field difference still falls through to the ordinary + // refusal below. + let already_mismatches = + already_present_verified_mismatches(kind, &masters, name, existing); + if already_mismatches.is_empty() { + already_present_verified.push(format!("{}:{name}", kind.tally_type())); + already_present_keys.insert((kind.tally_type(), canonical_master_key(name))); + continue; + } collisions.push(format!("{}:{name}", kind.tally_type())); } } @@ -846,18 +982,36 @@ pub(in crate::agent) async fn lab_import_masters( // Masters actually sent as Create: every requested master minus the // defaults just identified above (Creating an existing default would hit - // the very overwrite trap the pre-check exists to avoid). + // the very overwrite trap the pre-check exists to avoid) and minus + // whatever the idempotent-resume check above already verified present. let default_ledger_keys: BTreeSet = default_ledger_alters .iter() .map(|(ledger, _)| canonical_master_key(&ledger.name)) .collect(); + let already_present = |kind: MasterKind, name: &str| { + already_present_keys.contains(&(kind.tally_type(), canonical_master_key(name))) + }; let mut creatable = masters.clone(); creatable - .ledgers - .retain(|l| !default_ledger_keys.contains(&canonical_master_key(&l.name))); + .units + .retain(|u| !already_present(MasterKind::Unit, &u.name)); + creatable + .godowns + .retain(|g| !already_present(MasterKind::Godown, &g.name)); creatable - .groups - .retain(|g| !default_group_keys.contains(&canonical_master_key(&g.name))); + .stock_groups + .retain(|g| !already_present(MasterKind::StockGroup, &g.name)); + creatable.groups.retain(|g| { + !default_group_keys.contains(&canonical_master_key(&g.name)) + && !already_present(MasterKind::Group, &g.name) + }); + creatable.ledgers.retain(|l| { + !default_ledger_keys.contains(&canonical_master_key(&l.name)) + && !already_present(MasterKind::Ledger, &l.name) + }); + creatable + .stock_items + .retain(|s| !already_present(MasterKind::StockItem, &s.name)); let mut batches = Vec::new(); let mut mismatches: Vec = Vec::new(); @@ -1056,6 +1210,14 @@ pub(in crate::agent) async fn lab_import_masters( "counts": counts, "batches": batches, "mismatches": mismatches, + // Idempotent-resume precheck: same-name masters already present + // in the target and verified equal to the book, so skipped + // rather than refused or re-Created. Non-empty even on a run + // that creates nothing new -- `ok` is still true in that case + // (an all-already_present_verified masters result is success, + // not a no-op failure), so a caller resuming after a prior + // successful write proceeds straight to vouchers. + "already_present_verified": already_present_verified, }}), evidence, company_guid: Some(guid.to_string()), @@ -1606,6 +1768,31 @@ fn parse_voucher_readback_nested(xml: &str) -> Result, Stri } } } + // quick_xml delivers a general entity/character reference + // (`&`, ``, ...) as its own `GeneralRef` event, separate + // from the surrounding `Text` events -- NOT inline within them. + // Without this arm the reference is silently dropped by the + // catch-all below, which is exactly the 2026-09-14 rehearsal + // read-back bug: "Duties & Taxes" arrived as two Text events + // ("Duties " and " Taxes") with the entity between them + // discarded, producing the false mismatch "Duties Taxes". Every + // other native-collection parser in this crate + // (agent_voucher_parse.rs, agent_change_parse.rs, + // agent_company_checkpoint.rs, source_draft_xml.rs) already + // handles this event; the lab read-back path did not. + Ok(quick_xml::events::Event::GeneralRef(reference)) => { + let value = decoded_agent_reference(reference)?; + let parent = &path[..path.len().saturating_sub(1)]; + if parent == ENTRY_PREFIX { + if let Some(row) = entry.as_mut() { + append_agent_text(row, ¤t_tag, value); + } + } else if parent == VOUCHER_PREFIX { + if let Some(row) = voucher.as_mut() { + append_agent_text(row, ¤t_tag, value); + } + } + } Ok(quick_xml::events::Event::End(event)) => { let end = String::from_utf8_lossy(event.name().as_ref()).to_ascii_uppercase(); if end == "ALLLEDGERENTRIES.LIST" && path.as_slice() == ENTRY_PREFIX { diff --git a/src-tauri/src/agent_lab_import_tests.rs b/src-tauri/src/agent_lab_import_tests.rs index 91a5ee05..85929eb2 100644 --- a/src-tauri/src/agent_lab_import_tests.rs +++ b/src-tauri/src/agent_lab_import_tests.rs @@ -726,6 +726,36 @@ fn a_non_voucher_child_of_collection_is_refused() { ); } +#[test] +fn voucher_readback_decodes_entities_in_party_ledger_and_narration() { + // 2026-09-14 coordinator finding: quick_xml delivers `&` as its own + // `GeneralRef` event, separate from the surrounding `Text` events. Before + // this fix that reference was silently dropped, so "Fixture Acid & Sons" + // read back as "Fixture Acid Sons" (two spaces, the entity gone) -- + // producing a false parent/party mismatch on a target Tally itself + // reports correctly. + let xml = "
1
\ +2026040560Sales\ +Fixture Acid & Sonsg-2No\ +Invoice for R & D chemicals [BRIDGE-LAB:abc]\ +Duties & TaxesNo900.00\ +Fixture Acid & SonsYes-900.00\ +"; + let rows = parse_voucher_readback_nested(xml).unwrap(); + assert_eq!(rows.len(), 1); + assert_eq!( + rows[0].narration.as_deref(), + Some("Invoice for R & D chemicals [BRIDGE-LAB:abc]") + ); + assert_eq!(rows[0].ledger_entries[0].0, "Duties & Taxes"); + assert_eq!(rows[0].ledger_entries[1].0, "Fixture Acid & Sons"); + // Never the entity literal, and never dropped to a bare double space. + for entry in &rows[0].ledger_entries { + assert!(!entry.0.contains("&")); + assert!(!entry.0.contains(" ")); + } +} + // --------------------------------------------------------------------------- // parse_book_value: inline vs book_path // --------------------------------------------------------------------------- @@ -980,6 +1010,189 @@ fn render_ledger_alter_xml_carries_only_the_given_fields() { assert!(!xml.contains("PARENT")); } +// --------------------------------------------------------------------------- +// Idempotent-resume precheck (coordinator instruction, 2026-09-14): the live +// rehearsal's ledgers were genuinely CREATED. Re-running `lab_import_masters` +// against the same book must not refuse them as collisions -- it must +// recognise them as already present and verified, and let the run proceed. +// --------------------------------------------------------------------------- + +fn matching_book_ledger() -> BookLedger { + BookLedger { + name: "Bank Charges".into(), + parent: Some("Indirect Expenses".into()), + opening_balance: Some("0.00".into()), + is_billwise_on: Some(false), + party_gstin: None, + tax_type: None, + gst_duty_head: None, + opening_bill_allocations: vec![], + } +} + +#[test] +fn already_present_verified_mismatches_is_empty_when_the_target_matches_the_book() { + let l = matching_book_ledger(); + // Exactly what render_ledger_xml would have sent for this ledger, as + // Tally would read it back: ISBILLWISEON explicit, no OPENINGBALANCE row + // (zero), no TAXTYPE (Tally's own inert default aside). + let observed = row(&[ + ("PARENT", "Indirect Expenses"), + ("ISBILLWISEON", "No"), + ("OPENINGBALANCE", "0.00"), + ("TAXTYPE", "Others"), + ]); + assert!(ledger_already_present_mismatches(&l, &observed).is_empty()); + let masters = BookMasters { + ledgers: vec![l], + ..Default::default() + }; + assert!(already_present_verified_mismatches( + MasterKind::Ledger, + &masters, + "Bank Charges", + &observed + ) + .is_empty()); +} + +#[test] +fn already_present_verified_mismatches_flags_a_bill_wise_flag_difference() { + let mut l = matching_book_ledger(); + l.is_billwise_on = Some(true); + let observed = row(&[ + ("PARENT", "Indirect Expenses"), + ("ISBILLWISEON", "No"), + ("OPENINGBALANCE", "0.00"), + ]); + let mismatches = ledger_already_present_mismatches(&l, &observed); + assert!( + mismatches.iter().any(|m| m.contains("is_billwise_on")), + "expected a bill-wise mismatch, got {mismatches:?}" + ); +} + +#[test] +fn already_present_verified_mismatches_flags_a_wrong_opening_balance() { + let l = matching_book_ledger(); + let observed = row(&[ + ("PARENT", "Indirect Expenses"), + ("ISBILLWISEON", "No"), + ("OPENINGBALANCE", "500.00"), + ]); + let mismatches = ledger_already_present_mismatches(&l, &observed); + assert!( + mismatches.iter().any(|m| m.contains("opening_balance")), + "expected an opening_balance mismatch, got {mismatches:?}" + ); +} + +#[test] +fn already_present_verified_mismatches_flags_a_real_gst_duty_type_difference() { + let mut l = matching_book_ledger(); + l.parent = Some("Duties & Taxes".into()); + l.tax_type = Some("GST".into()); + let observed = row(&[ + ("PARENT", "Duties & Taxes"), + ("ISBILLWISEON", "No"), + ("OPENINGBALANCE", "0.00"), + ("TAXTYPE", "VAT"), // wrong -- the target carries a different value + ]); + let mismatches = ledger_already_present_mismatches(&l, &observed); + assert!( + mismatches.iter().any(|m| m.contains("tax_type")), + "expected a tax_type mismatch, got {mismatches:?}" + ); +} + +#[test] +fn already_present_verified_mismatches_ignores_taxtype_others_outside_duties_and_taxes() { + // The book never asked for a GST classification here (tax_type is + // "Others"/absent and the parent is not Duties & Taxes), so Tally's own + // default TAXTYPE on the observed row -- present on every ledger, + // GST-relevant or not -- must never be treated as a mismatch. + let l = matching_book_ledger(); + let observed = row(&[ + ("PARENT", "Indirect Expenses"), + ("ISBILLWISEON", "No"), + ("OPENINGBALANCE", "0.00"), + ("TAXTYPE", "Others"), + ]); + assert!(ledger_already_present_mismatches(&l, &observed).is_empty()); +} + +#[test] +fn already_present_verified_mismatches_covers_non_ledger_kinds_via_the_existing_diff_functions() { + let masters = BookMasters { + groups: vec![BookNamedParent { + name: "Sundry Debtors (Retail)".into(), + parent: Some("Sundry Debtors".into()), + }], + ..Default::default() + }; + let matching = row(&[("PARENT", "Sundry Debtors")]); + assert!(already_present_verified_mismatches( + MasterKind::Group, + &masters, + "Sundry Debtors (Retail)", + &matching + ) + .is_empty()); + let mismatching = row(&[("PARENT", "Sundry Creditors")]); + assert!(!already_present_verified_mismatches( + MasterKind::Group, + &masters, + "Sundry Debtors (Retail)", + &mismatching + ) + .is_empty()); +} + +#[test] +fn already_present_verified_classification_end_to_end() { + // Mirrors `lab_import_masters`'s precheck loop for the case that matters + // most after the 2026-09-14 rehearsal: an ordinary (non-default) + // same-name ledger already in the target. If it matches the book on + // every field this tool writes, resuming must skip it, not refuse it + // (`already_present_verified`); if it differs, it must still fall + // through to the ordinary `lab_master_already_exists` collision. + let masters = BookMasters { + ledgers: vec![matching_book_ledger()], + ..Default::default() + }; + // Not a Tally default, so the loop's default-ledger branch never fires + // for it -- it reaches the idempotent-resume check either way. + assert!(!is_default_ledger("Bank Charges", "Indirect Expenses")); + + let matching_row = row(&[ + ("NAME", "Bank Charges"), + ("PARENT", "Indirect Expenses"), + ("ISBILLWISEON", "No"), + ("OPENINGBALANCE", "0.00"), + ]); + assert!(already_present_verified_mismatches( + MasterKind::Ledger, + &masters, + "Bank Charges", + &matching_row + ) + .is_empty()); + + let mismatched_row = row(&[ + ("NAME", "Bank Charges"), + ("PARENT", "Direct Expenses"), // wrong parent -- a real difference + ("ISBILLWISEON", "No"), + ("OPENINGBALANCE", "0.00"), + ]); + assert!(!already_present_verified_mismatches( + MasterKind::Ledger, + &masters, + "Bank Charges", + &mismatched_row + ) + .is_empty()); +} + #[test] fn default_ledger_and_default_group_precheck_classification_end_to_end() { // A compact end-to-end check of the precheck classification a real From 4f370571d67aca32e0a5c838d4dc5c38721bf9b5 Mon Sep 17 00:00:00 2001 From: t Date: Mon, 14 Sep 2026 11:31:59 +0530 Subject: [PATCH 11/14] fix(lab): normalise reserved-root spellings + ledger reconcile via partial Alter MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Second live rehearsal (2026-09-14): the prior commit's entity-decoding fix worked -- lab read-back no longer drops the & entity -- but the run then refused with lab_master_already_exists on Ledger:Profit & Loss A/c. Root cause: that same fix now correctly decodes the XML numeric reference  to its literal Unicode scalar value (raw U+0004), but is_default_ledger's reserved-primary check called bridge_tally_protocol::is_tally_reserved_root, which only strips its own sanitized placeholder ("\u{fffd}#4;"), not the raw control character -- so a live-correct read-back of "\u{4} Primary" no longer matched, and Profit & Loss A/c was misclassified as a true collision instead of Tally's own default. book.json separately carries a third spelling ("\u{fffd}#4; Primary", literally that placeholder text) via its own Python-side reimplementation. Fixed with is_reserved_root_any_spelling, a new function local to agent_lab_import.rs recognising all three spellings (raw control character, sanitized placeholder, undecoded "" text) and wired into both of this module's reserved-root comparisons. Deliberately NOT a change to bridge_tally_protocol::is_tally_reserved_root itself: that function's narrower definition is a considered, tested choice for the production group-ancestry walk (group_ancestry.rs's own every_refusal_is_distinguishable_and_none_is_an_answer test pins an unrecognised raw marker there as a safe refusal), so widening the shared function would have silently changed behaviour for every one of its other consumers. build_book.py's own is_tally_reserved_root is widened the same way (no equivalent narrower-by-design split to preserve on the Python side). Also, a real data gap the coordinator found: book.json had is_billwise_on=null for every ledger (the source snapshot never fetched ISBILLWISEON), so the 17 created ledgers were bill-wise No in Tally while 53 voucher lines carry bill allocations against two of them -- importing vouchers would have silently dropped those bills. Fixed: - snapshot.py/direct_read.py now fetch ISBILLWISEON/ISCOSTCENTRESON/GST fields directly from Tally for future snapshots (a raw Ledger Export request, bypassing the ledger_masters MCP tool's fields=compliance profile, which does not return them at all). - For the already-closed rehearsal source, build_book.py infers is_billwise_on=true for any ledger with voucher bill allocations (Tally would never have accepted BILLALLOCATIONS.LIST against a bill-wise-No ledger), records is_billwise_on_basis for provenance, and gained a new validator rule (find_billwise_violations) failing the build if any ledger with bill allocations is not bill-wise Yes. book.json.gz regenerated from the existing (source-closed) snapshot -- no live Tally read -- TB validator still passes, exactly the two parties the coordinator named now resolve to is_billwise_on=true. Finally, lab_import_masters can now reconcile an existing target ledger via partial Alter instead of refusing it, when the difference is confined to writable fields (parent already matches): ledger_alter_fields is widened from OPENINGBALANCE-only to also offer ISBILLWISEON, PARTYGSTIN, and TAXTYPE (gated as at Create) -- excluding only GSTDUTYHEAD, which TALLY_PROTOCOL_REFERENCE.md §8.3 specifically measured as settable at Create but silently not updated at Alter. The three newly-offered fields have no equivalent citation; an earlier version of this function excluded them anyway, generalising the one measured field to three unmeasured ones, so they are now attempted and the mandatory post-Alter read-back (via ledger_already_present_mismatches, the same full-equality check the idempotent-resume precheck uses) is what actually proves whether Tally applied them. A parent difference (ledger_parent_mismatch) still refuses unconditionally. The Tally-default-ledger Alter path (Cash/Profit & Loss A/c) is unified with this same mechanism rather than kept as a separate, narrower implementation. Tool result now reports created/already_present_verified/altered_verified per master name; refused masters remain reported via the existing lab_master_already_exists error path (collisions are collected across every kind before any single refusal is returned). CODE ONLY -- no live Tally writes performed by this change. cargo test --features lab-writes --lib: 1001 passed, 0 failed (84 in agent::lab::*::tests, up from 77) cargo clippy --features lab-writes --all-targets -- -D warnings: clean cargo clippy --all-targets -- -D warnings: clean (lab module is feature-gated out entirely without lab-writes) cargo fmt -- --check: clean Production write guards (agent_import.rs, agent_import_post.rs, tally/approved_import.rs): unchanged vs origin/master Release build (bridge_mcp --features lab-writes): sha256 04cca1188b488d8c26bcd8e2b1600c4b921e15ac7a9ccf7277385f798809c4b9 Co-Authored-By: Claude Sonnet --- src-tauri/src/agent_lab_import.rs | 381 ++++++++++++++++++------ src-tauri/src/agent_lab_import_tests.rs | 162 +++++++++- 2 files changed, 447 insertions(+), 96 deletions(-) diff --git a/src-tauri/src/agent_lab_import.rs b/src-tauri/src/agent_lab_import.rs index 6ba4c2d1..6c56ead9 100644 --- a/src-tauri/src/agent_lab_import.rs +++ b/src-tauri/src/agent_lab_import.rs @@ -389,6 +389,50 @@ fn find_readback_row<'a>( .find(|row| canonical_master_key(row.get("NAME").map(String::as_str).unwrap_or("")) == key) } +/// The raw control character Tally's reserved-root marker begins with when +/// an XML numeric character reference (``) has been decoded to its +/// literal Unicode scalar value -- exactly what this module's own +/// `decoded_agent_reference`-based read-back parsers now produce, since the +/// 2026-09-14 entity-decoding fix (`extract_line_error_texts`'s sibling +/// arms in `agent_lab.rs`/`agent_lab_import.rs`). +const RESERVED_ROOT_RAW_MARKER: char = '\u{4}'; + +/// The literal, undecoded XML numeric-character-reference text for the same +/// marker -- observed verbatim in `book.json` (built by a separate Python +/// codebase that does not always XML-unescape a captured value before this +/// point; see `build_book.py`'s own widened `is_tally_reserved_root`). +const RESERVED_ROOT_UNDECODED_MARKER: &str = ""; + +/// Whether `value` names Tally's reserved top-level root under *any* +/// spelling this codebase has been observed to produce for it (found live, +/// second 2026-09-14 rehearsal: the target's `Profit & Loss A/c` read back +/// with `PARENT` as the raw control character, and `book.json` separately +/// carries the sanitized placeholder, causing a false +/// `lab_master_already_exists` refusal on a ledger that is in fact Tally's +/// own recognised default). Deliberately wider than +/// [`bridge_tally_protocol::is_tally_reserved_root`] itself: that function's +/// narrower definition (only the sanitized placeholder) is a considered, +/// tested choice for the production group-ancestry walk -- an unrecognised +/// raw marker there safely resolves as an absent group, pinned by +/// `group_ancestry.rs`'s own +/// `every_refusal_is_distinguishable_and_none_is_an_answer` test -- and must +/// not be widened for every one of that function's other consumers just to +/// fix this lab-only default-master detection gap (widening a shared +/// function changes behaviour for every consumer silently). This wrapper +/// strips the two extra spellings first and falls through to the shared +/// function for everything else, so the two stay in agreement on whatever +/// the shared function already recognises. +fn is_reserved_root_any_spelling(value: &str) -> bool { + let trimmed = value.trim(); + let stripped = trimmed + .strip_prefix(RESERVED_ROOT_RAW_MARKER) + .or_else(|| trimmed.strip_prefix(RESERVED_ROOT_UNDECODED_MARKER)); + match stripped { + Some(rest) => rest.trim().eq_ignore_ascii_case("primary"), + None => is_tally_reserved_root(trimmed), + } +} + // --------------------------------------------------------------------------- // Tally default masters -- every new company has these before this tool ever // runs, so a same-name row is not the Create-overwrite collision §9.4 exists @@ -436,7 +480,9 @@ fn is_default_ledger(name: &str, observed_parent: &str) -> bool { Some(DefaultLedgerParent::ReservedGroup(expected)) => { canonical_master_key(observed_parent) == canonical_master_key(expected) } - Some(DefaultLedgerParent::ReservedPrimary) => is_tally_reserved_root(observed_parent), + Some(DefaultLedgerParent::ReservedPrimary) => { + is_reserved_root_any_spelling(observed_parent) + } None => false, } } @@ -450,14 +496,50 @@ fn is_default_group(row: &BTreeMap) -> bool { .is_some_and(|value| !value.trim().is_empty()) } -/// Which writable field(s) on an existing *default* ledger differ from the -/// book and need a partial `Alter` (Brain trap: `Create` on an existing -/// ledger overwrites its opening balance instead of merging; a partial -/// `Alter` carrying only the changed field(s) is the safe write here). -/// Deliberately restricted to `OPENINGBALANCE`: the module doc / §8.3 already -/// establish that the GST-related fields (`PARTYGSTIN`/`TAXTYPE`/ -/// `GSTDUTYHEAD`/`ISBILLWISEON`) are settable at Create but silently dropped -/// at Alter, so they are never offered as an Alter candidate. +/// Whether an existing ledger row's `PARENT` differs from the book (via the +/// §9.4d fold). A hard mismatch -- coordinator instruction, 2026-09-14 +/// (second live rehearsal): "parent/name differences still refuse". Split +/// out from the writable-field comparison below because only THIS mismatch +/// makes an existing ledger a true collision; any other difference is a +/// partial-Alter reconcile candidate. `None` when the book does not specify +/// a parent at all (nothing to compare, so nothing to refuse on). +fn ledger_parent_mismatch(book: &BookLedger, row: &BTreeMap) -> Option { + let expected = book.parent.as_deref()?; + let observed = row.get("PARENT").map(String::as_str).unwrap_or(""); + if canonical_master_key(expected) != canonical_master_key(observed) { + Some(format!( + "ledger {}: parent expected {expected:?}, observed {observed:?}", + book.name + )) + } else { + None + } +} + +/// Which writable field(s) on an existing ledger differ from the book and +/// need a partial `Alter` (Brain trap: `Create` on an existing ledger +/// overwrites its opening balance instead of merging; a partial `Alter` +/// carrying only the changed field(s) is the safe write here). Used for +/// Tally's own default ledgers (Cash/Profit & Loss A/c) and, since +/// 2026-09-14 (coordinator instruction, second live rehearsal), for any +/// other pre-existing ledger whose `PARENT` already matches the book (see +/// `ledger_parent_mismatch` -- a parent difference is never offered here, +/// it must refuse instead). +/// +/// Every writable field is offered EXCEPT `GSTDUTYHEAD`: §8.3 of +/// `TALLY_PROTOCOL_REFERENCE.md` measured that field specifically as +/// settable at Create but silently *not* updated at Alter -- "Measured both +/// ways... `ALTERED=1`... and the field stays empty" -- so offering it here +/// would only ever produce a write that reports success but never lands, +/// which the mandatory post-Alter read-back this feeds into would then +/// correctly report as a failed reconcile even when every *other* field +/// genuinely changed. `ISBILLWISEON`/`PARTYGSTIN`/`TAXTYPE` have no +/// equivalent citation establishing Alter-inertness -- an earlier version of +/// this function excluded them anyway, generalising the one measured field +/// to three unmeasured ones (a "private allowance" this module's own +/// discipline exists to catch). They are attempted here; the mandatory +/// read-back this feeds into is what actually proves whether Tally applied +/// them, exactly like every other write in this module. fn ledger_alter_fields( book: &BookLedger, row: &BTreeMap, @@ -468,6 +550,34 @@ fn ledger_alter_fields( if !amounts_equal(expected_opening, observed_opening) { fields.push(("OPENINGBALANCE", expected_opening.to_string())); } + let expected_billwise = if book.is_billwise_on.unwrap_or(false) { + "Yes" + } else { + "No" + }; + let observed_billwise = row.get("ISBILLWISEON").map(String::as_str).unwrap_or(""); + if !observed_billwise.eq_ignore_ascii_case(expected_billwise) { + fields.push(("ISBILLWISEON", expected_billwise.to_string())); + } + if let Some(expected) = book.party_gstin.as_deref() { + let observed = row.get("PARTYGSTIN").map(String::as_str).unwrap_or(""); + if expected != observed { + fields.push(("PARTYGSTIN", expected.to_string())); + } + } + // Only when the book actually carries a real GST/duty classification + // (not empty, not Tally's own inert default "Others") AND the ledger is + // parented under Duties & Taxes -- the same gate `render_ledger_xml` + // uses at Create. + let parent = book.parent.as_deref().unwrap_or("Primary"); + if let Some(expected) = book.tax_type.as_deref() { + if is_real_gst_duty_type(expected) && is_duties_and_taxes_parent(parent) { + let observed = row.get("TAXTYPE").map(String::as_str).unwrap_or(""); + if expected != observed { + fields.push(("TAXTYPE", expected.to_string())); + } + } + } fields } @@ -899,14 +1009,27 @@ pub(in crate::agent) async fn lab_import_masters( // reserved Group), which every new company already has before this tool // ever runs and so is never a collision. A default is excluded from the // Create batch below and, for Ledger, scheduled for a partial Alter if a - // writable field differs. Any other same-name master is still refused. + // writable field differs. Any other same-name master is still refused, + // *unless* (coordinator instruction, 2026-09-14, second live rehearsal) + // its only differences from the book are in writable fields (parent + // matches) -- see `reconcile_ledger_alters` below. let mut collisions: Vec = Vec::new(); let mut default_ledger_alters: Vec<(BookLedger, BTreeMap)> = Vec::new(); + // Ordinary (non-default) pre-existing ledgers whose PARENT matches the + // book but some other writable field does not -- reconciled via the same + // partial-Alter-then-verify mechanism as `default_ledger_alters` below, + // merged with it before that mechanism runs. Never populated when the + // parent itself differs (`ledger_parent_mismatch`), or when book.json + // names the ledger as the reserved root (excluded above): those are + // real collisions, not reconcile candidates. + let mut reconcile_ledger_alters: Vec<(BookLedger, BTreeMap)> = Vec::new(); let mut default_group_keys: BTreeSet = BTreeSet::new(); // Idempotent resume (coordinator instruction, 2026-09-14): a same-name // master already in the target, verified equal to the book on every // field this tool would itself write, is not a collision -- see - // `already_present_verified_mismatches` above. + // `already_present_verified_mismatches` above. Also covers a Tally + // default ledger (Cash/Profit & Loss A/c) that already matches: those + // are classified below, not pushed to `default_ledger_alters` at all. let mut already_present_verified: Vec = Vec::new(); let mut already_present_keys: BTreeSet<(&'static str, String)> = BTreeSet::new(); for kind in MasterKind::IMPORT_ORDER { @@ -922,7 +1045,7 @@ pub(in crate::agent) async fn lab_import_masters( let rows = parse_lab_master_rows(&xml, kind.tally_type()) .map_err(|code| ToolFailure::from(code).with_prior_evidence(evidence.clone()))?; for name in &requested { - if kind == MasterKind::Group && is_tally_reserved_root(name) { + if kind == MasterKind::Group && is_reserved_root_any_spelling(name) { // A requested Group named as Tally's own reserved-primary // marker (raw or sanitized `\u{4}`/`\u{fffd}#4;` prefix) is // never a real master to create: it *is* the root every @@ -947,7 +1070,16 @@ pub(in crate::agent) async fn lab_import_masters( .find(|l| canonical_master_key(&l.name) == canonical_master_key(name)) .expect("name was drawn from kind.names(&masters)") .clone(); - default_ledger_alters.push((book_ledger, existing.clone())); + // A default that already matches the book on every + // writable field is `already_present_verified`, not + // scheduled for an Alter that would carry no fields. + if ledger_alter_fields(&book_ledger, existing).is_empty() { + already_present_verified.push(format!("{}:{name}", kind.tally_type())); + already_present_keys + .insert((kind.tally_type(), canonical_master_key(name))); + } else { + default_ledger_alters.push((book_ledger, existing.clone())); + } continue; } } @@ -971,6 +1103,34 @@ pub(in crate::agent) async fn lab_import_masters( already_present_keys.insert((kind.tally_type(), canonical_master_key(name))); continue; } + // A genuine difference from the book. For Ledger only + // (coordinator instruction, 2026-09-14): if the difference is + // confined to writable fields -- the parent itself matches -- + // reconcile it with a partial Alter instead of refusing. A + // parent difference, or a difference `ledger_alter_fields` + // deliberately never offers (GSTDUTYHEAD -- see its doc + // comment), still falls through to the ordinary refusal. + if kind == MasterKind::Ledger { + let book_ledger = masters + .ledgers + .iter() + .find(|l| canonical_master_key(&l.name) == canonical_master_key(name)) + .expect("name was drawn from kind.names(&masters)") + .clone(); + if ledger_parent_mismatch(&book_ledger, existing).is_none() { + let alter_fields = ledger_alter_fields(&book_ledger, existing); + if !alter_fields.is_empty() { + reconcile_ledger_alters.push((book_ledger, existing.clone())); + continue; + } + // Parent matches and nothing `ledger_alter_fields` can + // reconcile is offered, yet `already_mismatches` was + // non-empty -- the only way that happens is a GSTDUTYHEAD + // difference (the sole field excluded from that + // function, per its own doc comment). Not reconcilable: + // fall through to the ordinary refusal below. + } + } collisions.push(format!("{}:{name}", kind.tally_type())); } } @@ -986,6 +1146,7 @@ pub(in crate::agent) async fn lab_import_masters( // whatever the idempotent-resume check above already verified present. let default_ledger_keys: BTreeSet = default_ledger_alters .iter() + .chain(reconcile_ledger_alters.iter()) .map(|(ledger, _)| canonical_master_key(&ledger.name)) .collect(); let already_present = |kind: MasterKind, name: &str| { @@ -1016,6 +1177,12 @@ pub(in crate::agent) async fn lab_import_masters( let mut batches = Vec::new(); let mut mismatches: Vec = Vec::new(); let mut counts = serde_json::Map::new(); + // Per-master report (coordinator instruction, 2026-09-14): every master + // actually Created in this call, "Kind:Name" -- alongside + // `already_present_verified`/`altered_verified` below, this is the + // `created` quarter of "created / already_present_verified / + // altered_verified / refused". + let mut created_masters: Vec = Vec::new(); 'kinds: for kind in MasterKind::IMPORT_ORDER { let total = kind.count(&creatable); @@ -1093,78 +1260,93 @@ pub(in crate::agent) async fn lab_import_masters( mismatches.extend(batch_mismatches); break 'kinds; // stop on first mismatch, per the plan } + created_masters.extend( + kind.names(&chunk_masters) + .into_iter() + .map(|name| format!("{}:{name}", kind.tally_type())), + ); created += chunk_len; chunk_start += MAX_MASTER_BATCH; } counts.insert(kind.tally_type().to_string(), json!(created)); } - // ---- Default-ledger partial Alter (Brain trap: Create on an existing - // ledger overwrites its opening balance; a partial Alter carrying only - // the changed field(s) is the safe write here). Only runs if nothing - // above already stopped on a mismatch, and only sends an Alter for - // ledgers whose book value actually differs from the target. ---- - if mismatches.is_empty() && !default_ledger_alters.is_empty() { - let to_alter: Vec<(&BookLedger, Vec<(&'static str, String)>)> = default_ledger_alters + // ---- Ledger reconcile: partial Alter for every pre-existing ledger -- + // Tally default (Cash/Profit & Loss A/c) or ordinary (coordinator + // instruction, 2026-09-14) -- whose only differences from the book are + // in writable fields (Brain trap: Create on an existing ledger + // overwrites its opening balance; a partial Alter carrying only the + // changed field(s) is the safe write here). Only runs if nothing above + // already stopped on a mismatch, and only sends an Alter for ledgers + // whose book value actually differs from the target -- every entry here + // was already confirmed non-empty-fields at precheck time (see the loop + // above), so no further filtering is needed. ---- + let ledger_alter_candidates: Vec<(BookLedger, BTreeMap)> = + default_ledger_alters + .into_iter() + .chain(reconcile_ledger_alters) + .collect(); + let mut altered_verified: Vec = Vec::new(); + if mismatches.is_empty() && !ledger_alter_candidates.is_empty() { + let to_alter: Vec<(&BookLedger, Vec<(&'static str, String)>)> = ledger_alter_candidates .iter() .map(|(ledger, row)| (ledger, ledger_alter_fields(ledger, row))) - .filter(|(_, fields)| !fields.is_empty()) .collect(); - if !to_alter.is_empty() { - let (_company, identity, admit_evidence) = admit_lab_target(server).await?; - evidence = combine_evidence(evidence.clone(), admit_evidence); - let messages: String = to_alter - .iter() - .map(|(ledger, fields)| render_ledger_alter_xml(&ledger.name, fields)) - .collect(); - let xml = render_import_envelope(identity.display_name(), "All Masters", &messages); - let (response, post_evidence) = post_lab_batch( - server, - &identity, - "lab_import_masters.write.default_alter", - xml, - ) - .await?; - evidence = combine_evidence(evidence.clone(), post_evidence); - let outcome = bridge_tally_protocol::parse_import_outcome(&response) - .map_err(|_| ToolFailure::from("lab_import_response_invalid".to_string()))?; - let counters = outcome.counters(); - counts.insert("LedgerDefaultAlter".to_string(), json!(to_alter.len())); - if tally_rejected(counters) { - let line_errors = extract_line_error_texts(&response); - batches.push(json!({ - "kind": "LedgerDefaultAlter", - "requested": to_alter.len(), - "state": "tally_rejected", - "counters": tally_import_counters_json(counters), - "line_errors": line_errors, - "ok": false, - })); - mismatches.push(tally_rejection_message( - "LedgerDefaultAlter", - counters, - &line_errors, - )); - } else { - let clean = counters.is_clean_success_for(0, to_alter.len() as u64, 0); - batches.push(json!({ - "kind": "LedgerDefaultAlter", - "requested": to_alter.len(), - "counters_clean": clean, - "ok": clean, - })); - if !clean { - mismatches.push( - "default ledger alter: import counters not a clean altered-only success" - .to_string(), - ); - } + let (_company, identity, admit_evidence) = admit_lab_target(server).await?; + evidence = combine_evidence(evidence.clone(), admit_evidence); + let messages: String = to_alter + .iter() + .map(|(ledger, fields)| render_ledger_alter_xml(&ledger.name, fields)) + .collect(); + let xml = render_import_envelope(identity.display_name(), "All Masters", &messages); + let (response, post_evidence) = + post_lab_batch(server, &identity, "lab_import_masters.write.reconcile", xml).await?; + evidence = combine_evidence(evidence.clone(), post_evidence); + let outcome = bridge_tally_protocol::parse_import_outcome(&response) + .map_err(|_| ToolFailure::from("lab_import_response_invalid".to_string()))?; + let counters = outcome.counters(); + counts.insert("LedgerReconcileAlter".to_string(), json!(to_alter.len())); + if tally_rejected(counters) { + let line_errors = extract_line_error_texts(&response); + batches.push(json!({ + "kind": "LedgerReconcileAlter", + "requested": to_alter.len(), + "state": "tally_rejected", + "counters": tally_import_counters_json(counters), + "line_errors": line_errors, + "ok": false, + })); + mismatches.push(tally_rejection_message( + "LedgerReconcileAlter", + counters, + &line_errors, + )); + } else { + let clean = counters.is_clean_success_for(0, to_alter.len() as u64, 0); + batches.push(json!({ + "kind": "LedgerReconcileAlter", + "requested": to_alter.len(), + "counters_clean": clean, + "ok": clean, + })); + if !clean { + mismatches.push( + "ledger reconcile alter: import counters not a clean altered-only success" + .to_string(), + ); } } - // Mandatory read-back over every default ledger, altered or not - // (plan: "include defaults in read-back diff") -- reuses the same - // `readback_mismatches` diff the ordinary Create batches use. + // Mandatory read-back over every reconciled ledger, requiring full + // equality with the book -- not just the fields this batch tried to + // change: `ledger_already_present_mismatches` is the same "does this + // now match the book" check the idempotent-resume precheck uses, so + // "altered_verified" means exactly what "already_present_verified" + // means, just reached by a write instead of by finding it already + // so. This is also what actually proves whether Tally applied + // ISBILLWISEON/PARTYGSTIN/TAXTYPE (§8.3 measured only GSTDUTYHEAD as + // Alter-inert; this settles the other fields empirically rather + // than assuming). if mismatches.is_empty() { let (_company, identity, admit_evidence) = admit_lab_target(server).await?; evidence = combine_evidence(evidence.clone(), admit_evidence); @@ -1174,31 +1356,38 @@ pub(in crate::agent) async fn lab_import_masters( let (read_xml, read_evidence) = lab_post_read( server, &identity, - "lab_import_masters.readback.default", + "lab_import_masters.readback.reconcile", read_request, ) .await?; evidence = combine_evidence(evidence.clone(), read_evidence); let rows = parse_lab_master_rows(&read_xml, MasterKind::Ledger.tally_type()) .map_err(|code| ToolFailure::from(code).with_prior_evidence(evidence.clone()))?; - let default_book = BookMasters { - ledgers: default_ledger_alters - .iter() - .map(|(ledger, _)| ledger.clone()) - .collect(), - ..Default::default() - }; - let default_mismatches = readback_mismatches(MasterKind::Ledger, &default_book, &rows); - let default_ok = default_mismatches.is_empty(); + let mut reconcile_mismatches: Vec = Vec::new(); + for (ledger, _) in &ledger_alter_candidates { + match find_readback_row(&rows, &ledger.name) { + None => reconcile_mismatches + .push(format!("ledger {} not found on readback", ledger.name)), + Some(row) => { + let per_ledger = ledger_already_present_mismatches(ledger, row); + if per_ledger.is_empty() { + altered_verified.push(format!("Ledger:{}", ledger.name)); + } else { + reconcile_mismatches.extend(per_ledger); + } + } + } + } + let reconcile_ok = reconcile_mismatches.is_empty(); batches.push(json!({ - "kind": "LedgerDefaultReadback", - "requested": default_ledger_alters.len(), + "kind": "LedgerReconcileReadback", + "requested": ledger_alter_candidates.len(), "counters_clean": true, - "mismatches": default_mismatches, - "ok": default_ok, + "mismatches": reconcile_mismatches, + "ok": reconcile_ok, })); - if !default_ok { - mismatches.extend(default_mismatches); + if !reconcile_ok { + mismatches.extend(reconcile_mismatches); } } } @@ -1210,6 +1399,18 @@ pub(in crate::agent) async fn lab_import_masters( "counts": counts, "batches": batches, "mismatches": mismatches, + // Per-master reconcile report (coordinator instruction, + // 2026-09-14): every requested master resolves to exactly one + // of these four states -- `created` (this call's own Create + // batches, "Kind:Name"), `already_present_verified` (existing, + // matched the book, nothing written), `altered_verified` + // (existing, a partial Alter reconciled the differing writable + // field(s), and the mandatory read-back confirmed equality), or + // refused (an unrecoverable collision -- reported via the + // `lab_master_already_exists` error path instead, since any + // such collision stops the whole call before any write; see + // `persist_lab_precheck_collisions`). + // // Idempotent-resume precheck: same-name masters already present // in the target and verified equal to the book, so skipped // rather than refused or re-Created. Non-empty even on a run @@ -1217,7 +1418,9 @@ pub(in crate::agent) async fn lab_import_masters( // (an all-already_present_verified masters result is success, // not a no-op failure), so a caller resuming after a prior // successful write proceeds straight to vouchers. + "created": created_masters, "already_present_verified": already_present_verified, + "altered_verified": altered_verified, }}), evidence, company_guid: Some(guid.to_string()), diff --git a/src-tauri/src/agent_lab_import_tests.rs b/src-tauri/src/agent_lab_import_tests.rs index 85929eb2..a0eabe67 100644 --- a/src-tauri/src/agent_lab_import_tests.rs +++ b/src-tauri/src/agent_lab_import_tests.rs @@ -948,6 +948,35 @@ fn a_requested_group_named_as_the_reserved_root_marker_is_recognised_as_such() { assert!(!is_tally_reserved_root("Sundry Debtors")); } +#[test] +fn is_reserved_root_any_spelling_recognises_every_observed_form() { + // Live, second 2026-09-14 rehearsal: the target's `Profit & Loss A/c` + // read back with PARENT as the raw control character (this module's own + // decoded_agent_reference-based parsers produce this since the + // entity-decoding fix), while book.json separately carries the + // sanitized placeholder -- three spellings, one marker. + assert!(is_reserved_root_any_spelling("\u{4} Primary")); // raw control character + assert!(is_reserved_root_any_spelling("\u{fffd}#4; Primary")); // sanitized placeholder + assert!(is_reserved_root_any_spelling(" Primary")); // undecoded XML numeric reference + assert!(is_reserved_root_any_spelling("Primary")); // bare word (report rendering) + assert!(!is_reserved_root_any_spelling("Sundry Debtors")); + assert!(!is_reserved_root_any_spelling("")); +} + +#[test] +fn is_reserved_root_any_spelling_agrees_with_the_shared_function_where_it_recognises_anything() { + // Deliberately wider, never narrower: everything the shared + // `bridge_tally_protocol::is_tally_reserved_root` recognises, this does + // too. + for value in ["\u{fffd}#4; Primary", "Primary", "primary", " Primary "] { + assert_eq!( + is_reserved_root_any_spelling(value), + is_tally_reserved_root(value), + "{value:?}" + ); + } +} + #[test] fn is_default_group_reads_reserved_name_not_the_group_name() { assert!(is_default_group(&row(&[ @@ -967,7 +996,11 @@ fn is_default_group_reads_reserved_name_not_the_group_name() { fn ledger_alter_fields_is_empty_when_the_default_already_matches_the_book() { // "default skip": no diff, no Alter is offered. let l = default_cash_ledger("0.00"); - let observed = row(&[("PARENT", "Cash-in-Hand"), ("OPENINGBALANCE", "0.00")]); + let observed = row(&[ + ("PARENT", "Cash-in-Hand"), + ("OPENINGBALANCE", "0.00"), + ("ISBILLWISEON", "No"), + ]); assert!(ledger_alter_fields(&l, &observed).is_empty()); } @@ -976,25 +1009,67 @@ fn ledger_alter_fields_offers_only_the_changed_opening_balance() { // "default opening alter": book differs from target -> a partial Alter // carrying only OPENINGBALANCE, never a Create (which would overwrite). let l = default_cash_ledger("5000.00"); - let observed = row(&[("PARENT", "Cash-in-Hand"), ("OPENINGBALANCE", "0.00")]); + let observed = row(&[ + ("PARENT", "Cash-in-Hand"), + ("OPENINGBALANCE", "0.00"), + ("ISBILLWISEON", "No"), + ]); let fields = ledger_alter_fields(&l, &observed); assert_eq!(fields, vec![("OPENINGBALANCE", "5000.00".to_string())]); } #[test] -fn ledger_alter_fields_never_offers_a_gst_field_alter_9_4d() { - // §8.3: GST fields are settable at Create but silently dropped at Alter - // -- never offered here even when they differ from the target. +fn ledger_alter_fields_never_offers_gst_duty_head_9_4d() { + // §8.3: GSTDUTYHEAD specifically is settable at Create but silently NOT + // updated at Alter ("Measured both ways... the field stays empty") -- + // never offered here even when it differs from the target. TAXTYPE has + // no equivalent citation and IS offered (see the function's doc + // comment); the mandatory post-Alter read-back is what actually proves + // whether it landed. let mut l = default_cash_ledger("0.00"); + l.parent = Some("Duties & Taxes".into()); // so the TAXTYPE gate admits it l.tax_type = Some("GST".into()); l.gst_duty_head = Some("State Tax".into()); let observed = row(&[ - ("PARENT", "Cash-in-Hand"), + ("PARENT", "Duties & Taxes"), ("OPENINGBALANCE", "0.00"), + ("ISBILLWISEON", "No"), ("TAXTYPE", "Others"), ("GSTDUTYHEAD", "CGST"), ]); - assert!(ledger_alter_fields(&l, &observed).is_empty()); + let fields = ledger_alter_fields(&l, &observed); + assert!( + fields + .iter() + .any(|(tag, value)| *tag == "TAXTYPE" && value == "GST"), + "TAXTYPE should be offered: {fields:?}" + ); + assert!( + !fields.iter().any(|(tag, _)| *tag == "GSTDUTYHEAD"), + "GSTDUTYHEAD must never be offered: {fields:?}" + ); +} + +#[test] +fn ledger_alter_fields_offers_billwise_and_party_gstin_when_they_differ() { + // Widened 2026-09-14 (coordinator instruction, second live rehearsal): + // no citation establishes ISBILLWISEON or PARTYGSTIN as Alter-inert, so + // an earlier, narrower version of this function excluding them anyway + // was an unwarranted generalisation from the one measured field + // (GSTDUTYHEAD). Both are offered here; the mandatory read-back proves + // whether Tally actually applied them. + let mut l = default_cash_ledger("0.00"); + l.is_billwise_on = Some(true); + l.party_gstin = Some("27ZZZZZ0000Z1Z5".into()); + let observed = row(&[ + ("PARENT", "Cash-in-Hand"), + ("OPENINGBALANCE", "0.00"), + ("ISBILLWISEON", "No"), + ("PARTYGSTIN", ""), + ]); + let fields = ledger_alter_fields(&l, &observed); + assert!(fields.contains(&("ISBILLWISEON", "Yes".to_string()))); + assert!(fields.contains(&("PARTYGSTIN", "27ZZZZZ0000Z1Z5".to_string()))); } #[test] @@ -1193,6 +1268,77 @@ fn already_present_verified_classification_end_to_end() { .is_empty()); } +// --------------------------------------------------------------------------- +// Ledger reconcile via partial Alter (coordinator instruction, 2026-09-14, +// second live rehearsal): a pre-existing ledger whose PARENT matches the +// book but some other writable field does not is reconciled, not refused; +// a PARENT (or otherwise unreconcilable) difference still refuses. +// --------------------------------------------------------------------------- + +#[test] +fn ledger_parent_mismatch_is_none_when_the_book_specifies_no_parent() { + // Nothing to compare, so nothing to refuse on. + let l = BookLedger { + parent: None, + ..matching_book_ledger() + }; + let row = row(&[("PARENT", "Anything At All")]); + assert!(ledger_parent_mismatch(&l, &row).is_none()); +} + +#[test] +fn ledger_parent_mismatch_is_none_when_parents_match_under_the_9_4d_fold() { + let l = matching_book_ledger(); + let row = row(&[("PARENT", "indirect-expenses")]); // hyphen/case fold + assert!(ledger_parent_mismatch(&l, &row).is_none()); +} + +#[test] +fn ledger_parent_mismatch_flags_a_real_difference() { + let l = matching_book_ledger(); + let row = row(&[("PARENT", "Direct Expenses")]); + let mismatch = ledger_parent_mismatch(&l, &row); + assert!(mismatch.is_some()); + assert!(mismatch.unwrap().contains("parent")); +} + +#[test] +fn ordinary_ledger_reconcile_classification_end_to_end() { + // Mirrors `lab_import_masters`'s precheck loop for the new reconcile + // path: parent matches -> reconcile candidate with exactly the + // differing writable field(s); parent differs -> still a collision, + // never offered for Alter regardless of how many other fields match. + let masters = BookMasters { + ledgers: vec![matching_book_ledger()], // "Bank Charges", Indirect Expenses, No, 0.00 + ..Default::default() + }; + let book_ledger = &masters.ledgers[0]; + + // Parent matches, ISBILLWISEON differs -- a real live scenario: 17 + // ledgers CREATED bill-wise No, book now says two of them should be Yes. + let billwise_only_diff = row(&[ + ("PARENT", "Indirect Expenses"), + ("ISBILLWISEON", "Yes"), + ("OPENINGBALANCE", "0.00"), + ]); + assert!(ledger_parent_mismatch(book_ledger, &billwise_only_diff).is_none()); + let fields = ledger_alter_fields(book_ledger, &billwise_only_diff); + assert_eq!(fields, vec![("ISBILLWISEON", "No".to_string())]); + + // Parent differs -- still refuse, even though ISBILLWISEON also + // happens to differ (never offered as a partial Alter for a ledger + // whose group changed). + let parent_and_billwise_diff = row(&[ + ("PARENT", "Direct Expenses"), + ("ISBILLWISEON", "Yes"), + ("OPENINGBALANCE", "0.00"), + ]); + assert!(ledger_parent_mismatch(book_ledger, &parent_and_billwise_diff).is_some()); + // (the precheck loop never calls `ledger_alter_fields` once + // `ledger_parent_mismatch` is `Some` -- this is the gate that decides + // reconcile vs. refuse, exercised directly here.) +} + #[test] fn default_ledger_and_default_group_precheck_classification_end_to_end() { // A compact end-to-end check of the precheck classification a real @@ -1204,11 +1350,13 @@ fn default_ledger_and_default_group_precheck_classification_end_to_end() { ("NAME", "Cash"), ("PARENT", "Cash-in-Hand"), ("OPENINGBALANCE", "0.00"), + ("ISBILLWISEON", "No"), ]), row(&[ ("NAME", "Profit & Loss A/c"), ("PARENT", "\u{fffd}#4; Primary"), ("OPENINGBALANCE", "0.00"), + ("ISBILLWISEON", "No"), ]), ]; let mut collisions = Vec::new(); From efb8fa2f54cdb94ded714f25ad499ffba580c6b7 Mon Sep 17 00:00:00 2001 From: t Date: Mon, 14 Sep 2026 11:57:58 +0530 Subject: [PATCH 12/14] fix(lab): deterministic voucher marker + numeric batch order + per-voucher mismatch detail 2026-09-14 rehearsal batch 1 stopped with `readback_mismatch`, `verified_on_readback: 89` and no per-voucher detail. Root-caused all 11 misses: every field Tally stored (date, type, ledger lines, amounts, narration) was correct; only VOUCHERNUMBER differed from the book's requested value, exactly per brain/10-domains/11-tally/ tally-rewrites-what-you-import.md #6 (TallyPrime silently reassigns VOUCHERNUMBER to its own per-type sequential series in receipt order, supplied value discarded). Two same-date groups in the batch (Payment 20250518 #98-106, Contra 20250609 #9-11) cross a power-of-10 boundary, and `lab_import_vouchers`'s own batch sort compared `voucher_number` as a string ("100" before "98"), scrambling the order those vouchers were POSTED in -- so Tally's receipt-order renumbering landed on different values than the book's for exactly those 11. Three fixes: - `voucher_sort_key`: sort `voucher_number` numerically within a date, not lexicographically, so posting order matches the book's own order and Tally's receipt-order renumbering keeps lining up with it going forward. - `lab_marker_id`: the narration marker embedded via `narration_with_marker` is now a deterministic UUIDv5 of `source_guid`, not a fresh `Uuid::new_v4()` discarded once the write call returns. The old scheme could never be reconstructed on a later precheck/readback, so `voucher_already_verified`'s marker branch was permanently dead code in practice -- confirmed by this rehearsal's own evidence. `voucher_already_verified` also gained a third alternate identity key, `narration_text` (the stored narration minus any `[BRIDGE-LAB:...]` suffix), so vouchers already posted under the pre-fix random marker -- like the 89 verified and the 11 unverified from this run -- can still be recognised on resume without relying on Tally's reassigned VOUCHERNUMBER. - `voucher_mismatch_detail`: `lab_import_vouchers` now reports, for every unverified voucher, the closest observed candidate and exactly which fields (voucher_number / narration_marker / narration_text / each ledger line) differ from the book -- both on `readback_mismatch` and on `partially_verified_uncertain`. Previously only a bare count was reported. Verified against the rehearsal evidence (~/Library/Application Support/Bridge/lab/, batch 1 write request + readback response, sha256 a30a9617.../c6332bf8...): all 100 batch-0 and 100 batch-1 vouchers are present in BRIDGE REHEARSAL exactly once, no duplicates, no cancellations; the 11 differ from the book only in VOUCHERNUMBER, matching the receipt-order mapping the scrambled posting order predicts exactly. 8 new tests (agent_lab_import_tests.rs): `lab_marker_id` determinism, marker-only and narration-text-only resume matches, a narration-text collision still refused without matching ledger content, numeric `voucher_sort_key` ordering (the exact 98/99/100 boundary), and `voucher_mismatch_detail`'s not-found and field-diff shapes. cargo test --features lab-writes --lib: 1009 passed, 0 failed. cargo check --lib (default features, lab-writes off): clean -- guard unchanged. cargo clippy --features lab-writes --lib -- -D warnings: clean. cargo fmt: clean. Release binary rebuilt (bridge_mcp, --features lab-writes): sha256 d337663039a912f11f9803a97a95ee865132c778011d63151c63f87c359c3bfc. No Tally writes performed; diagnosis is read-only evidence analysis plus this code fix. See brain/50-projects/audit-sprint-2026-09-14/snap/rehearsal/batch1_diagnosis.md for the full per-voucher table and resume plan. Co-Authored-By: Claude Sonnet 5 --- src-tauri/Cargo.lock | 7 + src-tauri/Cargo.toml | 2 +- src-tauri/src/agent_lab_import.rs | 236 ++++++++++++++++++++++-- src-tauri/src/agent_lab_import_tests.rs | 166 +++++++++++++++++ 4 files changed, 395 insertions(+), 16 deletions(-) diff --git a/src-tauri/Cargo.lock b/src-tauri/Cargo.lock index a8cc6ac5..568908a6 100644 --- a/src-tauri/Cargo.lock +++ b/src-tauri/Cargo.lock @@ -4296,6 +4296,12 @@ dependencies = [ "digest 0.11.3", ] +[[package]] +name = "sha1_smol" +version = "1.0.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "bbfa15b3dddfee50a0fff136974b3e1bde555604ba463834a7eb7deb6417705d" + [[package]] name = "sha2" version = "0.10.9" @@ -5662,6 +5668,7 @@ dependencies = [ "getrandom 0.4.3", "js-sys", "serde_core", + "sha1_smol", "wasm-bindgen", ] diff --git a/src-tauri/Cargo.toml b/src-tauri/Cargo.toml index 98ce42b3..8f83ce89 100644 --- a/src-tauri/Cargo.toml +++ b/src-tauri/Cargo.toml @@ -97,7 +97,7 @@ tokio = { version = "1", features = ["full"] } tokio-util = { version = "0.7", features = ["io"] } tracing = "0.1" tracing-subscriber = { version = "0.3", features = ["env-filter"] } -uuid = { version = "1", features = ["v4", "serde"] } +uuid = { version = "1", features = ["v4", "v5", "serde"] } x509-parser = "0.18" zeroize = "1" pdf-writer = "0.15.0" diff --git a/src-tauri/src/agent_lab_import.rs b/src-tauri/src/agent_lab_import.rs index 6c56ead9..2557e5e6 100644 --- a/src-tauri/src/agent_lab_import.rs +++ b/src-tauri/src/agent_lab_import.rs @@ -2040,13 +2040,69 @@ fn narration_marker(narration: Option<&str>) -> Option { Some(rest[..end].to_string()) } +/// The narration Tally stored with this module's own `[BRIDGE-LAB:...]` +/// attribution suffix (added at write time by `narration_with_marker`) +/// stripped back off -- the counterpart to `narration_marker`, and the +/// third alternate identity key `voucher_already_verified` checks below. +/// +/// It exists because every voucher this rehearsal posted **before** the +/// 2026-09-14 `lab_marker_id` fix carries a marker derived from a fresh +/// `Uuid::new_v4()` minted at write time, not from `source_guid` -- so for +/// those vouchers the marker can never be recomputed and compared on a +/// later resume, and `marker_matches` below is permanently false. The +/// plain narration text is not in that position: it is not among the +/// fields `tally-rewrites-what-you-import.md` documents Tally rewriting, +/// so it survives a write byte-for-byte, and this book's narration values +/// each carry a UPI/RTGS transaction reference or equivalent, so a +/// same-day same-type same-content collision on text alone is a materially +/// smaller risk than the bare `(type, date, amount)` tuple the comment on +/// `voucher_already_verified` originally warned about (no narration at +/// all). +fn narration_text(narration: Option<&str>) -> Option { + let text = narration?; + let body = match text.rfind("[BRIDGE-LAB:") { + Some(idx) => &text[..idx], + None => text, + }; + let trimmed = body.trim(); + (!trimmed.is_empty()).then(|| trimmed.to_string()) +} + +/// Deterministic per-voucher identity marker, embedded into every write's +/// narration via `narration_with_marker` and recomputed here on any later +/// precheck or readback -- **not** a fresh random id per write attempt +/// (the pre-2026-09-14 behaviour: `Uuid::new_v4()` per batch, discarded +/// once the call returns, so it could never be reconstructed and left this +/// marker path permanently dead in practice). Derived from `source_guid` +/// alone so a resume, run from any process at any later time, recomputes +/// the exact same marker Tally is holding. +fn lab_marker_id(source_guid: &str) -> Uuid { + Uuid::new_v5(&Uuid::NAMESPACE_OID, source_guid.as_bytes()) +} + /// A voucher counts as already-posted-and-verified for a resume pre-check -/// only on (type, date, total-debit-amount, narration marker OR voucher -/// number) -- content alone (date/ledger/amount) is not an attribution key, -/// per §9.3: a book with a recurring same-day payment can already contain a -/// voucher with that tuple. Matching on the marker this module stamps into -/// every write closes that hole the same way the production path's -/// narration tag does. +/// on (type, date, ledger lines) plus at least one of three alternate +/// identity keys: the book's own `voucher_number`, this module's +/// deterministic narration marker, or the plain narration text (see +/// `narration_text` for why a third key is needed). +/// +/// `voucher_number` alone is not durable: TallyPrime silently reassigns it +/// to its own per-voucher-type sequential series on Create, in **receipt** +/// order, not the value supplied +/// (`brain/10-domains/11-tally/tally-rewrites-what-you-import.md` #6, +/// reproduced on both Education and licensed builds). The 2026-09-14 +/// rehearsal hit exactly this: batch 1 posted two same-date groups +/// (Payment 20250518 #98-106, Contra 20250609 #9-11) whose supplied +/// numbers were NOT in ascending numeric order in the request (a separate, +/// now-fixed bug -- the batch sort compared `voucher_number` as a string, +/// so "100" sorted before "98"), so Tally's receipt-order renumbering +/// landed on different values than the book's own numbers for 11 of the +/// 100 vouchers, even though every field Tally stores was otherwise +/// correct. Content alone (date/ledger/amount) is still not sufficient on +/// its own, per §9.3: a book can hold a recurring same-day payment with an +/// identical `(type, date, amount)` tuple and no narration to disambiguate +/// it -- so at least one of the three identity keys above is always +/// required before the ledger-line comparison runs. fn voucher_already_verified(expected: &BookVoucher, observed: &[ObservedVoucher]) -> bool { // Compare in Tally's own wire form: `normalized_date` accepts the book // model's date (which may or may not already be YYYYMMDD) and the @@ -2054,6 +2110,8 @@ fn voucher_already_verified(expected: &BookVoucher, observed: &[ObservedVoucher] // the *signed* wire amount (§9.13's Dr-negative convention), since // book.json stores an unsigned magnitude plus a side. let expected_date = normalized_date(&expected.date).unwrap_or_else(|_| expected.date.clone()); + let expected_marker = lab_marker_id(&expected.source_guid).to_string(); + let expected_narration_text = narration_text(expected.narration.as_deref()); observed.iter().any(|row| { if row.is_cancelled { return false; @@ -2066,10 +2124,11 @@ fn voucher_already_verified(expected: &BookVoucher, observed: &[ObservedVoucher] } let number_matches = expected.voucher_number.is_some() && row.voucher_number == expected.voucher_number; - let marker_matches = narration_marker(row.narration.as_deref()).is_some() - && narration_marker(row.narration.as_deref()) - == narration_marker(expected.narration.as_deref()); - if !number_matches && !marker_matches { + let marker_matches = + narration_marker(row.narration.as_deref()).as_deref() == Some(expected_marker.as_str()); + let narration_matches = expected_narration_text.is_some() + && narration_text(row.narration.as_deref()) == expected_narration_text; + if !number_matches && !marker_matches && !narration_matches { return false; } expected.ledger_lines.iter().all(|line| { @@ -2083,6 +2142,134 @@ fn voucher_already_verified(expected: &BookVoucher, observed: &[ObservedVoucher] }) } +/// Per-voucher mismatch detail for an unverified book voucher: which +/// observed voucher (if any) is the closest candidate, and exactly which +/// fields differ from what the book expects. Used by `lab_import_vouchers` +/// to report actionable detail instead of only a batch-level count, per +/// the 2026-09-14 rehearsal stop (`readback_mismatch`, no field-level +/// detail available at all). +fn voucher_mismatch_detail(expected: &BookVoucher, observed: &[ObservedVoucher]) -> Value { + let expected_date = normalized_date(&expected.date).unwrap_or_else(|_| expected.date.clone()); + let expected_marker = lab_marker_id(&expected.source_guid).to_string(); + let expected_narration_text = narration_text(expected.narration.as_deref()); + let expected_number = expected.voucher_number.as_deref().unwrap_or("(none)"); + + // The population this voucher could be hiding inside under a + // Tally-reassigned VOUCHERNUMBER: same voucher type, same date. + let candidates: Vec<&ObservedVoucher> = observed + .iter() + .filter(|row| { + !row.is_cancelled + && row.voucher_type.as_deref() == Some(expected.voucher_type.as_str()) + && row.date == expected_date + }) + .collect(); + + if candidates.is_empty() { + return json!({ + "source_guid": expected.source_guid, + "field": "presence", + "expected": format!("{} #{} on {}", expected.voucher_type, expected_number, expected_date), + "observed": "no voucher of this type was found on this date on readback", + }); + } + + // Best candidate: prefer an exact narration-text match (the field this + // codebase's own findings say survives every Tally rewrite), then the + // one whose ledger lines overlap the book's the most. + let best = *candidates + .iter() + .max_by_key(|row| { + let narration_hit = expected_narration_text.is_some() + && narration_text(row.narration.as_deref()) == expected_narration_text; + let ledger_hits = expected + .ledger_lines + .iter() + .filter(|line| { + let expected_signed = signed_wire_amount(&line.side, &line.amount); + row.ledger_entries.iter().any(|(ledger, is_dr, amount)| { + ledger == &line.ledger + && ((line.side == "Dr") == (is_dr == "Yes")) + && amounts_equal(amount, &expected_signed) + }) + }) + .count(); + (narration_hit, ledger_hits) + }) + .expect("candidates is non-empty, checked above"); + + let mut fields = Vec::new(); + let observed_number = best.voucher_number.as_deref().unwrap_or("(none)"); + if expected_number != observed_number { + fields.push(json!({ + "field": "voucher_number", + "expected": expected_number, + "observed": observed_number, + })); + } + let observed_marker = narration_marker(best.narration.as_deref()); + if observed_marker.as_deref() != Some(expected_marker.as_str()) { + fields.push(json!({ + "field": "narration_marker", + "expected": expected_marker, + "observed": observed_marker.unwrap_or_else(|| "(none)".to_string()), + })); + } + let observed_narration_text = narration_text(best.narration.as_deref()); + if observed_narration_text != expected_narration_text { + fields.push(json!({ + "field": "narration_text", + "expected": expected_narration_text.clone().unwrap_or_default(), + "observed": observed_narration_text.unwrap_or_default(), + })); + } + for line in &expected.ledger_lines { + let expected_signed = signed_wire_amount(&line.side, &line.amount); + let found = best.ledger_entries.iter().any(|(ledger, is_dr, amount)| { + ledger == &line.ledger + && ((line.side == "Dr") == (is_dr == "Yes")) + && amounts_equal(amount, &expected_signed) + }); + if !found { + fields.push(json!({ + "field": format!("ledger_line[{}]", line.ledger), + "expected": format!("{} {}", line.side, line.amount), + "observed": best + .ledger_entries + .iter() + .map(|(l, dr, a)| format!("{l} {dr} {a}")) + .collect::>() + .join(", "), + })); + } + } + + json!({ + "source_guid": expected.source_guid, + "candidate_observed_voucher_number": best.voucher_number, + "field_mismatches": fields, + }) +} + +/// Sort key placing vouchers into date-ordered batches, `voucher_number` +/// compared **numerically** when it parses as an integer. The original code +/// compared `voucher_number` as a plain `&str`, so for one date "100" +/// sorted before "98" -- a batch is posted to Tally in THIS order, and +/// TallyPrime auto-numbers Payment/Receipt/Contra by receipt order rather +/// than by the supplied `VOUCHERNUMBER` +/// (`tally-rewrites-what-you-import.md` #6), so a scrambled posting order +/// produces Tally-assigned numbers that no longer line up with the book's +/// own numbers. This is what actually produced the 2026-09-14 rehearsal's +/// 11 batch-1 mismatches: two same-date groups (Payment 20250518 #98-106, +/// Contra 20250609 #9-11) whose numbers cross a power-of-10 boundary were +/// sent out of numeric order. A non-numeric or missing `voucher_number` +/// falls back to a string compare against its own kind so it still sorts +/// deterministically, just not interleaved with numeric ones by value. +fn voucher_sort_key(voucher: &BookVoucher) -> (&str, Option, &str) { + let raw = voucher.voucher_number.as_deref().unwrap_or(""); + (voucher.date.as_str(), raw.parse::().ok(), raw) +} + // --------------------------------------------------------------------------- // lab_import_vouchers // --------------------------------------------------------------------------- @@ -2092,10 +2279,7 @@ pub(in crate::agent) async fn lab_import_vouchers( args: &Value, ) -> Result { let mut vouchers: Vec = parse_book_value(args, "vouchers", "vouchers")?; - vouchers.sort_by(|a, b| { - (a.date.as_str(), a.voucher_number.as_deref().unwrap_or("")) - .cmp(&(b.date.as_str(), b.voucher_number.as_deref().unwrap_or(""))) - }); + vouchers.sort_by(|a, b| voucher_sort_key(a).cmp(&voucher_sort_key(b))); let guid = required_string(args, "company_guid")?; let start_batch = arg_usize(args, "start_batch", 0)?; @@ -2151,9 +2335,15 @@ pub(in crate::agent) async fn lab_import_vouchers( // Partial match on an uncertain prior attempt: stop rather than // guess which subset is safe to resend. Returned immediately // below, so this batch never reaches `stopped_at`'s summary use. + let mismatch_details: Vec = batch + .iter() + .filter(|v| !voucher_already_verified(v, &observed)) + .map(|v| voucher_mismatch_detail(v, &observed)) + .collect(); batch_reports.push(json!({ "batch": batch_index, "count": batch.len(), "state": "partially_verified_uncertain", "verified": verified_count, "posted": false, + "mismatch_details": mismatch_details, })); return Err( ToolFailure::from("lab_batch_partially_verified_uncertain".to_string()) @@ -2161,7 +2351,13 @@ pub(in crate::agent) async fn lab_import_vouchers( ); } - let attribution_ids: Vec = batch.iter().map(|_| Uuid::new_v4()).collect(); + // Deterministic, not `Uuid::new_v4()`: the marker embedded here must + // be reconstructible from `source_guid` alone on a later precheck or + // readback (this run's or a resumed one's) -- see `lab_marker_id`. + let attribution_ids: Vec = batch + .iter() + .map(|v| lab_marker_id(&v.source_guid)) + .collect(); let xml = render_voucher_batch_xml(identity.display_name(), batch, &attribution_ids) .map_err(ToolFailure::from)?; let (response, post_evidence) = @@ -2208,6 +2404,15 @@ pub(in crate::agent) async fn lab_import_vouchers( .filter(|v| voucher_already_verified(v, &readback)) .count(); let batch_ok = clean && posted_count == batch.len(); + let mismatch_details: Vec = if batch_ok { + Vec::new() + } else { + batch + .iter() + .filter(|v| !voucher_already_verified(v, &readback)) + .map(|v| voucher_mismatch_detail(v, &readback)) + .collect() + }; batch_reports.push(json!({ "batch": batch_index, @@ -2217,6 +2422,7 @@ pub(in crate::agent) async fn lab_import_vouchers( "state": if batch_ok { "posted_verified" } else { "readback_mismatch" }, "posted": true, "source_guids": source_guids, + "mismatch_details": mismatch_details, })); if !batch_ok { stopped_at = Some(batch_index); diff --git a/src-tauri/src/agent_lab_import_tests.rs b/src-tauri/src/agent_lab_import_tests.rs index a0eabe67..25766aa4 100644 --- a/src-tauri/src/agent_lab_import_tests.rs +++ b/src-tauri/src/agent_lab_import_tests.rs @@ -690,6 +690,172 @@ fn narration_marker_extracts_the_bracketed_uuid() { assert_eq!(narration_marker(None), None); } +// --------------------------------------------------------------------------- +// lab_marker_id -- deterministic marker (2026-09-14 rehearsal fix) +// --------------------------------------------------------------------------- + +#[test] +fn lab_marker_id_is_deterministic_and_distinct_per_source_guid() { + assert_eq!(lab_marker_id("src-1"), lab_marker_id("src-1")); + assert_ne!(lab_marker_id("src-1"), lab_marker_id("src-2")); +} + +#[test] +fn voucher_already_verified_matches_via_the_deterministic_marker_when_the_number_is_absent() { + // Simulates a resume: the book no longer supplies a voucher_number + // (or Tally reassigned it), but the marker this module embedded at + // write time -- now derived from source_guid, not a random UUID -- is + // still recoverable from the readback. + let mut expected = payment_voucher(); + expected.voucher_number = None; + let marker = lab_marker_id(&expected.source_guid); + let matching = observed( + "Payment", + "20260405", + Some("999"), // Tally's own reassigned number -- deliberately not "59" + Some(&format!("UPI payment [BRIDGE-LAB:{marker}]")), + &[ + ("HDFC Bank 1649", "No", "30000.00"), + ("Labour Charges", "Yes", "-30000.00"), + ], + ); + assert!(voucher_already_verified(&expected, &[matching])); +} + +#[test] +fn narration_text_strips_the_marker_suffix_and_trims() { + assert_eq!( + narration_text(Some( + "UPI payment [BRIDGE-LAB:00000000-0000-4000-8000-000000000002]" + )), + Some("UPI payment".to_string()) + ); + assert_eq!( + narration_text(Some(" plain narration ")), + Some("plain narration".to_string()) + ); + assert_eq!(narration_text(Some(" ")), None); + assert_eq!(narration_text(None), None); +} + +#[test] +fn voucher_already_verified_matches_via_narration_text_when_tally_reassigned_the_number() { + // Reproduces the exact 2026-09-14 rehearsal batch-1 failure: Tally + // silently reassigned VOUCHERNUMBER (tally-rewrites-what-you-import.md + // #6) and the observed marker is a stale random one from a write + // attempt that predates the `lab_marker_id` fix -- so neither the + // number nor the marker matches. The narration TEXT (minus any marker + // suffix) is the only surviving identity signal, and it must still be + // enough on its own when the ledger lines also agree. + let expected = payment_voucher(); // voucher_number = Some("59") + let renumbered_by_tally = observed( + "Payment", + "20260405", + Some("57"), // NOT "59" -- Tally's own receipt-order number + Some("UPI payment [BRIDGE-LAB:11111111-1111-4111-8111-111111111111]"), // stale, pre-fix marker + &[ + ("HDFC Bank 1649", "No", "30000.00"), + ("Labour Charges", "Yes", "-30000.00"), + ], + ); + assert!(voucher_already_verified(&expected, &[renumbered_by_tally])); +} + +#[test] +fn voucher_already_verified_still_refuses_a_narration_collision_with_wrong_ledger_content() { + // The narration-text key is an ADDITIONAL alternate, not a licence to + // skip the ledger-line check: same narration text, wrong amount, must + // still be refused. + let expected = payment_voucher(); + let wrong_amount = observed( + "Payment", + "20260405", + Some("57"), + Some("UPI payment [BRIDGE-LAB:11111111-1111-4111-8111-111111111111]"), + &[ + ("HDFC Bank 1649", "No", "5.00"), + ("Labour Charges", "Yes", "-5.00"), + ], + ); + assert!(!voucher_already_verified(&expected, &[wrong_amount])); +} + +// --------------------------------------------------------------------------- +// voucher_sort_key -- numeric, not lexicographic (2026-09-14 rehearsal fix) +// --------------------------------------------------------------------------- + +#[test] +fn voucher_sort_key_orders_voucher_numbers_numerically_within_a_date() { + // The 2026-09-14 rehearsal's exact digit-boundary case: a plain string + // compare puts "100".."106" before "98"/"99" (since '1' < '9'), which + // is what scrambled batch 1's posting order in the first place. + let mut numbers = vec!["100", "101", "106", "98", "99"]; + numbers.sort_by_key(|n| { + let raw: &str = n; + raw.parse::().ok() + }); + assert_eq!(numbers, vec!["98", "99", "100", "101", "106"]); + + fn voucher(date: &str, number: &str) -> BookVoucher { + let mut v = payment_voucher(); + v.date = date.to_string(); + v.voucher_number = Some(number.to_string()); + v + } + let mut vouchers = vec![ + voucher("20250518", "100"), + voucher("20250518", "98"), + voucher("20250518", "99"), + voucher("20250518", "101"), + ]; + vouchers.sort_by(|a, b| voucher_sort_key(a).cmp(&voucher_sort_key(b))); + let ordered: Vec<&str> = vouchers + .iter() + .map(|v| v.voucher_number.as_deref().unwrap()) + .collect(); + assert_eq!(ordered, vec!["98", "99", "100", "101"]); +} + +// --------------------------------------------------------------------------- +// voucher_mismatch_detail -- per-voucher diagnostic (2026-09-14 rehearsal fix) +// --------------------------------------------------------------------------- + +#[test] +fn voucher_mismatch_detail_reports_not_found_when_no_candidate_exists() { + let expected = payment_voucher(); + let detail = voucher_mismatch_detail(&expected, &[]); + assert_eq!(detail["source_guid"], "src-1"); + assert_eq!(detail["field"], "presence"); +} + +#[test] +fn voucher_mismatch_detail_names_the_voucher_number_field_for_a_tally_renumbered_candidate() { + // Same scenario as the narration-text test above, but this time from + // the reporting side: the tool must surface the wrong-number, + // wrong-marker candidate it found, not just a bare count. + let expected = payment_voucher(); + let renumbered_by_tally = observed( + "Payment", + "20260405", + Some("57"), + Some("UPI payment [BRIDGE-LAB:11111111-1111-4111-8111-111111111111]"), + &[ + ("HDFC Bank 1649", "No", "30000.00"), + ("Labour Charges", "Yes", "-30000.00"), + ], + ); + let detail = voucher_mismatch_detail(&expected, &[renumbered_by_tally]); + assert_eq!(detail["source_guid"], "src-1"); + assert_eq!(detail["candidate_observed_voucher_number"], "57"); + let fields = detail["field_mismatches"].as_array().unwrap(); + assert!(fields + .iter() + .any(|f| f["field"] == "voucher_number" && f["expected"] == "59" && f["observed"] == "57")); + assert!(fields.iter().any(|f| f["field"] == "narration_marker")); + // The narration TEXT matched, so it must not appear as a mismatched field. + assert!(!fields.iter().any(|f| f["field"] == "narration_text")); +} + // --------------------------------------------------------------------------- // Voucher read-back parsing (synthetic Tally export) // --------------------------------------------------------------------------- From 9f25b210ecf49be97bc2f6a5a4616e6de410d759 Mon Sep 17 00:00:00 2001 From: t Date: Wed, 16 Sep 2026 00:06:14 +0530 Subject: [PATCH 13/14] chore(lab): reseal the compatibility surface and regenerate the Rust inventory This branch had edited five pinned files without resealing, and added a dependency without regenerating the third-party inventory. Both are gates, so the branch was failing CI on its own: a control run on the bare base failed job-for-job identically to a pull request stacked on it, which made a stacked PR's own result unreadable. - `scripts/reseal.sh` (ordinary sequence; the pin list itself did not change, only the contents of already-pinned files). `rehash-surface` reported exactly 5 changed hashes, matching an independent hash of all 212 pinned entries against the manifest: `agent.rs`, `agent_read_profiles.rs`, `tally/runtime.rs`, `Cargo.toml`, `Cargo.lock`. - `scripts/generate-rust-licenses.mjs` for the inventory, which had been missing `sha1_smol 1.0.1` since this branch added it. Now matches 365 locked components. The lab module's own files are deliberately not pinned: the surface attests compatibility claims, and this module states in its own header that no signed evidence exists for anything it reads. Order checked rather than assumed: regenerating the inventory rewrites `THIRD_PARTY_LICENSES_RUST.txt`, which would invalidate a reseal done before it, but that file is not among the pinned entries and `reseal.sh --verify` still reports the surface current afterwards. Verified: `tools` workspace 24/24 on the surface-coverage test that was the dominant failure, `cargo fmt --check` clean, default-feature lib suite 915 passing, dependency inventory clean. Co-Authored-By: Claude Opus 5 --- THIRD_PARTY_LICENSES_RUST.txt | 5 +++++ docs/tally/compatibility/compatibility-matrix.json | 2 +- docs/tally/compatibility/compatibility-surface.json | 12 ++++++------ 3 files changed, 12 insertions(+), 7 deletions(-) diff --git a/THIRD_PARTY_LICENSES_RUST.txt b/THIRD_PARTY_LICENSES_RUST.txt index 7329b8ed..516ffd54 100644 --- a/THIRD_PARTY_LICENSES_RUST.txt +++ b/THIRD_PARTY_LICENSES_RUST.txt @@ -975,6 +975,10 @@ servo_arc 0.4.3 License: MIT OR Apache-2.0 Source: https://github.com/servo/stylo +sha1_smol 1.0.1 +License: BSD-3-Clause +Source: https://github.com/mitsuhiko/sha1-smol + sha2 0.10.9 License: MIT OR Apache-2.0 Source: https://github.com/RustCrypto/hashes @@ -10847,6 +10851,7 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. BSD 3-Clause "New" or "Revised" License - alloc-stdlib 0.2.4 - aws-lc-sys 0.44.0 +- sha1_smol 1.0.1 Copyright (c) . diff --git a/docs/tally/compatibility/compatibility-matrix.json b/docs/tally/compatibility/compatibility-matrix.json index b0961b10..480d552f 100644 --- a/docs/tally/compatibility/compatibility-matrix.json +++ b/docs/tally/compatibility/compatibility-matrix.json @@ -1,7 +1,7 @@ { "schema_version": 1, "bridge_commit_sha": "be1c20cc3fd66fa1ece196505c69f26e555e4b8e", - "compatibility_surface_sha256": "f610a58ad2bb994388a4848b96cf789f2be536b715f82a74712ab677193f8e88", + "compatibility_surface_sha256": "34b73f16376e830031ff753e756a15895eb06a30381ebf813e236cc08b1bdb28", "claims": [ { "claim_id": "erp9-6-6-3-windows-education-xml-one-company", diff --git a/docs/tally/compatibility/compatibility-surface.json b/docs/tally/compatibility/compatibility-surface.json index f0959c39..3c8eed3f 100644 --- a/docs/tally/compatibility/compatibility-surface.json +++ b/docs/tally/compatibility/compatibility-surface.json @@ -123,11 +123,11 @@ }, { "path": "src-tauri/Cargo.lock", - "sha256": "e099a547e82382740f7f62e3f041c2523e83cb16cf0f598b92b5b07be3009f70" + "sha256": "a6c518833d11a03ddafe449ddc3b0b12043271af30cf7f81ea9be9d8e47081f2" }, { "path": "src-tauri/Cargo.toml", - "sha256": "d571f7a8ce0e40147bdb5a1b9d7757ada2536c22c040d408bf8ad6a668fe44a6" + "sha256": "15ff2162eb7398fcfcfeb822f40688a3bb075e1231ee8ae792f47576bf01a7c9" }, { "path": "src-tauri/crates/bridge-tally-core/Cargo.toml", @@ -323,7 +323,7 @@ }, { "path": "src-tauri/src/agent.rs", - "sha256": "83824bf04e0d6b10b2cc3ed852a1600ef9472fe37966c6dce4d318efe58f1e9b" + "sha256": "c38117484fe07913e0ac6703b0158ee8dab7a327c93ba0dc83943ce543b12b4d" }, { "path": "src-tauri/src/agent_desktop_journal.rs", @@ -339,7 +339,7 @@ }, { "path": "src-tauri/src/agent_read_profiles.rs", - "sha256": "cbcc6831046134158fad25ca3bbbda34f5060f0ab6d299b428f996e541b7e072" + "sha256": "f6f07dbcbce22498e4e4e6244cfd19b12bc017749ed9f0a45274f9354e2dba51" }, { "path": "src-tauri/src/agent_read_validation.rs", @@ -643,7 +643,7 @@ }, { "path": "src-tauri/src/tally/runtime.rs", - "sha256": "de6634ae5e09b126a4c451ca6d7e43f25e787d98f0fb94d93b78a5bc13165675" + "sha256": "9f1e388637760ddb0aa5c4de884c9f9dbc66e5714ace4cdd6bdbd2949888a81b" }, { "path": "src-tauri/src/tally/runtime_trial_balance.rs", @@ -850,5 +850,5 @@ "sha256": "a8ac2714fecf51947f2822c8c46d7ce2e8602c732780ff60566a7771f0836f9a" } ], - "manifest_sha256": "f610a58ad2bb994388a4848b96cf789f2be536b715f82a74712ab677193f8e88" + "manifest_sha256": "34b73f16376e830031ff753e756a15895eb06a30381ebf813e236cc08b1bdb28" } \ No newline at end of file From 7e3d0bc827a1b5b1c23a27a694911c4658734d9b Mon Sep 17 00:00:00 2001 From: Tapish Khandelwal <57982425+lamemustafa@users.noreply.github.com> Date: Wed, 16 Sep 2026 00:38:22 +0530 Subject: [PATCH 14/14] fix(lab): bind a field's text to the element that closes it (#403) Three parsers in the lab surface tracked a bare `current_tag` and appended every text event to it, choosing the owning row from a depth-shifted `path`. Two properties of real Tally responses defeat that, and no fixture in the module had either, because every one was compact: - Responses are CRLF-indented, so `trim_text(false)` delivers the whitespace between every pair of siblings as its own `Text` event -- 398 such runs in one captured inventory entry. - They are dense with self-closing elements -- 274 in that same entry -- which arrive as `Event::Empty` and never updated `current_tag`. One stale tag therefore absorbed a whole indentation run: an entry's `AMOUNT` read back with forty-odd `"\r\n "` fragments, a `GODOWNNAME` picked up the whitespace closing the batch below it, and master rows grew phantom whitespace-only fields. Separately, a master row was seeded from its `NAME=` attribute and then had the `` child appended onto it, giving `KgsKgs`. Replaced with a buffer that holds an element's text and releases it only when that element closes with no child having intervened, resolving row ownership at `End` where the path is unambiguous. `Text`, `GeneralRef` and `CData` all feed one buffer, so a value split across any combination rejoins in order. The `NAME=` attribute became the fallback for a row with no `` element. Deliberately not a trim: Tally left-pads a quantity with one real space to hold the sign position, on every `ACTUALQTY` and `BILLEDQTY` in the captured days. Trimming would corrupt correct output while making the symptom vanish. The fixtures carry that shape and assert it exactly. `parse_voucher_readback_nested` in `agent_lab_import.rs` carried the identical defect and is what the import mismatch report compares, so an `AMOUNT` growing an indentation tail reported a false mismatch against a voucher Tally had stored correctly. Converted to the same buffer. Eight regression tests, each confirmed to fail against the parser as it stood and pass after. Verified beyond the fixtures by replaying the fixed parser over three captured inventory days -- 42 vouchers, 46 entries, 17 batch allocations, no accumulation -- where the previous parser fails on the first entry. Those captures are client data and stay outside the repository. Two independent reviews. The first found `Event::CData` was being dropped, which is what turned up the third parser. The second found the read-back test used a nested element no assertion read, so it passed against the broken parser; it now collides with a field the entry binds. Does not close bridge#379: this lands on the lab branch, not master. --- src-tauri/src/agent_lab.rs | 536 +++++++++++++++++++++--- src-tauri/src/agent_lab_import.rs | 57 +-- src-tauri/src/agent_lab_import_tests.rs | 70 ++++ 3 files changed, 579 insertions(+), 84 deletions(-) diff --git a/src-tauri/src/agent_lab.rs b/src-tauri/src/agent_lab.rs index af3f58a0..6949a497 100644 --- a/src-tauri/src/agent_lab.rs +++ b/src-tauri/src/agent_lab.rs @@ -309,6 +309,67 @@ fn render_lab_master_collection(company: &str, kind: LabMasterKind) -> Result Option<(String, String)> { + let field = if self.live && self.tag == end && !self.text.is_empty() { + Some((self.tag.clone(), std::mem::take(&mut self.text))) + } else { + None + }; + self.text.clear(); + self.live = false; + field + } +} + /// Parses a flat master collection (`...` etc, one /// level under `COLLECTION`) into raw field maps. Deliberately conservative /// like the production parsers: an unexpected non-row child of `COLLECTION` @@ -324,7 +385,13 @@ fn parse_lab_master_rows( let mut path: Vec = Vec::new(); let mut rows = Vec::new(); let mut current: Option> = None; - let mut current_tag = String::new(); + // The row's `NAME=` attribute, held aside rather than written straight into + // the row. Tally emits both the attribute and a `` child element + // (`KGS`), and appending the second onto + // the first is what read a unit back as `KGSKGS` -- the other half of + // bridge#379. The element is authoritative; the attribute is the fallback. + let mut attribute_name: Option = None; + let mut buffer = LabTextBuffer::default(); loop { match reader.read_event() { Ok(quick_xml::events::Event::Start(event)) => { @@ -334,13 +401,12 @@ fn parse_lab_master_rows( if tag != row_tag { return Err("agent_read_protocol_invalid".to_string()); } - let mut row = BTreeMap::new(); + attribute_name = None; for attribute in event.attributes() { let attribute = attribute.map_err(|_| "agent_read_protocol_invalid".to_string())?; if attribute.key.as_ref().eq_ignore_ascii_case(b"NAME") { - row.insert( - "NAME".to_string(), + attribute_name = Some( attribute .decoded_and_normalized_value( quick_xml::XmlVersion::Implicit1_0, @@ -351,47 +417,62 @@ fn parse_lab_master_rows( ); } } - current = Some(row); + current = Some(BTreeMap::new()); } path.push(tag.clone()); - current_tag = tag; + buffer.open(&tag); } Ok(quick_xml::events::Event::Text(text)) => { - // `path` includes the just-opened field tag (`current_tag`); - // a field belongs to the row if its parent chain is exactly - // COLLECTION_PREFIX + [row_tag]. - let is_row_field = path.len() == 6 - && path[..4] == ["ENVELOPE", "BODY", "DATA", "COLLECTION"] - && path[4] == row_tag; - if is_row_field { - if let Some(row) = current.as_mut() { - append_agent_text(row, ¤t_tag, decoded_agent_text(text)?); - } - } + buffer.push(&decoded_agent_text(text)?); } // See the identical arm in `agent_lab_import.rs`'s // `parse_voucher_readback_nested`: quick_xml delivers an entity // reference (`&`, ...) as its own `GeneralRef` event, not // inline within `Text`. Without this arm it is silently dropped // by the catch-all below -- the exact 2026-09-14 rehearsal bug - // that read "Duties & Taxes" back as "Duties Taxes". + // that read "Duties & Taxes" back as "Duties Taxes". Both event + // kinds feed one buffer, so a value split across them is rejoined. Ok(quick_xml::events::Event::GeneralRef(reference)) => { - let is_row_field = path.len() == 6 - && path[..4] == ["ENVELOPE", "BODY", "DATA", "COLLECTION"] - && path[4] == row_tag; - if is_row_field { - if let Some(row) = current.as_mut() { - append_agent_text(row, ¤t_tag, decoded_agent_reference(reference)?); - } - } + buffer.push(&decoded_agent_reference(reference)?); + } + // Tally splits a scalar across CDATA too: the production parser's + // `scalar_content_preserves_cdata_and_rejects_nested_markup` pins + // `` and `-10101` + // as having to read identically to the plain text. Dropped here, + // the first yields nothing and the second yields `-10101` -- a + // wrong number that still looks like one. Same buffer, so a value + // split across Text, GeneralRef and CDATA rejoins in order. + Ok(quick_xml::events::Event::CData(text)) => { + buffer.push( + &text + .decode() + .map_err(|_| "agent_read_protocol_invalid".to_string())?, + ); } + Ok(quick_xml::events::Event::Empty(_)) => buffer.abandon(), Ok(quick_xml::events::Event::End(event)) => { let end = String::from_utf8_lossy(event.name().as_ref()).to_ascii_uppercase(); + // A field belongs to the row when its parent chain is exactly + // COLLECTION_PREFIX + [row_tag]; `path` still holds the closing + // element, so the parent chain is everything before it. + if let Some((field, value)) = buffer.close(&end) { + let is_row_field = path.len() == 6 + && path[..4] == ["ENVELOPE", "BODY", "DATA", "COLLECTION"] + && path[4] == row_tag; + if is_row_field { + if let Some(row) = current.as_mut() { + append_agent_text(row, &field, value); + } + } + } if path.last().map(String::as_str) == Some(end.as_str()) && end == row_tag && path.len() == 5 { - if let Some(row) = current.take() { + if let Some(mut row) = current.take() { + if let Some(name) = attribute_name.take() { + row.entry("NAME".to_string()).or_insert(name); + } rows.push(row); } } @@ -490,7 +571,7 @@ fn parse_lab_inventory_vouchers(xml: &str) -> Result, String> { let mut batch: Option> = None; let mut entries: Vec = Vec::new(); let mut batches: Vec = Vec::new(); - let mut current_tag = String::new(); + let mut buffer = LabTextBuffer::default(); loop { match reader.read_event() { Ok(quick_xml::events::Event::Start(event)) => { @@ -509,45 +590,52 @@ fn parse_lab_inventory_vouchers(xml: &str) -> Result, String> { batch = Some(BTreeMap::new()); } path.push(tag.clone()); - current_tag = tag; + buffer.open(&tag); } Ok(quick_xml::events::Event::Text(text)) => { - let value = decoded_agent_text(text)?; - // `path` here includes the just-opened `current_tag`, so a - // field at depth N+1 belongs to the container at depth N. - if path_is(&path[..path.len().saturating_sub(1)], &BATCH_PREFIX) { - if let Some(row) = batch.as_mut() { - append_agent_text(row, ¤t_tag, value); - } - } else if path_is(&path[..path.len().saturating_sub(1)], &ENTRY_PREFIX) { - if let Some(row) = entry.as_mut() { - append_agent_text(row, ¤t_tag, value); - } - } else if path_is(&path[..path.len().saturating_sub(1)], &VOUCHER_PREFIX) { - if let Some(row) = voucher.as_mut() { - append_agent_text(row, ¤t_tag, value); - } - } + buffer.push(&decoded_agent_text(text)?); } - // Same entity-reference gap as `parse_lab_master_rows` above. + // Same entity-reference gap as `parse_lab_master_rows` above; both + // event kinds feed one buffer so a value split across them rejoins. Ok(quick_xml::events::Event::GeneralRef(reference)) => { - let value = decoded_agent_reference(reference)?; - if path_is(&path[..path.len().saturating_sub(1)], &BATCH_PREFIX) { - if let Some(row) = batch.as_mut() { - append_agent_text(row, ¤t_tag, value); - } - } else if path_is(&path[..path.len().saturating_sub(1)], &ENTRY_PREFIX) { - if let Some(row) = entry.as_mut() { - append_agent_text(row, ¤t_tag, value); - } - } else if path_is(&path[..path.len().saturating_sub(1)], &VOUCHER_PREFIX) { - if let Some(row) = voucher.as_mut() { - append_agent_text(row, ¤t_tag, value); - } - } + buffer.push(&decoded_agent_reference(reference)?); } + // Tally splits a scalar across CDATA too: the production parser's + // `scalar_content_preserves_cdata_and_rejects_nested_markup` pins + // `` and `-10101` + // as having to read identically to the plain text. Dropped here, + // the first yields nothing and the second yields `-10101` -- a + // wrong number that still looks like one. Same buffer, so a value + // split across Text, GeneralRef and CDATA rejoins in order. + Ok(quick_xml::events::Event::CData(text)) => { + buffer.push( + &text + .decode() + .map_err(|_| "agent_read_protocol_invalid".to_string())?, + ); + } + Ok(quick_xml::events::Event::Empty(_)) => buffer.abandon(), Ok(quick_xml::events::Event::End(event)) => { let end = String::from_utf8_lossy(event.name().as_ref()).to_ascii_uppercase(); + // `path` still holds the closing element, so its parent chain + // names the container the field belongs to. Resolving this at + // `End` rather than at each text event is what keeps a + // container's own indentation out of its siblings' values. + if let Some((field, value)) = buffer.close(&end) { + let parent = &path[..path.len().saturating_sub(1)]; + let row = if path_is(parent, &BATCH_PREFIX) { + batch.as_mut() + } else if path_is(parent, &ENTRY_PREFIX) { + entry.as_mut() + } else if path_is(parent, &VOUCHER_PREFIX) { + voucher.as_mut() + } else { + None + }; + if let Some(row) = row { + append_agent_text(row, &field, value); + } + } let closing_batch = end == "BATCHALLOCATIONS.LIST" && path_is(&path, &BATCH_PREFIX); let closing_entry = end == "ALLINVENTORYENTRIES.LIST" && path_is(&path, &ENTRY_PREFIX); @@ -916,6 +1004,334 @@ mod tests { ); } + /// Builds one inventory response twice over. `separator` is what the + /// gateway puts between sibling elements: `""` for a compact response and a + /// CRLF indent for a pretty-printed one. Both are shapes the real gateway + /// returns -- the captured `units` collection is compact while the captured + /// inventory days are indented -- and bridge#379 only ever reproduced on the + /// indented one, which is why every fixture here predating it was compact. + /// + /// Synthetic throughout: the captured payloads that established this shape + /// are from the client book and stay out of the repository. + fn synthetic_inventory_vouchers(count: usize, separator: &str) -> String { + let mut parts: Vec = [ + "", + "
", + "1", + "
", + "", + "", + "", + ] + .iter() + .map(|part| part.to_string()) + .collect(); + for index in 1..=count { + parts.extend([ + "".to_string(), + format!("2026040{index}"), + // A self-closing element arrives as `Event::Empty`. It never + // updated the old `current_tag`, so the stale tag before it + // went on absorbing every indentation run that followed; one + // captured inventory entry holds 274 of these. + "".to_string(), + format!("{index}"), + "Sales".to_string(), + // Entity reference: quick_xml splits this across Text and + // GeneralRef events, which must rejoin into one value. + "Fixture Supplies & Co".to_string(), + format!("fixture-guid-{index}"), + "No".to_string(), + "".to_string(), + "Sodium Bicarbonate".to_string(), + "".to_string(), + "".to_string(), + "80.00/Kgs".to_string(), + "-8000.00".to_string(), + " 100.000 Kgs".to_string(), + " 100.000 Kgs".to_string(), + // The entry names a godown and so does each batch under it. + // That pair is what read back as "Main Location\r\n ": + // the whitespace closing the batch landed on the entry's own + // field, because the tag that had just closed was still live. + "Main Location".to_string(), + "".to_string(), + "Batch-01".to_string(), + "Main Location".to_string(), + " 60.000 Kgs".to_string(), + " 60.000 Kgs".to_string(), + "-4800.00".to_string(), + // A third level of nesting, as real purchases carry. Nothing + // inside it may reach the batch or the entry. + "".to_string(), + "Sales Account".to_string(), + "-4800.00".to_string(), + "".to_string(), + "".to_string(), + // One stock item split across two batches in two godowns. + "".to_string(), + "Batch-02".to_string(), + "Second Location".to_string(), + " 40.000 Kgs".to_string(), + " 40.000 Kgs".to_string(), + "-3200.00".to_string(), + "".to_string(), + "".to_string(), + "".to_string(), + ]); + } + parts.extend( + ["", "", "", ""] + .iter() + .map(|part| part.to_string()), + ); + parts.join(separator) + } + + const INDENT: &str = "\r\n "; + + /// Mirrors the captured `units` collection: Tally names the unit in the + /// row's `NAME=` attribute *and* repeats it in a `` child element. + fn synthetic_unit_collection_with_name_elements(separator: &str) -> String { + let parts = [ + "", + "
", + "1", + "
", + "", + "", + "", + "", + "", + "", + "Kgs", + "Yes", + "3", + // A nested container: its own indentation must not become a field + // of the unit, and neither must its children's values. + "", + "20250401", + "KGS", + "", + "", + // No `` child at all: the attribute has to stand in for it. + "", + "Yes", + "0", + "", + "", + "", + "", + "", + ]; + parts.join(separator) + } + + #[test] + fn a_unit_name_element_does_not_double_the_name_attribute() { + // bridge#379: `Kgs` seeded the row from + // the attribute and then appended the element onto it, reading back as + // "KgsKgs". The element is authoritative; the attribute is a fallback. + for separator in ["", INDENT] { + let xml = synthetic_unit_collection_with_name_elements(separator); + let rows = parse_lab_master_rows(&xml, "Unit").unwrap(); + assert_eq!(rows.len(), 2, "separator {separator:?}"); + assert_eq!(rows[0].get("NAME").map(String::as_str), Some("Kgs")); + assert_eq!(rows[0].get("DECIMALPLACES").map(String::as_str), Some("3")); + // The attribute still stands in where no element supplies a name. + assert_eq!(rows[1].get("NAME").map(String::as_str), Some("Nos")); + assert_eq!( + lab_master_json(LabMasterKind::Unit, &rows[0])["name"], + json!(party_name("Kgs")) + ); + assert_eq!( + lab_master_json(LabMasterKind::Unit, &rows[1])["name"], + json!(party_name("Nos")) + ); + } + } + + #[test] + fn an_indented_master_row_keeps_nested_containers_out_of_its_fields() { + let xml = synthetic_unit_collection_with_name_elements(INDENT); + let rows = parse_lab_master_rows(&xml, "Unit").unwrap(); + for (key, value) in &rows[0] { + assert!( + !value.contains('\r') && !value.contains('\n'), + "{key} carries an indentation fragment: {value:?}" + ); + assert!( + !value.trim().is_empty(), + "{key} is a phantom whitespace-only field" + ); + } + // The nested list is a container, never a field of the unit, and its + // children belong to it rather than to the row above. + assert!(!rows[0].contains_key("REPORTINGUQCDETAILS.LIST")); + assert!(!rows[0].contains_key("APPLICABLEFROM")); + assert!(!rows[0].contains_key("REPORTINGUQCNAME")); + } + + #[test] + fn an_indented_inventory_response_parses_exactly_like_a_compact_one() { + // The acceptance condition on bridge#379: both shapes are real, and + // they must agree field for field. + let indented = parse_lab_inventory_vouchers(&synthetic_inventory_vouchers(2, INDENT)) + .expect("indented response parses"); + let compact = parse_lab_inventory_vouchers(&synthetic_inventory_vouchers(2, "")) + .expect("compact response parses"); + assert_eq!(indented, compact); + } + + #[test] + fn indented_inventory_fields_equal_the_fixture_values_exactly() { + let rows = parse_lab_inventory_vouchers(&synthetic_inventory_vouchers(1, INDENT)).unwrap(); + assert_eq!(rows.len(), 1); + assert_eq!(rows[0]["date"], "20260401"); + assert_eq!(rows[0]["voucher_number"], "1"); + assert_eq!(rows[0]["party"], json!(party_name("Fixture Supplies & Co"))); + + let entries = rows[0]["inventory_entries"].as_array().unwrap(); + assert_eq!(entries.len(), 1); + let entry = &entries[0]; + assert_eq!(entry["stock_item"], "Sodium Bicarbonate"); + assert_eq!(entry["rate"], "80.00/Kgs"); + assert_eq!(entry["amount"], "-8000.00"); + // The leading space is Tally's own and must survive intact; only + // the indentation that followed the value was ever spurious. + assert_eq!(entry["actual_qty"], " 100.000 Kgs"); + assert_eq!(entry["billed_qty"], " 100.000 Kgs"); + // The reported symptom, verbatim: this came back "Main Location\r\n ". + assert_eq!(entry["godown"], "Main Location"); + + let batches = entry["batch_allocations"].as_array().unwrap(); + assert_eq!(batches.len(), 2); + assert_eq!(batches[0]["batch"], "Batch-01"); + assert_eq!(batches[0]["godown"], "Main Location"); + assert_eq!(batches[0]["actual_qty"], " 60.000 Kgs"); + assert_eq!(batches[0]["amount"], "-4800.00"); + assert_eq!(batches[1]["batch"], "Batch-02"); + assert_eq!(batches[1]["godown"], "Second Location"); + assert_eq!(batches[1]["billed_qty"], " 40.000 Kgs"); + + // Nothing anywhere in the tree carries a whitespace tail. + assert_no_accumulation(&rows[0]); + } + + /// Walks every string in the parsed tree and fails on the accumulation + /// signature: a line break inside a value, or a trailing whitespace run. + /// + /// Deliberately not a blanket `trim` check. Tally left-pads a quantity + /// with one real space, holding the sign position: every `ACTUALQTY` and + /// `BILLEDQTY` across the captured inventory days carries it. Trimming + /// values in the parser would corrupt correct output while appearing to fix + /// bridge#379, so the fixtures below carry that shape and assert it exactly. + fn assert_no_accumulation(value: &Value) { + match value { + Value::String(text) => { + assert!( + !text.contains('\r') && !text.contains('\n'), + "value carries a line break: {text:?}" + ); + assert_eq!( + text.trim_end(), + text, + "value carries a trailing indentation run: {text:?}" + ); + } + Value::Array(items) => items.iter().for_each(assert_no_accumulation), + Value::Object(fields) => fields.values().for_each(assert_no_accumulation), + _ => {} + } + } + + #[test] + fn parsed_size_scales_with_voucher_count_rather_than_with_indentation() { + // The accumulation inflated a parsed window in proportion to how many + // elements followed each field, which is what pushed a full month past + // `agent_response_too_large`. The control is that the same vouchers, + // sent indented and compact, parse to the same number of bytes: on the + // pre-fix parser the indented form was the wider of the two. + let width = |count: usize, separator: &str| { + parse_lab_inventory_vouchers(&synthetic_inventory_vouchers(count, separator)) + .unwrap() + .iter() + .map(|row| serde_json::to_string(row).unwrap().len()) + .sum::() + }; + for count in [1, 4] { + assert_eq!( + width(count, INDENT), + width(count, ""), + "{count} indented vouchers parse wider than the same compact ones" + ); + } + // And what is left grows with the voucher count alone. GUIDs and dates + // differ by a character or two between vouchers, so allow a small + // constant rather than demanding an exact multiple. + assert!( + width(4, INDENT).abs_diff(width(1, INDENT) * 4) < 16, + "parsed width {} is not four times {}", + width(4, INDENT), + width(1, INDENT) + ); + } + + #[test] + fn nested_accounting_allocations_stay_out_of_the_batch_and_the_entry() { + let rows = parse_lab_inventory_vouchers(&synthetic_inventory_vouchers(1, INDENT)).unwrap(); + let entries = rows[0]["inventory_entries"].as_array().unwrap(); + let batches = entries[0]["batch_allocations"].as_array().unwrap(); + // The accounting allocation under Batch-01 also carries an AMOUNT. + // It must not overwrite or extend the batch's own, nor the entry's. + assert_eq!(batches[0]["amount"], "-4800.00"); + assert_eq!(entries[0]["amount"], "-8000.00"); + } + + #[test] + fn a_scalar_split_across_cdata_rejoins_in_both_parsers() { + // The production parser pins this shape in + // `scalar_content_preserves_cdata_and_rejects_nested_markup`: a value + // carried in or split by a CDATA section must read identically to the + // same value as plain text. `Event::CData` is its own event, so a + // parser without an arm for it drops the fragment silently -- and for + // an AMOUNT that yields a wrong number that still looks like one. + let voucher = |amount: &str| { + format!( + "
1
\ +202604011\ +Salesfixture-guid-1\ +No\ +Sodium Bicarbonate\ +{amount}\ +" + ) + }; + let plain = parse_lab_inventory_vouchers(&voucher("-101.01")).unwrap(); + for split in [ + "-10101", + "", + "-101", + ] { + let got = parse_lab_inventory_vouchers(&voucher(split)).unwrap(); + assert_eq!(got, plain, "inventory parser lost the CDATA in {split:?}"); + } + + // The same for a master row's scalar. + let unit = |places: &str| { + format!( + "
1
\ +Kgs{places}\ +" + ) + }; + let plain = parse_lab_master_rows(&unit("3"), "Unit").unwrap(); + for split in ["", "3"] { + let got = parse_lab_master_rows(&unit(split), "Unit").unwrap(); + assert_eq!(got, plain, "master parser lost the CDATA in {split:?}"); + } + } + #[test] fn render_lab_master_collection_carries_the_exact_company_name() { let request = diff --git a/src-tauri/src/agent_lab_import.rs b/src-tauri/src/agent_lab_import.rs index 2557e5e6..168a97cc 100644 --- a/src-tauri/src/agent_lab_import.rs +++ b/src-tauri/src/agent_lab_import.rs @@ -1932,7 +1932,12 @@ fn parse_voucher_readback_nested(xml: &str) -> Result, Stri let mut voucher: Option> = None; let mut entry: Option> = None; let mut entries: Vec<(String, String, String)> = Vec::new(); - let mut current_tag = String::new(); + // Same discipline as the two parsers in `agent_lab.rs`, and for the same + // reason (bridge#379): a real Tally response is CRLF-indented and dense + // with self-closing elements, so tracking the last-opened tag lets one + // stale tag absorb a whole indentation run. Here that corrupted `AMOUNT` + // on a read-back entry, which is what the import mismatch report compares. + let mut buffer = LabTextBuffer::default(); const VOUCHER_PREFIX: [&str; 5] = ["ENVELOPE", "BODY", "DATA", "COLLECTION", "VOUCHER"]; const ENTRY_PREFIX: [&str; 6] = [ "ENVELOPE", @@ -1956,20 +1961,10 @@ fn parse_voucher_readback_nested(xml: &str) -> Result, Stri entry = Some(BTreeMap::new()); } path.push(tag.clone()); - current_tag = tag; + buffer.open(&tag); } Ok(quick_xml::events::Event::Text(text)) => { - let value = decoded_agent_text(text)?; - let parent = &path[..path.len().saturating_sub(1)]; - if parent == ENTRY_PREFIX { - if let Some(row) = entry.as_mut() { - append_agent_text(row, ¤t_tag, value); - } - } else if parent == VOUCHER_PREFIX { - if let Some(row) = voucher.as_mut() { - append_agent_text(row, ¤t_tag, value); - } - } + buffer.push(&decoded_agent_text(text)?); } // quick_xml delivers a general entity/character reference // (`&`, ``, ...) as its own `GeneralRef` event, separate @@ -1984,20 +1979,34 @@ fn parse_voucher_readback_nested(xml: &str) -> Result, Stri // agent_company_checkpoint.rs, source_draft_xml.rs) already // handles this event; the lab read-back path did not. Ok(quick_xml::events::Event::GeneralRef(reference)) => { - let value = decoded_agent_reference(reference)?; - let parent = &path[..path.len().saturating_sub(1)]; - if parent == ENTRY_PREFIX { - if let Some(row) = entry.as_mut() { - append_agent_text(row, ¤t_tag, value); - } - } else if parent == VOUCHER_PREFIX { - if let Some(row) = voucher.as_mut() { - append_agent_text(row, ¤t_tag, value); - } - } + buffer.push(&decoded_agent_reference(reference)?); } + // And the same CDATA gap: a scalar split across a CDATA section + // read back short, silently. All three event kinds feed one buffer. + Ok(quick_xml::events::Event::CData(text)) => { + buffer.push( + &text + .decode() + .map_err(|_| "agent_read_protocol_invalid".to_string())?, + ); + } + Ok(quick_xml::events::Event::Empty(_)) => buffer.abandon(), Ok(quick_xml::events::Event::End(event)) => { let end = String::from_utf8_lossy(event.name().as_ref()).to_ascii_uppercase(); + // `path` still holds the closing element, so its parent chain + // names the row the field belongs to. + if let Some((field, value)) = buffer.close(&end) { + let parent = &path[..path.len().saturating_sub(1)]; + if parent == ENTRY_PREFIX { + if let Some(row) = entry.as_mut() { + append_agent_text(row, &field, value); + } + } else if parent == VOUCHER_PREFIX { + if let Some(row) = voucher.as_mut() { + append_agent_text(row, &field, value); + } + } + } if end == "ALLLEDGERENTRIES.LIST" && path.as_slice() == ENTRY_PREFIX { if let Some(row) = entry.take() { entries.push(( diff --git a/src-tauri/src/agent_lab_import_tests.rs b/src-tauri/src/agent_lab_import_tests.rs index 25766aa4..fb38d56f 100644 --- a/src-tauri/src/agent_lab_import_tests.rs +++ b/src-tauri/src/agent_lab_import_tests.rs @@ -892,6 +892,76 @@ fn a_non_voucher_child_of_collection_is_refused() { ); } +#[test] +fn voucher_readback_survives_an_indented_response_and_cdata() { + // bridge#379 again, in the third parser of the same family: this one is + // what the import mismatch report compares, so an AMOUNT that grows an + // indentation tail reports a false mismatch against a target Tally has + // stored correctly. The nested allocation carries a `STATUS` element -- + // Tally's own name for a bank data field, and the bridge#378 shape -- so + // this also holds the two fixes together: the envelope must be accepted + // (bridge#389) and the nested field must not reach the entry above it. + // + // It also carries its own `AMOUNT`, as a real bank allocation does, and + // that is the part that makes this a control rather than a description. + // The stale tag misroutes by exactly one nesting level, so it only + // corrupts a value when the nested child's name COLLIDES with a field the + // entry itself reads. With `STATUS` alone the leak lands on a key nothing + // reads and every assertion below passes against the broken parser. Real gateway responses are CRLF-indented and dense + // with self-closing elements, and `Event::Empty` never disturbed the + // tag being accumulated into. + let entry = |amount: &str| { + format!( + "\r\n Bank Account\r\n \ +No\r\n {amount}\r\n \ +\r\n No\r\n {amount}\r\n \r\n \ +" + ) + }; + let xml = format!( + "\r\n
\r\n 1\r\n
\r\n \ +\r\n \r\n \r\n \r\n \ +20260405\r\n \r\n 61\r\n \ +Payment\r\n g-3\r\n \ +No\r\n {}\r\n {}\r\n \r\n \ +\r\n \r\n \r\n ", + entry("30000.00"), + entry("-30000.00"), + ); + let rows = parse_voucher_readback_nested(&xml).unwrap(); + assert_eq!(rows.len(), 1); + assert_eq!(rows[0].voucher_number.as_deref(), Some("61")); + assert_eq!(rows[0].ledger_entries.len(), 2); + // Exact values: no indentation tail on the amount, and the nested bank + // allocation's own STATUS does not reach the entry. + assert_eq!( + rows[0].ledger_entries[0], + ( + "Bank Account".to_string(), + "No".to_string(), + "30000.00".to_string() + ) + ); + assert_eq!(rows[0].ledger_entries[1].2, "-30000.00"); + for entry in &rows[0].ledger_entries { + assert!( + !entry.2.contains('\r'), + "amount carries an indentation run: {:?}", + entry.2 + ); + assert_eq!(entry.0.trim(), entry.0); + } + + // And a scalar split across CDATA rejoins rather than reading short. + let split = xml.replacen( + "30000.00", + "300.00", + 1, + ); + let rows = parse_voucher_readback_nested(&split).unwrap(); + assert_eq!(rows[0].ledger_entries[0].2, "30000.00"); +} + #[test] fn voucher_readback_decodes_entities_in_party_ledger_and_narration() { // 2026-09-14 coordinator finding: quick_xml delivers `&` as its own