From a705977f2e9aed4d8faa9e896f44d5ebaed51e39 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 18 Aug 2026 06:07:01 +0000 Subject: [PATCH] plain arm: the config-consuming extractor and the promotion gate it enables Increment 3 of the self-adaptive drill loop, closing the boundary drill.rs named twice now ("this does NOT build a promotion gate"). One rule wired end-to-end, not all four residual reasons at once - a real config surface plus a real gate is worth more than a wide, partially-wired one. PlainDrillConfig::unwrap_optional_annotation resolves `T | None` / `None | T` to T's own field_type reading. Chosen because it was the only cross-corpus candidate from #97's measurement (dismech 96, ruff/scripts 12, genuinely absent - not merely below threshold - on the A2UI SDK), unlike the LinkML call:PermissibleValue/call:EnumDefinition factories, which are corpus-scoped and would need a per-corpus row, not a generic one. field_type_from_annotation recurses into the non-None operand; a chained union (`str | int | None`) has a BinOp as its left operand, so the shape check fails and it stays unresolved rather than guessing which arm to keep. Off by default - PlainDrillConfig::default() is pinned byte-identical to the config-free path. drill::ratify_optional_unwrap is the gate: runs extraction with the rule OFF and ON over one source, then independently verifies the claim two ways that share no code with the resolver - 1. a SEPARATELY-WRITTEN shape test (is_shaped_t_or_none), applied by re-parsing the source and re-walking the raw AST directly - not by trusting field_type_from_annotation's own success/failure - counting how many of the newly-resolved sites are genuinely T|None shaped. Must equal the resolver's own count exactly, in EITHER direction: the resolver claiming to fix something the independent check disputes is exactly as much a defect as missing one it should have caught. 2. every resolved value is checked against a real-type-name shape (non-empty, alphanumeric) rather than trusted as "present, so fine". Measured, real corpora, per-file (`plain_ratify` example): dismech (84 files): baseline=96 resolved=91 verified=91 ratified=YES ruff/scripts (23 files): baseline=12 resolved=12 verified=12 ratified=YES a2ui sdk (127 files): baseline=0 resolved=0 verified=0 ratified=YES dismech's 96-91=5 gap is the honest remainder: non-optional binop shapes (chained unions, `int | str`) the rule correctly declines rather than guesses on - the ratification proves the claim is EXACT, not that the rule resolves everything. Falsifier discipline, including a finding worth recording rather than discarding: the first disable run on the independent verifier (is_shaped_t_or_none forced to always return true) reported the target test green under mutation - genuinely uninformative, not a false pass, because the independent-verification loop only visits sites the resolver itself flipped, and a correct resolver structurally can never disagree with a correct independent check without a SECOND bug existing first. Traced to the right level: RatificationReport::ratified()'s own disagreement-detection logic now has direct unit coverage (ratified_is_false_when_the_counts_disagree_in_either_direction, four hand-built cases), disable-run verified on THAT function directly (forced to `true`, the new test failed; restored, green). Two more disable runs on the resolver itself (optional_operand neutered; the config gate bypassed) both verified red-then-green normally. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01AGVLyRZNEKKBSfBDJfbY3V --- .../ruff_python_spo/examples/plain_ratify.rs | 73 ++++ crates/ruff_python_spo/src/drill.rs | 316 +++++++++++++++++- crates/ruff_python_spo/src/lib.rs | 10 +- crates/ruff_python_spo/src/plain.rs | 167 ++++++++- 4 files changed, 542 insertions(+), 24 deletions(-) create mode 100644 crates/ruff_python_spo/examples/plain_ratify.rs diff --git a/crates/ruff_python_spo/examples/plain_ratify.rs b/crates/ruff_python_spo/examples/plain_ratify.rs new file mode 100644 index 00000000000000..cfc13170867809 --- /dev/null +++ b/crates/ruff_python_spo/examples/plain_ratify.rs @@ -0,0 +1,73 @@ +//! Dev probe: run `ratify_optional_unwrap` over every `*.py` file under a +//! source tree, aggregate the report, and print it — the promotion gate +//! for the ONE drill rule wired end-to-end +//! ([`ruff_python_spo::PlainDrillConfig::unwrap_optional_annotation`]). +//! Prints every file whose ratification fails, so a non-ratifying corpus +//! is visible per-file, not just as an aggregate number. +//! +//! Usage: `cargo run -p ruff_python_spo --example plain_ratify -- ` + +#![expect( + clippy::print_stdout, + reason = "a dev probe's stdout report IS its deliverable" +)] + +use std::fs; +use std::path::Path; + +use ruff_python_spo::ratify_optional_unwrap; + +fn collect_py_files(dir: &Path, out: &mut Vec) { + let Ok(entries) = fs::read_dir(dir) else { + return; + }; + for entry in entries.flatten() { + let path = entry.path(); + if path.is_dir() { + collect_py_files(&path, out); + } else if path.extension().is_some_and(|e| e == "py") { + out.push(path); + } + } +} + +fn main() { + let root = std::env::args().nth(1).expect("usage: plain_ratify "); + let root = Path::new(&root); + + let mut files = Vec::new(); + collect_py_files(root, &mut files); + + let mut baseline_unresolved = 0usize; + let mut newly_resolved = 0usize; + let mut independently_verified = 0usize; + let mut failing_files = 0usize; + + for path in &files { + let Ok(src) = fs::read_to_string(path) else { + continue; + }; + let module = path.to_string_lossy(); + let report = ratify_optional_unwrap(&src, &module); + baseline_unresolved += report.baseline_unresolved; + newly_resolved += report.newly_resolved_sites; + independently_verified += report.independently_shape_verified; + if report.baseline_unresolved > 0 && !report.ratified() { + failing_files += 1; + println!("NOT RATIFIED: {} -> {report:?}", path.display()); + } + } + + println!("files scanned: {}", files.len()); + println!( + "baseline_unresolved={baseline_unresolved} newly_resolved={newly_resolved} independently_verified={independently_verified}" + ); + println!( + "still_unresolved_after_rule={} (non-optional binop shapes: chained unions, `int | str`, …)", + baseline_unresolved - newly_resolved + ); + println!( + "ratified: {}", + if failing_files == 0 { "YES" } else { "NO" } + ); +} diff --git a/crates/ruff_python_spo/src/drill.rs b/crates/ruff_python_spo/src/drill.rs index 6104708f3f7f49..3c17b27aba237e 100644 --- a/crates/ruff_python_spo/src/drill.rs +++ b/crates/ruff_python_spo/src/drill.rs @@ -1,24 +1,30 @@ -//! The self-adaptive drill's proposer — turns a concentrated residual -//! ledger ([`crate::plain::PlainResidual`]) into candidate config rows. +//! The self-adaptive drill's proposer AND its ratification gate — turns a +//! concentrated residual ledger ([`crate::plain::PlainResidual`]) into +//! candidate config rows, then (for the one rule wired end-to-end so far, +//! [`crate::plain::PlainDrillConfig::unwrap_optional_annotation`]) +//! independently verifies a candidate's exact-count coverage-delta claim +//! against a real corpus. //! //! # Scope boundary (read before extending this module) //! -//! This module builds the **grouping and cross-corpus classification** -//! stage only. It does NOT build a promotion gate that re-runs extraction -//! with a candidate row "active" and checks the coverage delta is exactly -//! the row's support — that check needs a config-consuming extractor -//! (a trie the plain arm reads before deciding a site's classification), -//! which does not exist yet. Building that engine is the next increment; -//! claiming this module ratifies rows would overstate what it measures. -//! -//! What this module DOES do, honestly: group residual rows by +//! [`propose`]/[`classify_across_corpora`] build the **grouping and +//! cross-corpus classification** stage: group residual rows by //! `(reason, detail)`, threshold on support, and — when residuals from //! more than one corpus are supplied — classify each candidate as //! [`RowScope::Generic`] (fires in every corpus measured) or -//! [`RowScope::CorpusScoped`] (fires in some but not all). That -//! generic/scoped split is itself a measured, falsifiable fact: a row is -//! `Generic` only because it was observed above `min_support` in EVERY -//! supplied corpus, not because of a similarity heuristic. +//! [`RowScope::CorpusScoped`] (fires in some but not all). That split is +//! itself a measured, falsifiable fact: a row is `Generic` only because +//! it was observed above `min_support` in EVERY supplied corpus, not +//! because of a similarity heuristic. +//! +//! [`ratify_optional_unwrap`] is the promotion gate the previous revision +//! of this doc said did not exist yet. It closes ONE candidate — not a +//! generic "activate any `CandidateRow`" mechanism, because only one rule +//! ([`crate::plain::PlainDrillConfig::unwrap_optional_annotation`]) is +//! wired into the extractor to activate. Extending the gate to the other +//! three drillable reasons needs each its own extractor-side rule first +//! (same shape as this one), which is future work, not silently implied +//! by this function's existence. //! //! A candidate row is data-shaped by design (`reason` + `detail` string + //! per-corpus support) so a downstream session can serialise it straight @@ -26,6 +32,8 @@ use std::collections::BTreeMap; +use ruff_python_ast::{Expr, Operator}; + use crate::plain::{PlainResidual, PlainResidualReason}; /// Reasons whose `detail` field is dense enough to drill on. The other @@ -160,6 +168,158 @@ pub fn classify_across_corpora( .collect() } +/// The result of running [`ratify_optional_unwrap`] against one corpus: +/// what the rule claims to have resolved, and independent verification +/// that the claim is exactly true — never "the counts happened to match, +/// so assume it worked." +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub struct RatificationReport { + /// `UnresolvedAnnotation`/`binop` residuals with the rule OFF. + pub baseline_unresolved: usize, + /// The same count with the rule ON. + pub with_rule_unresolved: usize, + /// `baseline_unresolved - with_rule_unresolved` — sites the rule + /// newly resolved. Named separately from the raw counts so a caller + /// doesn't have to re-derive the claim being verified. + pub newly_resolved_sites: usize, + /// Independently counted, from the SAME baseline-unresolved sites, + /// how many are genuinely `T | None`/`None | T` shaped — computed by + /// re-walking the source and testing shape directly, not by trusting + /// the resolver's own success/failure. Must equal + /// `newly_resolved_sites` for the claim to ratify; a mismatch in + /// EITHER direction (the rule resolved something it shouldn't have, + /// or missed something it should have caught) is a defect, not noise + /// to average away. + pub independently_shape_verified: usize, + /// For every newly-resolved site, whether its `field_type` equals the + /// inner type's OWN [`crate::plain::extract_plain_from_source`] + /// reading (config OFF) — i.e. the rule reduces to "read the inner + /// type," never a different value. `false` if any site's value + /// diverges. + pub field_type_matches_inner_reading: bool, +} + +impl RatificationReport { + /// The rule's claim survives: every newly-resolved site was + /// independently shape-verified (no more, no less) AND every + /// resolved value matches the inner type's own reading. + #[must_use] + pub fn ratified(&self) -> bool { + self.newly_resolved_sites == self.independently_shape_verified + && self.field_type_matches_inner_reading + } +} + +/// Independently re-derive, from a raw annotation expression, whether it +/// is exactly `T | None` / `None | T` shaped. Deliberately does NOT call +/// [`crate::plain`]'s private `optional_operand` — the whole point of an +/// independent check is that a bug shared between resolution and +/// verification would pass both; this is a second, separately-written +/// implementation of the same shape test. +fn is_shaped_t_or_none(annotation: &Expr) -> bool { + let Expr::BinOp(binop) = annotation else { + return false; + }; + binop.op == Operator::BitOr + && (matches!(&*binop.left, Expr::NoneLiteral(_)) + || matches!(&*binop.right, Expr::NoneLiteral(_))) +} + +/// Ratify (or refute) the `unwrap_optional_annotation` rule against one +/// source file, by the exact-count discipline the drill loop was +/// designed around — see [`RatificationReport`] for what each field +/// independently checks and why neither check trusts the resolver's own +/// success/failure as its own proof. +#[must_use] +pub fn ratify_optional_unwrap(source: &str, module: &str) -> RatificationReport { + use ruff_python_ast::{Expr, Stmt}; + use ruff_python_parser::parse_module; + + use crate::plain::{ + PlainDrillConfig, PlainResidualReason, extract_plain_from_source_with_config, + }; + + let off = PlainDrillConfig::default(); + let on = PlainDrillConfig { + unwrap_optional_annotation: true, + }; + + let (graph_off, residuals_off) = extract_plain_from_source_with_config(source, module, off); + let (graph_on, residuals_on) = extract_plain_from_source_with_config(source, module, on); + + let baseline_unresolved = residuals_off + .iter() + .filter(|r| r.reason == PlainResidualReason::UnresolvedAnnotation) + .count(); + let with_rule_unresolved = residuals_on + .iter() + .filter(|r| r.reason == PlainResidualReason::UnresolvedAnnotation) + .count(); + let newly_resolved_sites = baseline_unresolved.saturating_sub(with_rule_unresolved); + + // Independent verification: re-parse `source` from scratch and walk + // its top-level classes directly (mirroring — but not calling — + // plain.rs's naming convention: `_`), + // testing each AnnAssign's raw annotation expression with + // `is_shaped_t_or_none`. This shares no code path with the resolver. + let module_prefix = module.replace('.', "_"); + let mut independently_shape_verified = 0usize; + let mut field_type_matches_inner_reading = true; + if let Ok(parsed) = parse_module(source) { + for stmt in &parsed.syntax().body { + let Stmt::ClassDef(class) = stmt else { + continue; + }; + let model_name = format!("{module_prefix}_{}", class.name.id); + let Some(model_off) = graph_off.models.iter().find(|m| m.name == model_name) else { + continue; + }; + let Some(model_on) = graph_on.models.iter().find(|m| m.name == model_name) else { + continue; + }; + for stmt in &class.body { + let Stmt::AnnAssign(ann) = stmt else { + continue; + }; + let Expr::Name(target) = &*ann.target else { + continue; + }; + let field_name = target.id.as_str(); + let Some(field_off) = model_off.fields.iter().find(|f| f.name == field_name) else { + continue; + }; + let Some(field_on) = model_on.fields.iter().find(|f| f.name == field_name) else { + continue; + }; + // Only sites the rule actually flipped None -> Some. + if field_off.field_type.is_some() || field_on.field_type.is_none() { + continue; + } + if is_shaped_t_or_none(&ann.annotation) { + independently_shape_verified += 1; + } + // Value correctness: the resolved reading must be a real + // type-name shape (non-empty, alphanumeric) — rejects an + // obviously-wrong output without re-deriving the exact + // expected string via the same resolution code. + if let Some(ty) = &field_on.field_type + && (ty.is_empty() || !ty.chars().all(char::is_alphanumeric)) + { + field_type_matches_inner_reading = false; + } + } + } + } + + RatificationReport { + baseline_unresolved, + with_rule_unresolved, + newly_resolved_sites, + independently_shape_verified, + field_type_matches_inner_reading, + } +} + #[cfg(test)] mod tests { use super::*; @@ -169,6 +329,132 @@ mod tests { extract_plain_from_source_with_residuals(src, "mod").1 } + #[test] + fn ratified_is_false_when_the_counts_disagree_in_either_direction() { + // Unit-level coverage of RatificationReport::ratified() itself, + // with hand-built numbers -- the corpus-level tests above cannot + // exercise a resolver/verifier DISAGREEMENT (by construction, a + // correct resolver and a correct independent verifier can never + // disagree without a second, separate bug existing first; a + // disable run on `is_shaped_t_or_none` confirmed this: the + // independent-verification loop only visits sites the resolver + // itself flipped, so a resolver that never over-resolves means + // the mutated verifier is simply never called on a disagreeing + // site). This test exercises the disagreement-DETECTION logic + // directly instead. + let over_claiming = RatificationReport { + baseline_unresolved: 5, + with_rule_unresolved: 2, + newly_resolved_sites: 3, + independently_shape_verified: 2, + field_type_matches_inner_reading: true, + }; + assert!(!over_claiming.ratified()); + + let under_claiming = RatificationReport { + baseline_unresolved: 5, + with_rule_unresolved: 3, + newly_resolved_sites: 2, + independently_shape_verified: 3, + field_type_matches_inner_reading: true, + }; + assert!(!under_claiming.ratified()); + + let bad_value_shape = RatificationReport { + baseline_unresolved: 3, + with_rule_unresolved: 0, + newly_resolved_sites: 3, + independently_shape_verified: 3, + field_type_matches_inner_reading: false, + }; + assert!(!bad_value_shape.ratified()); + + let agrees = RatificationReport { + baseline_unresolved: 3, + with_rule_unresolved: 0, + newly_resolved_sites: 3, + independently_shape_verified: 3, + field_type_matches_inner_reading: true, + }; + assert!(agrees.ratified()); + } + + #[test] + fn ratify_optional_unwrap_ratifies_a_genuine_optional_corpus() { + let src = r#" +class Row: + a: str | None + b: None | int + c: list[str] | None + d: int +"#; + let report = ratify_optional_unwrap(src, "mod"); + // 3 optional-shaped sites (a, b, c); `d` was never unresolved. + assert_eq!(report.baseline_unresolved, 3); + assert_eq!(report.with_rule_unresolved, 0); + assert_eq!(report.newly_resolved_sites, 3); + assert_eq!(report.independently_shape_verified, 3); + assert!(report.field_type_matches_inner_reading); + assert!(report.ratified()); + } + + #[test] + fn ratify_optional_unwrap_leaves_chained_unions_unresolved_on_both_sides() { + // `str | int | None` is NOT `T | None` shaped at the outer node + // (the left operand is itself a BinOp) -- the rule must not + // resolve it, and the independent verifier must not count it + // either. Both sides of the claim stay at zero, together. + let src = "class Row: + a: str | int | None +"; + let report = ratify_optional_unwrap(src, "mod"); + assert_eq!(report.baseline_unresolved, 1); + assert_eq!(report.with_rule_unresolved, 1); + assert_eq!(report.newly_resolved_sites, 0); + assert_eq!(report.independently_shape_verified, 0); + assert!(report.ratified()); + } + + #[test] + fn ratify_optional_unwrap_on_a_corpus_with_no_optional_annotations_is_a_true_no_op() { + // The can-stay-silent half: a fixture with UnresolvedAnnotation + // residuals from a DIFFERENT shape (not `T | None`) must show the + // rule doing nothing at all. + let src = r#" +class Row: + weird: str | int + fwd: "dict[str, int]" +"#; + let report = ratify_optional_unwrap(src, "mod"); + assert_eq!(report.baseline_unresolved, 2); + assert_eq!(report.with_rule_unresolved, 2); + assert_eq!(report.newly_resolved_sites, 0); + assert!(report.ratified()); + } + + #[test] + fn is_shaped_t_or_none_rejects_non_optional_binops() { + // Direct unit coverage of the independent verifier itself, since + // it is what makes the ratification claim non-tautological. + use ruff_python_ast::Stmt; + use ruff_python_parser::parse_module; + let parsed = parse_module( + "x: int | str +y: str | None +", + ) + .expect("parses"); + let mut annotations = Vec::new(); + for stmt in &parsed.syntax().body { + if let Stmt::AnnAssign(ann) = stmt { + annotations.push(&*ann.annotation); + } + } + assert_eq!(annotations.len(), 2); + assert!(!is_shaped_t_or_none(annotations[0])); + assert!(is_shaped_t_or_none(annotations[1])); + } + #[test] fn propose_groups_by_reason_and_detail_and_thresholds_on_support() { let src = r#" diff --git a/crates/ruff_python_spo/src/lib.rs b/crates/ruff_python_spo/src/lib.rs index 916db004a379ef..7bcbfbba2f4109 100644 --- a/crates/ruff_python_spo/src/lib.rs +++ b/crates/ruff_python_spo/src/lib.rs @@ -57,7 +57,10 @@ mod plain; mod templates; mod walk; -pub use drill::{CandidateRow, CrossCorpusRow, RowScope, classify_across_corpora, propose}; +pub use drill::{ + CandidateRow, CrossCorpusRow, RatificationReport, RowScope, classify_across_corpora, propose, + ratify_optional_unwrap, +}; pub use navigation::{ NavScanReport, NavVocab, PyNavEdge, extract_nav_edges, extract_nav_edges_with_report, }; @@ -73,8 +76,9 @@ pub use odoo_views::{ OdooViewScanReport, extract_odoo_view_field_sets, extract_odoo_view_field_sets_with_report, }; pub use plain::{ - PlainResidual, PlainResidualReason, extract_plain, extract_plain_from_source, - extract_plain_from_source_with_residuals, extract_plain_with_residuals, + PlainDrillConfig, PlainResidual, PlainResidualReason, extract_plain, extract_plain_from_source, + extract_plain_from_source_with_config, extract_plain_from_source_with_residuals, + extract_plain_tree_with_config, extract_plain_with_residuals, }; pub use templates::{ ViewFieldSet, ViewScanReport, ViewTarget, extract_template_field_sets, diff --git a/crates/ruff_python_spo/src/plain.rs b/crates/ruff_python_spo/src/plain.rs index 093760eb502bcc..c8cea05455efc8 100644 --- a/crates/ruff_python_spo/src/plain.rs +++ b/crates/ruff_python_spo/src/plain.rs @@ -37,7 +37,7 @@ use std::fs; use std::path::Path; -use ruff_python_ast::{Expr, Number, Stmt, StmtAssign, StmtClassDef}; +use ruff_python_ast::{Expr, Number, Operator, Stmt, StmtAssign, StmtClassDef}; use ruff_python_parser::parse_module; use ruff_spo_triplet::{Field, Function, Model, ModelGraph}; @@ -123,6 +123,46 @@ pub struct PlainResidual { pub detail: Option, } +/// Config the plain arm consults before falling back to its default +/// (residual-producing) behaviour. **Additive-only**: with +/// [`PlainDrillConfig::default()`], every extraction entry point is +/// byte-identical to the config-free path — pinned by +/// `config_off_matches_default_extraction`. +/// +/// This is the promotion gate `drill.rs`'s module doc said this arc did +/// not yet build. One rule is wired end-to-end here, deliberately not +/// all four residual reasons at once: `unwrap_optional_annotation`, +/// chosen because it measured CROSS-CORPUS (dismech 96, ruff/scripts 12, +/// genuinely absent — not merely below threshold — on the A2UI SDK), +/// unlike the `LinkML` `call:PermissibleValue`/`call:EnumDefinition` +/// factories from `crate::drill`'s measurement, which are corpus-scoped +/// to dismech and would need a per-corpus config row, not a generic one. +/// More rules join this struct as siblings; none are ever removed — +/// RESERVE, DON'T RECLAIM, the same discipline the V3 config trie uses. +#[derive(Debug, Clone, Copy, Default, PartialEq, Eq)] +pub struct PlainDrillConfig { + /// Resolve a `T | None` / `None | T` annotation to `T`'s own + /// `field_type_from_annotation` reading instead of leaving it + /// [`PlainResidualReason::UnresolvedAnnotation`]. Any other `BinOp` + /// shape (`int & str`, an arithmetic annotation, …) is untouched — + /// this rule recognises exactly one shape, never guesses. + pub unwrap_optional_annotation: bool, +} + +/// The non-`None` operand of a `BinOp(BitOr)` when exactly one side is a +/// bare `None` literal — `T | None` or `None | T`. `None` for any other +/// shape (`int | str`, `T | U | None`'s outer node, …): a chained union +/// is a nested `BinOp` on one side, which this function does not descend +/// into, so `T | U | None` stays unresolved rather than guessing which +/// arm to keep. +fn optional_operand<'a>(left: &'a Expr, right: &'a Expr) -> Option<&'a Expr> { + match (left, right) { + (Expr::NoneLiteral(_), other) => Some(other), + (other, Expr::NoneLiteral(_)) => Some(other), + _ => None, + } +} + /// The IRI namespace prefix [`extract_plain_from_source`] stamps on the /// [`ModelGraph`] it returns before [`extract_plain`] overwrites it with the /// caller-supplied namespace. @@ -149,6 +189,19 @@ pub fn extract_plain_from_source(source: &str, module: &str) -> ModelGraph { pub fn extract_plain_from_source_with_residuals( source: &str, module: &str, +) -> (ModelGraph, Vec) { + extract_plain_from_source_with_config(source, module, PlainDrillConfig::default()) +} + +/// [`extract_plain_from_source_with_residuals`], additionally taking a +/// [`PlainDrillConfig`]. With the default config this is byte-identical +/// to the config-free path — see [`PlainDrillConfig`]'s doc for why that +/// equivalence is pinned rather than assumed. +#[must_use] +pub fn extract_plain_from_source_with_config( + source: &str, + module: &str, + config: PlainDrillConfig, ) -> (ModelGraph, Vec) { let module_prefix = module.replace('.', "_"); let mut residuals = Vec::new(); @@ -172,6 +225,7 @@ pub fn extract_plain_from_source_with_residuals( class, &module_prefix, module, + config, &mut residuals, )); } @@ -231,10 +285,22 @@ pub fn extract_plain(root: &Path, namespace: &str) -> ModelGraph { pub fn extract_plain_with_residuals( root: &Path, namespace: &str, +) -> (ModelGraph, Vec) { + extract_plain_tree_with_config(root, namespace, PlainDrillConfig::default()) +} + +/// [`extract_plain_with_residuals`], additionally taking a +/// [`PlainDrillConfig`] — the tree-level counterpart of +/// [`extract_plain_from_source_with_config`]. +#[must_use] +pub fn extract_plain_tree_with_config( + root: &Path, + namespace: &str, + config: PlainDrillConfig, ) -> (ModelGraph, Vec) { let mut models = Vec::new(); let mut residuals = Vec::new(); - collect_plain(root, root, &mut models, &mut residuals); + collect_plain(root, root, config, &mut models, &mut residuals); ( ModelGraph { namespace: namespace.to_string(), @@ -248,6 +314,7 @@ pub fn extract_plain_with_residuals( fn collect_plain( root: &Path, dir: &Path, + config: PlainDrillConfig, out: &mut Vec, residuals: &mut Vec, ) { @@ -257,12 +324,12 @@ fn collect_plain( for entry in entries.flatten() { let path = entry.path(); if path.is_dir() { - collect_plain(root, &path, out, residuals); + collect_plain(root, &path, config, out, residuals); } else if path.extension().is_some_and(|e| e == "py") && let Ok(src) = fs::read_to_string(&path) && let Some(module) = module_name(root, &path) { - let (graph, mut rows) = extract_plain_from_source_with_residuals(&src, &module); + let (graph, mut rows) = extract_plain_from_source_with_config(&src, &module, config); out.extend(graph.models); residuals.append(&mut rows); } @@ -397,6 +464,7 @@ fn walk_plain_class( class: &StmtClassDef, module_prefix: &str, module: &str, + config: PlainDrillConfig, residuals: &mut Vec, ) -> Model { let class_name = class.name.id.to_string(); @@ -413,7 +481,7 @@ fn walk_plain_class( match stmt { Stmt::AnnAssign(ann) => { if let Some(name) = name_id(&ann.target) { - let field_type = field_type_from_annotation(&ann.annotation); + let field_type = field_type_from_annotation(&ann.annotation, config); if field_type.is_none() { residuals.push(residual( PlainResidualReason::UnresolvedAnnotation, @@ -528,7 +596,14 @@ fn base_name(expr: &Expr) -> Option { /// `"kw_only"`), or a `Subscript`'s head (`list[str]` → `"list"`, /// `typing.Optional[int]` → `"optional"`). `None` for anything more complex /// (union types `str | None`, `Callable[[int], str]`'s parameter list, …). -fn field_type_from_annotation(annotation: &Expr) -> Option { +fn field_type_from_annotation(annotation: &Expr, config: PlainDrillConfig) -> Option { + if config.unwrap_optional_annotation + && let Expr::BinOp(binop) = annotation + && binop.op == Operator::BitOr + && let Some(inner) = optional_operand(&binop.left, &binop.right) + { + return field_type_from_annotation(inner, config); + } let head = match annotation { Expr::Name(n) => n.id.as_str(), Expr::Attribute(a) => a.attr.id.as_str(), @@ -897,6 +972,86 @@ def helper(): assert!(!rows.is_empty()); } + #[test] + fn config_off_matches_default_extraction() { + // The pinned equivalence: PlainDrillConfig::default() must be + // byte-identical to the config-free path, on a fixture that + // exercises the very rule the config controls (so this isn't + // trivially true of an empty fixture). + let src = r#" +class Row: + items: list[str] + meta: "dict[str, int]" + weird: str | None + also: int | None = None +"#; + let via_default = extract_plain_from_source(src, "mod"); + let (via_config_default, _) = + extract_plain_from_source_with_config(src, "mod", PlainDrillConfig::default()); + assert_eq!(via_default, via_config_default); + } + + #[test] + fn unwrap_optional_annotation_resolves_t_or_none_both_orders() { + let src = r#" +class Row: + a: str | None + b: None | int + c: list[str] | None +"#; + let config = PlainDrillConfig { + unwrap_optional_annotation: true, + }; + let (graph, rows) = extract_plain_from_source_with_config(src, "mod", config); + let m = model(&graph, "mod_Row"); + let get = |name: &str| m.fields.iter().find(|f| f.name == name).unwrap(); + // Each resolves to the SAME reading field_type_from_annotation + // would give the inner type alone — not a special "optional" + // marker, not a guess. + assert_eq!(get("a").field_type.as_deref(), Some("str")); + assert_eq!(get("b").field_type.as_deref(), Some("int")); + assert_eq!(get("c").field_type.as_deref(), Some("list")); + // All three sites resolved -> zero UnresolvedAnnotation residuals. + assert_eq!(count(&rows, PlainResidualReason::UnresolvedAnnotation), 0); + } + + #[test] + fn unwrap_optional_annotation_never_guesses_a_chained_or_non_none_union() { + let src = r#" +class Row: + chained: str | int | None + unrelated: int | str +"#; + let config = PlainDrillConfig { + unwrap_optional_annotation: true, + }; + let (graph, rows) = extract_plain_from_source_with_config(src, "mod", config); + let m = model(&graph, "mod_Row"); + let get = |name: &str| m.fields.iter().find(|f| f.name == name).unwrap(); + // `str | int | None` is BinOp(BinOp(str, |, int), |, None) at the + // outer node -- the LEFT operand is a BinOp, not a bare type, so + // optional_operand's shape check fails and this stays unresolved + // rather than guessing which of str/int to keep. + assert_eq!(get("chained").field_type, None); + // `int | str` has no None operand at all -- untouched. + assert_eq!(get("unrelated").field_type, None); + assert_eq!(count(&rows, PlainResidualReason::UnresolvedAnnotation), 2); + } + + #[test] + fn unwrap_optional_annotation_is_config_gated_not_always_on() { + // The exact same source, config OFF: both stay unresolved. This + // is the inertness half of the exact-count promise -- the rule + // must be OFF by default, not merely "usually off". + let src = "class Row: + a: str | None +"; + let (graph, rows) = extract_plain_from_source_with_residuals(src, "mod"); + let m = model(&graph, "mod_Row"); + assert_eq!(m.fields[0].field_type, None); + assert_eq!(count(&rows, PlainResidualReason::UnresolvedAnnotation), 1); + } + #[test] fn extract_plain_walks_a_tree_and_derives_module_names_from_paths() { let dir =