From ecf4fa858a2442028b5a8c729caf721fb9dec566 Mon Sep 17 00:00:00 2001 From: Rick Wilson <113149377+Rick-Wilson@users.noreply.github.com> Date: Sat, 22 Aug 2026 06:38:32 -0700 Subject: [PATCH 1/4] feat(cli): annotate PBN trees in place, and only where analysis is missing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The binary could already write Bridge Composer compatible DD and par tags, but only one file at a time, always recomputing, and it fabricated results for boards that have no deal. A build could not simply point it at a collection. Now it can. Skip incomplete deals. A board written as [Deal "N:... ... ... ..."] — Bridge Composer's form for an auction-only teaching board — parses successfully into empty hands, so it was solved and stamped with an all-zero table plus [OptimumScore "NS 0"] and [ParContract "NS Pass"]. On one real lesson file that produced 19 annotated boards where 6 were analyzable. Hands::is_complete now gates the pass and such boards are left untouched. Fill in only what is missing. A board that already carries [DoubleDummyTricks] is passed through byte-for-byte; --recalculate redoes those. A stray par tag without a DD table does not count as analyzed, so it still gets filled in and the orphaned value replaced. Accept several inputs, and directories, searched recursively for *.pbn, with --in-place to rewrite them. Unchanged files are not rewritten, so a re-run touches nothing and mtimes do not churn; in-place writes go through a temporary file and a rename. Naming more than one file without --in-place is an error rather than a surprise. Verified against Bridge Composer's own output: across 25 files it had already analyzed, all 102 distinct DoubleDummyTricks values were reproduced exactly. Annotating a 63-file, 828-deal collection in one command added analysis to every deal, produced no all-zero tables, removed no lines, and the second run updated nothing. Co-Authored-By: Claude Opus 5 (1M context) --- Cargo.lock | 2 +- Cargo.toml | 2 +- README.md | 29 ++++ src/bin/bridge-solver/main.rs | 265 ++++++++++++++++++++++++++++++---- src/hands.rs | 11 ++ 5 files changed, 277 insertions(+), 32 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 381c93b..9fed93e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -63,7 +63,7 @@ dependencies = [ [[package]] name = "bridge-solver" -version = "0.4.0" +version = "0.5.0" dependencies = [ "bridge-types", "clap", diff --git a/Cargo.toml b/Cargo.toml index f07c902..8f8e68f 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "bridge-solver" -version = "0.4.0" +version = "0.5.0" edition = "2021" license = "GPL-2.0-only" description = "Bridge Double-Dummy Solver - Port of macroxue/bridge-solver" diff --git a/README.md b/README.md index b39d2c1..46f5a28 100644 --- a/README.md +++ b/README.md @@ -26,3 +26,32 @@ The following are original work with no upstream counterpart: - `wasm/` — WebAssembly bindings and LIN parsing - `web/` — the browser application - PBN input/output, and the `bin/bridge-solver` PBN-processing binary + +## Annotating PBN files + +The `bridge-solver` binary (build with `--features cli`) fills in double-dummy +and par analysis on PBN files, writing Bridge Composer compatible tags: +`DoubleDummyTricks`, `OptimumResultTable`, and — when the board states its +vulnerability — `OptimumScore` and `ParContract`. + +```sh +bridge-solver -i deals.pbn # analyze one file to stdout +bridge-solver -i deals.pbn -o out.pbn # ...or to another file +bridge-solver -w -i Curated/ # annotate a tree in place +``` + +Directories are searched recursively for `*.pbn`, so a build can annotate a +whole collection in one command with no scripting around it. The pass is +designed to be safe to run over source material: + +- **Only what is missing.** A board that already has a `DoubleDummyTricks` tag + is passed through byte-for-byte. Use `--recalculate` to redo those too. +- **Nothing else is touched.** Files are edited line by line rather than + reparsed and rewritten, so `%` directives (Bridge Composer's fonts, page + setup and colours), `;` comments, and hand-authored `{...}` commentary all + survive exactly as written. Annotating a collection only ever adds lines. +- **Incomplete deals are skipped.** Auction-only teaching boards, written as + `[Deal "N:... ... ... ..."]`, parse into empty hands; they are left alone + rather than stamped with a fabricated all-zero table. +- **Re-runs are no-ops.** Unchanged files are not rewritten, so mtimes do not + churn. In-place writes go through a temporary file and a rename. diff --git a/src/bin/bridge-solver/main.rs b/src/bin/bridge-solver/main.rs index 006a9cb..86abf7a 100644 --- a/src/bin/bridge-solver/main.rs +++ b/src/bin/bridge-solver/main.rs @@ -7,7 +7,12 @@ //! - ParContract (if vulnerability is known) //! - OptimumResultTable (full table) //! -//! Usage: bridge-solver --input --output +//! Boards whose deal is incomplete are passed through untouched. +//! +//! Usage: +//! bridge-solver -i -o # one file to another +//! bridge-solver -i # one file to stdout +//! bridge-solver -w -i ... # annotate in place, recursively use bridge_solver::{ par, CutoffCache, DdTricks, Hands, PatternCache, Solver, CLUB, DIAMOND, EAST, HEART, NORTH, @@ -16,20 +21,33 @@ use bridge_solver::{ use clap::Parser; use std::fs; use std::io::{self, Write}; +use std::path::{Path, PathBuf}; #[derive(Parser)] #[command(name = "bridge-solver")] #[command(about = "Double-dummy solver for PBN files")] #[command(version)] struct Args { - /// Input PBN file - #[arg(short = 'i', long = "input", required = true)] - input: String, + /// Input PBN file(s) or director(ies); directories are searched recursively + /// for *.pbn. Accepts several, which requires --in-place. + #[arg(short = 'i', long = "input", required = true, num_args = 1..)] + input: Vec, /// Output PBN file (if not specified, writes to stdout) - #[arg(short = 'o', long = "output")] + #[arg(short = 'o', long = "output", conflicts_with = "in_place")] output: Option, + /// Rewrite each input file in place. Files whose content is unchanged are + /// left alone, so a re-run touches nothing and build systems see no churn. + #[arg(short = 'w', long = "in-place")] + in_place: bool, + + /// Recompute analysis for boards that already carry it. By default a board + /// with a [DoubleDummyTricks] tag is left exactly as found, so annotating a + /// collection only fills in what is missing. + #[arg(long = "recalculate")] + recalculate: bool, + /// Verbose output - show progress #[arg(short = 'v', long = "verbose")] verbose: bool, @@ -79,37 +97,123 @@ impl DdResults { fn main() { let args = Args::parse(); - // Read input file - let content = match fs::read_to_string(&args.input) { - Ok(c) => c, + let files = match collect_inputs(&args.input) { + Ok(f) => f, Err(e) => { - eprintln!("Error reading input file '{}': {}", args.input, e); + eprintln!("Error: {e}"); std::process::exit(1); } }; + if files.is_empty() { + eprintln!("Error: no .pbn files found in the given input(s)"); + std::process::exit(1); + } + if files.len() > 1 && !args.in_place { + eprintln!( + "Error: {} input files matched; use --in-place to annotate them, \ + or name a single file with --output", + files.len() + ); + std::process::exit(1); + } - // Process the PBN content - let result = process_pbn(&content, args.verbose); + let mut changed = 0usize; + for path in &files { + let content = match fs::read_to_string(path) { + Ok(c) => c, + Err(e) => { + eprintln!("Error reading input file '{}': {}", path.display(), e); + std::process::exit(1); + } + }; + if args.verbose { + eprintln!("Processing {}...", path.display()); + } + let result = process_pbn(&content, args.verbose, args.recalculate); - // Write output - match args.output { - Some(path) => { - if let Err(e) = fs::write(&path, &result) { - eprintln!("Error writing output file '{}': {}", path, e); + if args.in_place { + // Unchanged files are left untouched so a re-run is a true no-op + // and does not churn mtimes in a build. + if result == content { + continue; + } + if let Err(e) = write_atomically(path, &result) { + eprintln!("Error writing '{}': {}", path.display(), e); std::process::exit(1); } - if args.verbose { - eprintln!("Output written to {}", path); + changed += 1; + } else { + match args.output { + Some(ref out) => { + if let Err(e) = fs::write(out, &result) { + eprintln!("Error writing output file '{out}': {e}"); + std::process::exit(1); + } + if args.verbose { + eprintln!("Output written to {out}"); + } + } + None => { + io::stdout().write_all(result.as_bytes()).unwrap(); + } } } - None => { - io::stdout().write_all(result.as_bytes()).unwrap(); + } + + if args.in_place && args.verbose { + eprintln!("{changed} of {} file(s) updated", files.len()); + } +} + +/// Expand the input arguments into a sorted, de-duplicated list of PBN files. +/// A directory contributes every `*.pbn` beneath it; a file is taken as given, +/// whatever its extension, so an oddly-named file can still be named directly. +fn collect_inputs(inputs: &[String]) -> io::Result> { + let mut files = Vec::new(); + for raw in inputs { + let path = PathBuf::from(raw); + if path.is_dir() { + collect_pbn_files(&path, &mut files)?; + } else if path.exists() { + files.push(path); + } else { + return Err(io::Error::new( + io::ErrorKind::NotFound, + format!("no such file or directory: {raw}"), + )); } } + files.sort(); + files.dedup(); + Ok(files) +} + +/// Recursively gather `*.pbn` under `dir`. +fn collect_pbn_files(dir: &Path, out: &mut Vec) -> io::Result<()> { + for entry in fs::read_dir(dir)? { + let path = entry?.path(); + if path.is_dir() { + collect_pbn_files(&path, out)?; + } else if path + .extension() + .is_some_and(|e| e.eq_ignore_ascii_case("pbn")) + { + out.push(path); + } + } + Ok(()) +} + +/// Write via a sibling temporary file and rename, so an interrupted run cannot +/// leave a half-written lesson file behind. +fn write_atomically(path: &Path, contents: &str) -> io::Result<()> { + let tmp = path.with_extension("pbn.tmp"); + fs::write(&tmp, contents)?; + fs::rename(&tmp, path) } /// Process a PBN file: find deals, solve them, insert/replace DD tags -fn process_pbn(content: &str, verbose: bool) -> String { +fn process_pbn(content: &str, verbose: bool, recalculate: bool) -> String { // Split into deal blocks (separated by blank lines outside of brace comments) let mut result = String::new(); let mut deal_count = 0; @@ -159,7 +263,7 @@ fn process_pbn(content: &str, verbose: bool) -> String { // Process this block let block_lines = &lines[block_start..block_end]; - let processed = process_deal_block(block_lines, &mut deal_count, verbose); + let processed = process_deal_block(block_lines, &mut deal_count, verbose, recalculate); result.push_str(&processed); } @@ -171,7 +275,12 @@ fn process_pbn(content: &str, verbose: bool) -> String { } /// Process a single deal block -fn process_deal_block(lines: &[&str], deal_count: &mut usize, verbose: bool) -> String { +fn process_deal_block( + lines: &[&str], + deal_count: &mut usize, + verbose: bool, + recalculate: bool, +) -> String { // Find the Deal tag to extract hands let mut deal_str: Option<&str> = None; let mut vulnerability: Option = None; @@ -199,16 +308,42 @@ fn process_deal_block(lines: &[&str], deal_count: &mut usize, verbose: bool) -> return out; }; - // Parse the deal - let Some(hands) = Hands::from_pbn(deal_str) else { - // Can't parse, pass through unchanged + // Parse the deal. A board with no cards — BridgeComposer writes + // [Deal "N:... ... ... ..."] for auction-only teaching boards — parses fine + // into empty hands, so completeness is checked too. Annotating one of those + // would stamp a fabricated all-zero table and a "Pass" par onto a board that + // has no deal to analyze. + let unchanged = || { let mut out = String::new(); for line in lines { out.push_str(line); out.push('\n'); } - return out; + out }; + // Already analyzed? Leave it alone unless asked to redo the work. The + // [DoubleDummyTricks] tag is the marker: a board carrying a stray par tag + // but no DD table has not been analyzed, and still gets filled in. + if !recalculate + && lines + .iter() + .any(|l| extract_tag_name(l) == Some("DoubleDummyTricks")) + { + if verbose { + eprintln!("Skipping board that already has analysis"); + } + return unchanged(); + } + + let Some(hands) = Hands::from_pbn(deal_str) else { + return unchanged(); + }; + if !hands.is_complete() { + if verbose { + eprintln!("Skipping board with an incomplete deal"); + } + return unchanged(); + } *deal_count += 1; if verbose { @@ -573,7 +708,7 @@ mod tests { [Deal "N:AKQT3.J6.KJ42.95 652.AK42.AQ87.T4 J74.QT95.T.AK863 98.873.9653.QJ72"] [Dealer "N"] "#; - let result = process_pbn(pbn, false); + let result = process_pbn(pbn, false, false); assert!(result.contains("[DoubleDummyTricks")); assert!(result.contains("[OptimumResultTable")); assert!(result.contains("N NT")); @@ -610,7 +745,7 @@ W D 0 W C 0 [Dealer "N"] "#; - let result = process_pbn(pbn, false); + let result = process_pbn(pbn, false, true); // Each tag we generate must appear exactly once: the stale copy is // stripped and replaced, not duplicated. `Vulnerable` is present, so // the par tags are generated too. @@ -625,6 +760,76 @@ W C 0 assert!(result.contains(r#"[ParContract "NS 4S="]"#)); } + /// The default is to fill in only what is missing: a board that already + /// carries a DD table is passed through byte-for-byte, however stale its + /// values, so a builder can point the tool at a whole collection safely. + #[test] + fn test_existing_analysis_is_kept_unless_recalculating() { + let pbn = r#"[Event "Test"] +[Vulnerable "None"] +[Deal "N:AKQT3.J6.KJ42.95 652.AK42.AQ87.T4 J74.QT95.T.AK863 98.873.9653.QJ72"] +[DoubleDummyTricks "00000000000000000000"] +[Dealer "N"] +"#; + let kept = process_pbn(pbn, false, false); + assert_eq!(kept, pbn, "default must not touch an analyzed board"); + + let redone = process_pbn(pbn, false, true); + assert!(redone.contains(r#"[DoubleDummyTricks "9a8789a8784346543465"]"#)); + } + + /// A board holding a par tag but no DD table has not been analyzed, so the + /// default still fills it in — and replaces the orphaned par value. + #[test] + fn test_par_tag_alone_does_not_count_as_analyzed() { + let pbn = r#"[Event "Test"] +[Vulnerable "None"] +[Deal "N:AKQT3.J6.KJ42.95 652.AK42.AQ87.T4 J74.QT95.T.AK863 98.873.9653.QJ72"] +[OptimumScore "NS 9999"] +[Dealer "N"] +"#; + let result = process_pbn(pbn, false, false); + assert_eq!(result.matches("[DoubleDummyTricks").count(), 1); + assert!(!result.contains("NS 9999")); + } + + /// A board with no cards must be left exactly as found. BridgeComposer + /// writes `[Deal "N:... ... ... ..."]` for auction-only teaching boards; + /// those parse into empty hands, and annotating one would stamp a + /// fabricated all-zero table and a "Pass" par onto a board with no deal. + #[test] + fn test_placeholder_deals_pass_through_untouched() { + let pbn = r#"[Event "Test"] +[Board "1"] +[Vulnerable "None"] +[Deal "N:... ... ... ..."] +[Auction "N"] +1S Pass 2S AP +"#; + let result = process_pbn(pbn, false, false); + assert_eq!(result, pbn, "placeholder board must be byte-identical"); + assert!(!result.contains("DoubleDummyTricks")); + assert!(!result.contains("OptimumScore")); + } + + /// A file mixing real and placeholder boards annotates only the real ones. + #[test] + fn test_annotates_only_complete_deals_in_mixed_file() { + let pbn = r#"[Event "Test"] +[Board "1"] +[Vulnerable "None"] +[Deal "N:... ... ... ..."] + +[Event "Test"] +[Board "2"] +[Vulnerable "None"] +[Deal "N:AKQT3.J6.KJ42.95 652.AK42.AQ87.T4 J74.QT95.T.AK863 98.873.9653.QJ72"] +"#; + let result = process_pbn(pbn, false, false); + assert_eq!(result.matches("[DoubleDummyTricks").count(), 1); + assert!(!result.contains("\"00000000000000000000\"")); + } + /// Without a `Vulnerable` tag par cannot be scored, so the par tags are /// omitted — and a stale copy in the input is still stripped. #[test] @@ -635,7 +840,7 @@ W C 0 [ParContract "NS Pass"] [Dealer "N"] "#; - let result = process_pbn(pbn, false); + let result = process_pbn(pbn, false, false); assert_eq!(result.matches("[DoubleDummyTricks").count(), 1); assert_eq!(result.matches("[OptimumScore").count(), 0); assert_eq!(result.matches("[ParContract").count(), 0); diff --git a/src/hands.rs b/src/hands.rs index 82a4356..f621b08 100644 --- a/src/hands.rs +++ b/src/hands.rs @@ -59,6 +59,17 @@ impl Hands { self.hands[WEST].size() } + /// True only when all four seats hold a full 13-card hand. + /// + /// PBN files legitimately carry placeholder deals — BridgeComposer writes + /// `[Deal "N:... ... ... ..."]` for auction-only teaching boards — and those + /// parse successfully into empty hands. Solving one yields an all-zero + /// table, so callers that annotate files must skip incomplete deals rather + /// than record a fabricated result. + pub fn is_complete(&self) -> bool { + self.hands.iter().all(|h| h.size() == 13) + } + /// Parse from PBN-style deal string /// Format: "N:AKQT3.J6.KJ42.95 652.AK42.AQ87.T4 J74.QT95.T.AK863 98.873.9653.QJ72" /// Order after first seat: rotates clockwise (N E S W or W N E S, etc.) From a6a30416cdb51715d0d23145686c53f5b91b13ea Mon Sep 17 00:00:00 2001 From: Rick Wilson <113149377+Rick-Wilson@users.noreply.github.com> Date: Sat, 22 Aug 2026 06:46:40 -0700 Subject: [PATCH 2/4] fix(lint): allow the manual isolate-lowest-bit idiom in pattern.rs A newer stable clippy added `manual_isolate_lowest_one`, which flags `m & m.wrapping_neg()` in pack_bits/unpack_bits and turns CI's Lint job red on main. Its suggested `isolate_lowest_one()` is still unstable, so adopting it would raise this crate's minimum Rust version for no gain in a hot path. Suppress the lint locally instead, paired with `unknown_lints` so compilers predating it stay quiet about the allow itself. Co-Authored-By: Claude Opus 5 (1M context) --- src/pattern.rs | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/src/pattern.rs b/src/pattern.rs index 6b41071..6b63f11 100644 --- a/src/pattern.rs +++ b/src/pattern.rs @@ -14,6 +14,12 @@ use super::types::*; /// Pack bits: extract bits from source where mask has 1s, compress them to low bits /// Example: PackBits(0b10100, 0b11100) = 0b101 (extracts bits 2,3,4 and packs to 0,1,2) #[inline] +// `m & m.wrapping_neg()` is the classic isolate-lowest-set-bit idiom. Clippy +// (since the toolchain that stabilized it) wants `m.isolate_lowest_one()`, but +// that method is still unstable on older compilers, so adopting it would raise +// this crate's minimum Rust version for no benefit in a hot path. `unknown_lints` +// keeps compilers that predate the lint quiet about the allow itself. +#[allow(unknown_lints, clippy::manual_isolate_lowest_one)] pub fn pack_bits(source: u64, mask: u64) -> u64 { #[cfg(target_feature = "bmi2")] { @@ -43,6 +49,12 @@ pub fn pack_bits(source: u64, mask: u64) -> u64 { /// Unpack bits: scatter source bits to positions where mask has 1s /// Example: UnpackBits(0b101, 0b11100) = 0b10100 (scatters bits 0,1,2 to positions 2,3,4) #[inline] +// `m & m.wrapping_neg()` is the classic isolate-lowest-set-bit idiom. Clippy +// (since the toolchain that stabilized it) wants `m.isolate_lowest_one()`, but +// that method is still unstable on older compilers, so adopting it would raise +// this crate's minimum Rust version for no benefit in a hot path. `unknown_lints` +// keeps compilers that predate the lint quiet about the allow itself. +#[allow(unknown_lints, clippy::manual_isolate_lowest_one)] pub fn unpack_bits(source: u64, mask: u64) -> u64 { #[cfg(target_feature = "bmi2")] { From 25d1bffaaf3dcb391c941a2546b43b2a1d1f7347 Mon Sep 17 00:00:00 2001 From: Rick Wilson <113149377+Rick-Wilson@users.noreply.github.com> Date: Sat, 22 Aug 2026 06:46:40 -0700 Subject: [PATCH 3/4] feat(cli): --mark-verified to record DD provenance in BCFlags Sets bit 0x00080000 ("double-dummy data has been verified") in each annotated board's [BCFlags], adding the tag if the board has none and preserving every bit already present. This records provenance only. No documented BCFlags bit controls whether the DD table is displayed, and Bridge Composer does not set this one itself: across 388 boards in files where it had written DoubleDummyTricks, the bit was set zero times, and the flag values there (1f, 17, df, 40001f) are indistinguishable from files with no analysis at all. The flag is therefore off by default and documented for what it does rather than what it might be assumed to do. Co-Authored-By: Claude Opus 5 (1M context) --- README.md | 5 ++ src/bin/bridge-solver/main.rs | 108 ++++++++++++++++++++++++++++++---- 2 files changed, 102 insertions(+), 11 deletions(-) diff --git a/README.md b/README.md index 46f5a28..360712d 100644 --- a/README.md +++ b/README.md @@ -53,5 +53,10 @@ designed to be safe to run over source material: - **Incomplete deals are skipped.** Auction-only teaching boards, written as `[Deal "N:... ... ... ..."]`, parse into empty hands; they are left alone rather than stamped with a fabricated all-zero table. +- **`--mark-verified`** sets bit `0x00080000` ("double-dummy data has been + verified") in each annotated board's `[BCFlags]`, adding the tag if absent and + preserving every bit already there. This records provenance only — no + documented BCFlags bit controls whether the DD table is *displayed*, and + Bridge Composer does not set this bit itself when it computes a table. - **Re-runs are no-ops.** Unchanged files are not rewritten, so mtimes do not churn. In-place writes go through a temporary file and a rename. diff --git a/src/bin/bridge-solver/main.rs b/src/bin/bridge-solver/main.rs index 86abf7a..919bdc6 100644 --- a/src/bin/bridge-solver/main.rs +++ b/src/bin/bridge-solver/main.rs @@ -42,6 +42,12 @@ struct Args { #[arg(short = 'w', long = "in-place")] in_place: bool, + /// Set the "double-dummy data has been verified" bit (0x00080000) in each + /// annotated board's [BCFlags], adding the tag if absent. Note this marks + /// provenance; it does not make Bridge Composer display the DD table. + #[arg(long = "mark-verified")] + mark_verified: bool, + /// Recompute analysis for boards that already carry it. By default a board /// with a [DoubleDummyTricks] tag is left exactly as found, so annotating a /// collection only fills in what is missing. @@ -129,7 +135,7 @@ fn main() { if args.verbose { eprintln!("Processing {}...", path.display()); } - let result = process_pbn(&content, args.verbose, args.recalculate); + let result = process_pbn(&content, args.verbose, args.recalculate, args.mark_verified); if args.in_place { // Unchanged files are left untouched so a re-run is a true no-op @@ -212,8 +218,26 @@ fn write_atomically(path: &Path, contents: &str) -> io::Result<()> { fs::rename(&tmp, path) } +/// Bridge Composer's [BCFlags] bit meaning "double-dummy data has been +/// verified". Note this records provenance only: no documented BCFlags bit +/// controls whether the DD table is *displayed*, and Bridge Composer does not +/// set this one itself when it computes a table. +const BC_FLAG_DD_VERIFIED: u64 = 0x0008_0000; + +/// Return a `[BCFlags]` line with the verified bit set, preserving every other +/// bit. An unparsable value is replaced rather than propagated, since the tag +/// is meaningless if it is not hex. +fn with_verified_bit(line: &str) -> String { + let current = line + .split('"') + .nth(1) + .and_then(|v| u64::from_str_radix(v.trim(), 16).ok()) + .unwrap_or(0); + format!("[BCFlags \"{:x}\"]", current | BC_FLAG_DD_VERIFIED) +} + /// Process a PBN file: find deals, solve them, insert/replace DD tags -fn process_pbn(content: &str, verbose: bool, recalculate: bool) -> String { +fn process_pbn(content: &str, verbose: bool, recalculate: bool, mark_verified: bool) -> String { // Split into deal blocks (separated by blank lines outside of brace comments) let mut result = String::new(); let mut deal_count = 0; @@ -263,7 +287,13 @@ fn process_pbn(content: &str, verbose: bool, recalculate: bool) -> String { // Process this block let block_lines = &lines[block_start..block_end]; - let processed = process_deal_block(block_lines, &mut deal_count, verbose, recalculate); + let processed = process_deal_block( + block_lines, + &mut deal_count, + verbose, + recalculate, + mark_verified, + ); result.push_str(&processed); } @@ -280,6 +310,7 @@ fn process_deal_block( deal_count: &mut usize, verbose: bool, recalculate: bool, + mark_verified: bool, ) -> String { // Find the Deal tag to extract hands let mut deal_str: Option<&str> = None; @@ -361,6 +392,7 @@ fn process_deal_block( // 2. Insert our new DD tags in the right place let mut output_lines: Vec = Vec::new(); + let mut saw_bcflags = false; let mut found_dd_tag = false; let mut skipping_optimum_data = false; let mut insertion_point: Option = None; @@ -376,6 +408,14 @@ fn process_deal_block( for line in lines { let trimmed = line.trim(); + // Fold the verified bit into an existing [BCFlags], keeping every other + // bit the board already carried. + if mark_verified && extract_tag_name(trimmed) == Some("BCFlags") { + saw_bcflags = true; + output_lines.push(with_verified_bit(trimmed)); + continue; + } + // Check if this is one of our DD tags if let Some(tag_name) = extract_tag_name(trimmed) { if dd_tag_names.contains(&tag_name) { @@ -425,6 +465,13 @@ fn process_deal_block( // Build the output let mut result = String::new(); + // A board with no [BCFlags] of its own still needs one to carry the bit. + let dd_tags = if mark_verified && !saw_bcflags { + format!("[BCFlags \"{:x}\"]\n{dd_tags}", BC_FLAG_DD_VERIFIED) + } else { + dd_tags + }; + let insert_at = insertion_point.unwrap_or(output_lines.len()); for (idx, line) in output_lines.iter().enumerate() { @@ -708,7 +755,7 @@ mod tests { [Deal "N:AKQT3.J6.KJ42.95 652.AK42.AQ87.T4 J74.QT95.T.AK863 98.873.9653.QJ72"] [Dealer "N"] "#; - let result = process_pbn(pbn, false, false); + let result = process_pbn(pbn, false, false, false); assert!(result.contains("[DoubleDummyTricks")); assert!(result.contains("[OptimumResultTable")); assert!(result.contains("N NT")); @@ -745,7 +792,7 @@ W D 0 W C 0 [Dealer "N"] "#; - let result = process_pbn(pbn, false, true); + let result = process_pbn(pbn, false, true, false); // Each tag we generate must appear exactly once: the stale copy is // stripped and replaced, not duplicated. `Vulnerable` is present, so // the par tags are generated too. @@ -771,10 +818,10 @@ W C 0 [DoubleDummyTricks "00000000000000000000"] [Dealer "N"] "#; - let kept = process_pbn(pbn, false, false); + let kept = process_pbn(pbn, false, false, false); assert_eq!(kept, pbn, "default must not touch an analyzed board"); - let redone = process_pbn(pbn, false, true); + let redone = process_pbn(pbn, false, true, false); assert!(redone.contains(r#"[DoubleDummyTricks "9a8789a8784346543465"]"#)); } @@ -788,11 +835,50 @@ W C 0 [OptimumScore "NS 9999"] [Dealer "N"] "#; - let result = process_pbn(pbn, false, false); + let result = process_pbn(pbn, false, false, false); assert_eq!(result.matches("[DoubleDummyTricks").count(), 1); assert!(!result.contains("NS 9999")); } + /// --mark-verified folds bit 0x00080000 into the board's existing BCFlags + /// without disturbing the bits it already carried. + #[test] + fn test_mark_verified_preserves_other_bcflags_bits() { + let pbn = r#"[Event "Test"] +[Vulnerable "None"] +[Deal "N:AKQT3.J6.KJ42.95 652.AK42.AQ87.T4 J74.QT95.T.AK863 98.873.9653.QJ72"] +[BCFlags "40001f"] +"#; + let result = process_pbn(pbn, false, false, true); + // 0x40001f | 0x80000 == 0x48001f — every original bit survives. + assert!( + result.contains(r#"[BCFlags "48001f"]"#), + "got: +{result}" + ); + assert_eq!(result.matches("[BCFlags").count(), 1); + + // Without the flag the tag is left exactly as written. + let plain = process_pbn(pbn, false, false, false); + assert!(plain.contains(r#"[BCFlags "40001f"]"#)); + } + + /// A board with no BCFlags of its own gets one carrying just that bit. + #[test] + fn test_mark_verified_adds_bcflags_when_absent() { + let pbn = r#"[Event "Test"] +[Vulnerable "None"] +[Deal "N:AKQT3.J6.KJ42.95 652.AK42.AQ87.T4 J74.QT95.T.AK863 98.873.9653.QJ72"] +"#; + let result = process_pbn(pbn, false, false, true); + assert!( + result.contains(r#"[BCFlags "80000"]"#), + "got: +{result}" + ); + assert_eq!(result.matches("[BCFlags").count(), 1); + } + /// A board with no cards must be left exactly as found. BridgeComposer /// writes `[Deal "N:... ... ... ..."]` for auction-only teaching boards; /// those parse into empty hands, and annotating one would stamp a @@ -806,7 +892,7 @@ W C 0 [Auction "N"] 1S Pass 2S AP "#; - let result = process_pbn(pbn, false, false); + let result = process_pbn(pbn, false, false, false); assert_eq!(result, pbn, "placeholder board must be byte-identical"); assert!(!result.contains("DoubleDummyTricks")); assert!(!result.contains("OptimumScore")); @@ -825,7 +911,7 @@ W C 0 [Vulnerable "None"] [Deal "N:AKQT3.J6.KJ42.95 652.AK42.AQ87.T4 J74.QT95.T.AK863 98.873.9653.QJ72"] "#; - let result = process_pbn(pbn, false, false); + let result = process_pbn(pbn, false, false, false); assert_eq!(result.matches("[DoubleDummyTricks").count(), 1); assert!(!result.contains("\"00000000000000000000\"")); } @@ -840,7 +926,7 @@ W C 0 [ParContract "NS Pass"] [Dealer "N"] "#; - let result = process_pbn(pbn, false, false); + let result = process_pbn(pbn, false, false, false); assert_eq!(result.matches("[DoubleDummyTricks").count(), 1); assert_eq!(result.matches("[OptimumScore").count(), 0); assert_eq!(result.matches("[ParContract").count(), 0); From 965447099ffa7566a52c0d54b39c409b244cdb4c Mon Sep 17 00:00:00 2001 From: Rick Wilson <113149377+Rick-Wilson@users.noreply.github.com> Date: Sat, 22 Aug 2026 06:50:04 -0700 Subject: [PATCH 4/4] fix(build): declare time_optimal_line's required features The example uses `analyse_play`, which is gated behind `play-analysis`, but unlike bench_fixtures beside it, it had no [[example]] block. Auto-discovered with no required-features, it failed to compile under CI's featureless `clippy --workspace --all-targets` pass and turned Lint red on main. Declare it the same way its sibling is declared, so the pass skips it. Co-Authored-By: Claude Opus 5 (1M context) --- Cargo.toml | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/Cargo.toml b/Cargo.toml index 8f8e68f..b98feaf 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -43,6 +43,11 @@ name = "bench_fixtures" path = "examples/bench_fixtures.rs" required-features = ["play-analysis"] +[[example]] +name = "time_optimal_line" +path = "examples/time_optimal_line.rs" +required-features = ["play-analysis"] + [dependencies] bridge-types = { git = "https://github.com/bridge-craftwork/bridge-types" } clap = { version = "4", features = ["derive"], optional = true }