From 4ef2c61ae6307e74c14a1b639eee2c039478bbe3 Mon Sep 17 00:00:00 2001 From: Konstantin Vyatkin Date: Fri, 10 Jul 2026 13:16:38 +0200 Subject: [PATCH 1/2] feat(metrics): explain SQL change risk contributions --- .cargo/config.toml | 3 + Cargo.lock | 1 + crates/mehen-core/src/analysis.rs | 83 ++++++ crates/mehen-core/src/lib.rs | 3 +- crates/mehen-core/src/report.rs | 37 ++- crates/mehen-report/src/json.rs | 26 +- crates/mehen-report/src/markdown.rs | 61 ++++- crates/mehen-sql/src/composite.rs | 24 +- crates/mehen-sql/src/facts.rs | 329 +++++++++++++++++++++--- crates/mehen-sql/src/lib.rs | 21 +- crates/mehen-sql/tests/contributions.rs | 145 +++++++++++ docs/commands/metrics.mdx | 23 +- docs/concepts/output-formats.mdx | 13 + xtask/Cargo.toml | 1 + xtask/src/main.rs | 12 +- xtask/src/metric_contributions.rs | 58 +++++ 16 files changed, 777 insertions(+), 63 deletions(-) create mode 100644 crates/mehen-sql/tests/contributions.rs create mode 100644 xtask/src/metric_contributions.rs diff --git a/.cargo/config.toml b/.cargo/config.toml index 8ffef529..0ce2d227 100644 --- a/.cargo/config.toml +++ b/.cargo/config.toml @@ -1,3 +1,6 @@ +[alias] +xtask = "run --package xtask --" + [net] # Ruff is consumed as a tagged git dependency (see workspace `Cargo.toml`). # Cargo's bundled libgit2 does not pick up `url.git@github.com:.insteadof` diff --git a/Cargo.lock b/Cargo.lock index 29649b28..c3853b9a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4211,6 +4211,7 @@ dependencies = [ "clap", "mehen-c", "mehen-go", + "serde_json", "tree-sitter", ] diff --git a/crates/mehen-core/src/analysis.rs b/crates/mehen-core/src/analysis.rs index 038c52de..177406c3 100644 --- a/crates/mehen-core/src/analysis.rs +++ b/crates/mehen-core/src/analysis.rs @@ -121,6 +121,73 @@ pub struct MetricContribution { pub reason: ContributionReason, } +/// Optional, deterministically ordered sink for analyzer contribution evidence. +/// +/// Analyzers can construct this directly from +/// [`crate::AnalysisConfig::emit_contributions`]. When disabled, [`record`](Self::record) +/// is a cheap no-op; when enabled, [`finish`](Self::finish) returns entries in +/// source order so JSON and snapshot output remain stable across parser walk +/// implementations and platforms. +#[derive(Debug)] +pub struct ContributionCollector { + enabled: bool, + entries: Vec, +} + +impl ContributionCollector { + pub fn new(enabled: bool) -> Self { + Self { + enabled, + entries: Vec::new(), + } + } + + pub fn is_enabled(&self) -> bool { + self.enabled + } + + pub fn record( + &mut self, + metric: impl Into, + span: SourceSpan, + amount: f64, + reason: impl Into, + ) { + if !self.enabled { + return; + } + self.entries.push(MetricContribution { + metric: metric.into(), + span, + amount, + reason: ContributionReason::new(reason), + }); + } + + pub fn finish(mut self) -> Vec { + self.entries.sort_by(|a, b| { + ( + a.span.start_byte, + a.span.end_byte, + a.span.start_line, + a.span.end_line, + &a.metric, + a.reason.as_str(), + ) + .cmp(&( + b.span.start_byte, + b.span.end_byte, + b.span.start_line, + b.span.end_line, + &b.metric, + b.reason.as_str(), + )) + .then_with(|| a.amount.total_cmp(&b.amount)) + }); + self.entries + } +} + /// A namespaced reason code attached to a [`MetricContribution`]. /// /// Stored as a string so language crates can publish their own reason codes @@ -177,4 +244,20 @@ mod tests { let keys: Vec<&str> = set.iter().map(|(k, _)| k.as_str()).collect(); assert_eq!(keys, vec!["a", "z"]); } + + #[test] + fn contribution_collector_is_gated_and_deterministic() { + let later = SourceSpan::new(20, 24, 3, 3); + let earlier = SourceSpan::new(2, 6, 1, 1); + let mut collector = ContributionCollector::new(true); + collector.record("risk", later, 2.0, "risk.write"); + collector.record("risk", earlier, 8.0, "risk.drop"); + let entries = collector.finish(); + assert_eq!(entries[0].span, earlier); + assert_eq!(entries[1].span, later); + + let mut disabled = ContributionCollector::new(false); + disabled.record("risk", earlier, 8.0, "risk.drop"); + assert!(disabled.finish().is_empty()); + } } diff --git a/crates/mehen-core/src/lib.rs b/crates/mehen-core/src/lib.rs index b381193f..a3dc762f 100644 --- a/crates/mehen-core/src/lib.rs +++ b/crates/mehen-core/src/lib.rs @@ -38,7 +38,8 @@ mod span; mod threshold; pub use analysis::{ - ContributionReason, LanguageAnalysis, MetricContribution, MetricSet, MetricValue, + ContributionCollector, ContributionReason, LanguageAnalysis, MetricContribution, MetricSet, + MetricValue, }; pub use analyzer::{LanguageAnalyzer, LanguageDispatcher}; pub use backend::AnalysisBackend; diff --git a/crates/mehen-core/src/report.rs b/crates/mehen-core/src/report.rs index d08b1a69..6ca4c8a7 100644 --- a/crates/mehen-core/src/report.rs +++ b/crates/mehen-core/src/report.rs @@ -5,8 +5,9 @@ use camino::Utf8PathBuf; use serde::{Deserialize, Serialize}; use crate::{ - AnalysisBackend, AnalysisConfig, Language, LanguageAnalysis, MetricSelector, MetricSpace, - ParseDiagnostic, SourceFile, SourceSpan, SpaceId, SpaceKind, Threshold, ThresholdViolation, + AnalysisBackend, AnalysisConfig, Language, LanguageAnalysis, MetricContribution, + MetricSelector, MetricSpace, ParseDiagnostic, SourceFile, SourceSpan, SpaceId, SpaceKind, + Threshold, ThresholdViolation, }; /// Inputs to `analyze_metrics`. @@ -26,6 +27,10 @@ pub struct MetricsReport { pub analysis_backend: AnalysisBackend, pub diagnostics: Vec, pub root: MetricSpace, + /// Source-resolved evidence emitted by the analyzer. Empty for analyzers + /// or profiles that do not request contribution collection. + #[serde(default, skip_serializing_if = "Vec::is_empty")] + pub contributions: Vec, } impl MetricsReport { @@ -40,6 +45,7 @@ impl MetricsReport { analysis_backend: AnalysisBackend::TreeSitter, diagnostics: Vec::new(), root: MetricSpace::new(SpaceId(0), SpaceKind::Unit, SourceSpan::empty()), + contributions: Vec::new(), } } } @@ -54,10 +60,37 @@ impl From for MetricsReport { analysis_backend: analysis.backend, diagnostics: analysis.diagnostics, root: analysis.root, + contributions: analysis.contributions, } } } +#[cfg(test)] +mod tests { + use super::*; + use crate::{ContributionReason, MetricContribution, MetricKey}; + + #[test] + fn language_analysis_conversion_preserves_contributions() { + let contribution = MetricContribution { + metric: MetricKey::new("sql.change_risk_score"), + span: SourceSpan::new(0, 4, 1, 1), + amount: 8.0, + reason: ContributionReason::new("sql.change_risk.drop"), + }; + let analysis = LanguageAnalysis { + language: Language::Sql, + backend: AnalysisBackend::Sqruff, + diagnostics: Vec::new(), + root: MetricSpace::new(SpaceId(0), SpaceKind::Unit, SourceSpan::empty()), + contributions: vec![contribution.clone()], + }; + + let report = MetricsReport::from(analysis); + assert_eq!(report.contributions, vec![contribution]); + } +} + /// Inputs to `analyze_diff`. /// /// Phase 1 ships the type so that later phases can fill in the orchestration diff --git a/crates/mehen-report/src/json.rs b/crates/mehen-report/src/json.rs index 7bf2ee7c..3d0f3d0f 100644 --- a/crates/mehen-report/src/json.rs +++ b/crates/mehen-report/src/json.rs @@ -60,7 +60,8 @@ pub fn render_diff_json(report: &DiffReport, pretty: bool) -> serde_json::Result mod tests { use super::*; use mehen_core::{ - AnalysisBackend, MetricKey, MetricSpace, MetricsReport, SourceSpan, SpaceId, SpaceKind, + AnalysisBackend, ContributionReason, MetricContribution, MetricKey, MetricSpace, + MetricsReport, SourceSpan, SpaceId, SpaceKind, }; fn report_with(language: Language, key: &str, value: f64) -> MetricsReport { @@ -74,6 +75,7 @@ mod tests { analysis_backend: AnalysisBackend::Sqruff, diagnostics: Vec::new(), root, + contributions: Vec::new(), } } @@ -109,4 +111,26 @@ mod tests { "source-code report must carry the pivoted `metrics` family block" ); } + + #[test] + fn contributions_are_serialized_only_when_present() { + let empty = report_with(Language::Sql, "sql.change_risk_score", 0.0); + let empty_json: serde_json::Value = + serde_json::from_str(&render_metrics_json(&empty, false).unwrap()).unwrap(); + assert!(empty_json.get("contributions").is_none()); + + let mut explained = report_with(Language::Sql, "sql.change_risk_score", 8.0); + explained.contributions.push(MetricContribution { + metric: MetricKey::new("sql.change_risk_score"), + span: SourceSpan::new(0, 12, 1, 1), + amount: 8.0, + reason: ContributionReason::new("sql.change_risk.drop"), + }); + let json: serde_json::Value = + serde_json::from_str(&render_metrics_json(&explained, false).unwrap()).unwrap(); + assert_eq!(json["contributions"][0]["metric"], "sql.change_risk_score"); + assert_eq!(json["contributions"][0]["amount"], 8.0); + assert_eq!(json["contributions"][0]["reason"], "sql.change_risk.drop"); + assert_eq!(json["contributions"][0]["span"]["start_line"], 1); + } } diff --git a/crates/mehen-report/src/markdown.rs b/crates/mehen-report/src/markdown.rs index afce6a4f..77c124c7 100644 --- a/crates/mehen-report/src/markdown.rs +++ b/crates/mehen-report/src/markdown.rs @@ -4,8 +4,8 @@ use std::fmt::Write; use mehen_core::{ - DiagnosticSeverity, DiffReport, Language, MetricKey, MetricSet, MetricSpace, MetricsReport, - ParseDiagnostic, SpaceKind, + DiagnosticSeverity, DiffReport, Language, MetricContribution, MetricKey, MetricSet, + MetricSpace, MetricsReport, ParseDiagnostic, SpaceKind, }; use crate::metrics_json::{ @@ -42,12 +42,46 @@ pub fn render_metrics_markdown(report: &MetricsReport) -> String { let _ = writeln!(out, "- schema: `{}`", report.schema_version); write_diagnostics(&mut out, &report.diagnostics); + write_contributions(&mut out, &report.contributions); write_unit_metrics(&mut out, &report.root.metrics, report.language); write_nested_spaces(&mut out, &report.root.spaces, 0, report.language); out } +fn write_contributions(out: &mut String, contributions: &[MetricContribution]) { + if contributions.is_empty() { + return; + } + let _ = writeln!(out); + let _ = writeln!(out, "## Contributions"); + let _ = writeln!(out); + let _ = writeln!(out, "| metric | amount | reason | lines |"); + let _ = writeln!(out, "|---|---:|---|---:|"); + for contribution in contributions { + let sign = if contribution.amount.is_sign_negative() { + "-" + } else { + "+" + }; + let amount = fmt_metric(contribution.amount.abs()); + let lines = if contribution.span.start_line == contribution.span.end_line { + format!("L{}", contribution.span.start_line) + } else { + format!( + "L{}–L{}", + contribution.span.start_line, contribution.span.end_line + ) + }; + let _ = writeln!( + out, + "| `{}` | {sign}{amount} | `{}` | {lines} |", + contribution.metric, + contribution.reason.as_str(), + ); + } +} + fn write_diagnostics(out: &mut String, diagnostics: &[ParseDiagnostic]) { if diagnostics.is_empty() { return; @@ -727,8 +761,8 @@ pub fn render_diff_github_markdown(report: &DiffReport) -> String { mod tests { use super::*; use mehen_core::{ - AnalysisBackend, Language, MetricKey, MetricSpace, ParseDiagnostic, SourceSpan, SpaceId, - SpaceKind, + AnalysisBackend, ContributionReason, Language, MetricContribution, MetricKey, MetricSpace, + ParseDiagnostic, SourceSpan, SpaceId, SpaceKind, }; fn report_with_metrics(pairs: &[(&str, f64)]) -> MetricsReport { @@ -744,6 +778,7 @@ mod tests { analysis_backend: AnalysisBackend::PythonRuff, diagnostics: Vec::new(), root, + contributions: Vec::new(), } } @@ -816,6 +851,21 @@ mod tests { assert!(!md.contains("## Diagnostics")); } + #[test] + fn renders_contribution_evidence_when_present() { + let mut report = report_with_metrics(&[("cyclomatic.sum", 1.0)]); + report.contributions.push(MetricContribution { + metric: MetricKey::new("sql.change_risk_score"), + span: SourceSpan::new(10, 30, 2, 3), + amount: 8.0, + reason: ContributionReason::new("sql.change_risk.drop"), + }); + let md = render_metrics_markdown(&report); + assert!(md.contains("## Contributions")); + assert!(md.contains("| `sql.change_risk_score` | +8 |")); + assert!(md.contains("`sql.change_risk.drop` | L2–L3")); + } + #[test] fn renders_markdown_family_when_language_is_markdown() { // Regression: previously `write_unit_metrics` always pivoted @@ -847,6 +897,7 @@ mod tests { analysis_backend: AnalysisBackend::PulldownCmark, diagnostics: Vec::new(), root, + contributions: Vec::new(), }; let md = render_metrics_markdown(&report); @@ -914,6 +965,7 @@ mod tests { analysis_backend: AnalysisBackend::PythonRuff, diagnostics: Vec::new(), root, + contributions: Vec::new(), }; let md = render_metrics_markdown(&report); assert!(md.contains("## Spaces")); @@ -950,6 +1002,7 @@ mod tests { analysis_backend: AnalysisBackend::Sqruff, diagnostics: Vec::new(), root, + contributions: Vec::new(), }; let md = render_metrics_markdown(&report); let space_section = md.split("## Spaces").nth(1).expect("Spaces section"); diff --git a/crates/mehen-sql/src/composite.rs b/crates/mehen-sql/src/composite.rs index 3fe74add..f4d7e4a2 100644 --- a/crates/mehen-sql/src/composite.rs +++ b/crates/mehen-sql/src/composite.rs @@ -10,7 +10,7 @@ //! judgments — the analyzer publishes the raw metrics first and these second. use crate::dialect::DialectResolution; -use crate::facts::SqlFileFacts; +use crate::facts::{ChangeRiskFactor, SqlFileFacts}; use crate::loc::SqlLoc; /// The six composite scores published per file. @@ -182,17 +182,17 @@ fn modularization_credit(f: &SqlFileFacts) -> f64 { /// `+ 5 * dynamic_sql_count` term here. fn change_risk(f: &SqlFileFacts) -> f64 { let o = &f.objects; - 8.0 * o.drop_count as f64 - + 8.0 * o.truncate_count as f64 - + 6.0 * o.alter_count as f64 - + 6.0 * o.delete_without_where_count as f64 - + 6.0 * o.update_without_where_count as f64 - + 5.0 * o.grant_revoke_count as f64 - + 4.0 * o.merge_count as f64 - + 4.0 * o.create_or_replace_count as f64 - + 3.0 * o.transaction_control_count as f64 - + 2.0 * o.write_object_count as f64 - + 1.0 * o.read_object_count as f64 + ChangeRiskFactor::Drop.amount() * o.drop_count as f64 + + ChangeRiskFactor::Truncate.amount() * o.truncate_count as f64 + + ChangeRiskFactor::Alter.amount() * o.alter_count as f64 + + ChangeRiskFactor::DeleteWithoutWhere.amount() * o.delete_without_where_count as f64 + + ChangeRiskFactor::UpdateWithoutWhere.amount() * o.update_without_where_count as f64 + + ChangeRiskFactor::GrantRevoke.amount() * o.grant_revoke_count as f64 + + ChangeRiskFactor::Merge.amount() * o.merge_count as f64 + + ChangeRiskFactor::CreateOrReplace.amount() * o.create_or_replace_count as f64 + + ChangeRiskFactor::TransactionControl.amount() * o.transaction_control_count as f64 + + ChangeRiskFactor::WriteObject.amount() * o.write_object_count as f64 + + ChangeRiskFactor::ReadObject.amount() * o.read_object_count as f64 } /// SQL Review Burden Index (research foundation §8.3), 0..100. diff --git a/crates/mehen-sql/src/facts.rs b/crates/mehen-sql/src/facts.rs index 8af363dc..61e89c18 100644 --- a/crates/mehen-sql/src/facts.rs +++ b/crates/mehen-sql/src/facts.rs @@ -17,6 +17,7 @@ //! (`Rc>`) that would conflict with borrows held across the call //! (see [`extract_cte_graph`]). +use mehen_core::SourceSpan; use sqruff_lib_core::dialects::Dialect; use sqruff_lib_core::dialects::syntax::{SyntaxKind, SyntaxSet}; use sqruff_lib_core::parser::segments::ErasedSegment; @@ -230,6 +231,62 @@ pub(crate) struct ObjectFacts { pub touch_count: u32, } +/// Stable classification for one term in `sql.change_risk_score`. +/// +/// The factor owns both its public reason code and score weight so the +/// aggregate formula and emitted evidence cannot drift independently. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub(crate) enum ChangeRiskFactor { + Drop, + Truncate, + Alter, + DeleteWithoutWhere, + UpdateWithoutWhere, + GrantRevoke, + Merge, + CreateOrReplace, + TransactionControl, + WriteObject, + ReadObject, +} + +impl ChangeRiskFactor { + pub(crate) fn amount(self) -> f64 { + match self { + Self::Drop | Self::Truncate => 8.0, + Self::Alter | Self::DeleteWithoutWhere | Self::UpdateWithoutWhere => 6.0, + Self::GrantRevoke => 5.0, + Self::Merge | Self::CreateOrReplace => 4.0, + Self::TransactionControl => 3.0, + Self::WriteObject => 2.0, + Self::ReadObject => 1.0, + } + } + + pub(crate) fn reason(self) -> &'static str { + match self { + Self::Drop => "sql.change_risk.drop", + Self::Truncate => "sql.change_risk.truncate", + Self::Alter => "sql.change_risk.alter", + Self::DeleteWithoutWhere => "sql.change_risk.delete_without_where", + Self::UpdateWithoutWhere => "sql.change_risk.update_without_where", + Self::GrantRevoke => "sql.change_risk.grant_revoke", + Self::Merge => "sql.change_risk.merge", + Self::CreateOrReplace => "sql.change_risk.create_or_replace", + Self::TransactionControl => "sql.change_risk.transaction_control", + Self::WriteObject => "sql.change_risk.write_object", + Self::ReadObject => "sql.change_risk.read_object", + } + } +} + +/// One source-resolved term in `sql.change_risk_score`. +#[derive(Clone, Debug, PartialEq)] +pub(crate) struct ChangeRiskEvidence { + pub span: SourceSpan, + pub factor: ChangeRiskFactor, +} + /// Per-statement facts with source span (research foundation §5.2). #[derive(Clone, Debug)] pub(crate) struct StatementFacts { @@ -271,6 +328,7 @@ pub(crate) struct SqlFileFacts { pub output: OutputFacts, pub ctes: CteFacts, pub objects: ObjectFacts, + pub change_risk_evidence: Vec, pub halstead: HalsteadFacts, pub relation_ref_count: u32, /// Count of `SyntaxKind::Unparsable` segments (parser-health, §6.16). @@ -298,6 +356,7 @@ pub(crate) fn extract( root: &ErasedSegment, dialect: &Dialect, line_at: impl Fn(u32) -> u32, + emit_contributions: bool, ) -> SqlFileFacts { let mut facts = SqlFileFacts::default(); @@ -378,7 +437,7 @@ pub(crate) fn extract( extract_cte_graph(root, dialect, &mut facts.ctes); // ── object-touch / DML-DDL risk ───────────────────────────────── - extract_objects(root, &line_at, &mut facts); + extract_objects(root, &line_at, &mut facts, emit_contributions); // ── Halstead ──────────────────────────────────────────────────── extract_halstead(root, &mut facts.halstead); @@ -1900,25 +1959,83 @@ fn longest_chain(edges: &std::collections::BTreeMap>, nodes: // ── object-touch / DML-DDL risk ─────────────────────────────────────── -fn extract_objects(root: &ErasedSegment, line_at: &impl Fn(u32) -> u32, facts: &mut SqlFileFacts) { - let _ = line_at; +fn extract_objects( + root: &ErasedSegment, + line_at: &impl Fn(u32) -> u32, + facts: &mut SqlFileFacts, + emit_contributions: bool, +) { + let fallback_span = if emit_contributions { + segment_span(root, line_at).unwrap_or_else(SourceSpan::empty) + } else { + SourceSpan::empty() + }; let obj = &mut facts.objects; + let evidence = &mut facts.change_risk_evidence; // Per-statement-kind counters (used by the DML/DDL/TCL metric keys). for stmt in &facts.statements { match stmt.kind { StatementKind::Insert => obj.insert_count += 1, StatementKind::Update => obj.update_count += 1, StatementKind::Delete => obj.delete_count += 1, - StatementKind::Merge => obj.merge_count += 1, + StatementKind::Merge => { + obj.merge_count += 1; + record_change_risk( + evidence, + emit_contributions, + statement_span(stmt), + ChangeRiskFactor::Merge, + ); + } StatementKind::CreateTable | StatementKind::CreateTableAsSelect | StatementKind::CreateView | StatementKind::CreateOther => obj.create_count += 1, - StatementKind::AlterTable => obj.alter_count += 1, - StatementKind::Drop => obj.drop_count += 1, - StatementKind::Truncate => obj.truncate_count += 1, - StatementKind::Grant | StatementKind::Revoke => obj.grant_revoke_count += 1, - StatementKind::TransactionControl => obj.transaction_control_count += 1, + StatementKind::AlterTable => { + obj.alter_count += 1; + record_change_risk( + evidence, + emit_contributions, + statement_span(stmt), + ChangeRiskFactor::Alter, + ); + } + StatementKind::Drop => { + obj.drop_count += 1; + record_change_risk( + evidence, + emit_contributions, + statement_span(stmt), + ChangeRiskFactor::Drop, + ); + } + StatementKind::Truncate => { + obj.truncate_count += 1; + record_change_risk( + evidence, + emit_contributions, + statement_span(stmt), + ChangeRiskFactor::Truncate, + ); + } + StatementKind::Grant | StatementKind::Revoke => { + obj.grant_revoke_count += 1; + record_change_risk( + evidence, + emit_contributions, + statement_span(stmt), + ChangeRiskFactor::GrantRevoke, + ); + } + StatementKind::TransactionControl => { + obj.transaction_control_count += 1; + record_change_risk( + evidence, + emit_contributions, + statement_span(stmt), + ChangeRiskFactor::TransactionControl, + ); + } _ => {} } } @@ -1929,10 +2046,32 @@ fn extract_objects(root: &ErasedSegment, line_at: &impl Fn(u32) -> u32, facts: & // both read and written is touched once. Read objects are table references // in FROM/JOIN positions; write objects are the statement-level targets of // write statements. Names are uppercased so case variants collapse. - let (read_objects, write_objects) = collect_touched_objects(root); + let (read_objects, write_objects) = collect_touched_objects(root, line_at, emit_contributions); obj.read_object_count = read_objects.len() as u32; obj.write_object_count = write_objects.len() as u32; - obj.touch_count = read_objects.union(&write_objects).count() as u32; + obj.touch_count = read_objects + .keys() + .chain(write_objects.keys()) + .collect::>() + .len() as u32; + if emit_contributions { + for span in write_objects.values() { + record_change_risk( + evidence, + true, + span.unwrap_or(fallback_span), + ChangeRiskFactor::WriteObject, + ); + } + for span in read_objects.values() { + record_change_risk( + evidence, + true, + span.unwrap_or(fallback_span), + ChangeRiskFactor::ReadObject, + ); + } + } // UPDATE/DELETE without WHERE. The WHERE crawl must stop at nested // SELECT nodes: `UPDATE t SET v = (SELECT v FROM u WHERE u.id = t.id)` has no @@ -1949,6 +2088,14 @@ fn extract_objects(root: &ErasedSegment, line_at: &impl Fn(u32) -> u32, facts: & for u in &updates { if !has_own_where_clause(u) { obj.update_without_where_count += 1; + if emit_contributions { + record_change_risk( + evidence, + true, + segment_span(u, line_at).unwrap_or(fallback_span), + ChangeRiskFactor::UpdateWithoutWhere, + ); + } } } let deletes = root.recursive_crawl( @@ -1960,6 +2107,14 @@ fn extract_objects(root: &ErasedSegment, line_at: &impl Fn(u32) -> u32, facts: & for d in &deletes { if !has_own_where_clause(d) { obj.delete_without_where_count += 1; + if emit_contributions { + record_change_risk( + evidence, + true, + segment_span(d, line_at).unwrap_or(fallback_span), + ChangeRiskFactor::DeleteWithoutWhere, + ); + } } } @@ -1967,14 +2122,32 @@ fn extract_objects(root: &ErasedSegment, line_at: &impl Fn(u32) -> u32, facts: & // (comments/whitespace excluded) so a comment like `-- RETURNING id` or // `/* CREATE OR REPLACE */` never trips them, and arbitrary whitespace // between keywords is irrelevant (we look at adjacency in the token list). - let code_words = code_token_words(root); + let code_tokens = code_tokens(root, line_at, emit_contributions); // CREATE OR REPLACE — three consecutive code tokens, handling // `CREATE\nOR REPLACE` / `CREATE OR REPLACE` (whitespace-insensitive). - obj.create_or_replace_count = code_words + let create_or_replace = code_tokens .windows(3) - .filter(|w| w[0] == "CREATE" && w[1] == "OR" && w[2] == "REPLACE") - .count() as u32; + .filter(|w| w[0].word == "CREATE" && w[1].word == "OR" && w[2].word == "REPLACE") + .collect::>(); + obj.create_or_replace_count = create_or_replace.len() as u32; + for tokens in create_or_replace { + let span = match (tokens[0].span, tokens[2].span) { + (Some(start), Some(end)) => SourceSpan::new( + start.start_byte, + end.end_byte, + start.start_line, + end.end_line, + ), + _ => fallback_span, + }; + record_change_risk( + evidence, + emit_contributions, + span, + ChangeRiskFactor::CreateOrReplace, + ); + } // RETURNING (Postgres/Oracle) / OUTPUT (T-SQL) DML result clauses, counted // from `Keyword` tokens inside DML statements (INSERT/UPDATE/DELETE/MERGE). @@ -1997,8 +2170,22 @@ fn extract_objects(root: &ErasedSegment, line_at: &impl Fn(u32) -> u32, facts: & /// The uppercased text of every *code* leaf token in `node` (comments, /// whitespace, and meta tokens excluded), in source order. Used for /// adjacency/word checks that must ignore comments and be whitespace-agnostic. -fn code_token_words(node: &ErasedSegment) -> Vec { - fn walk(node: &ErasedSegment, out: &mut Vec) { +struct CodeToken { + word: String, + span: Option, +} + +fn code_tokens( + node: &ErasedSegment, + line_at: &impl Fn(u32) -> u32, + emit_contributions: bool, +) -> Vec { + fn walk( + node: &ErasedSegment, + line_at: &impl Fn(u32) -> u32, + emit_contributions: bool, + out: &mut Vec, + ) { let children = node.segments(); if children.is_empty() { if node.is_comment() || node.is_whitespace() || node.is_meta() { @@ -2006,16 +2193,23 @@ fn code_token_words(node: &ErasedSegment) -> Vec { } let raw = node.raw().trim(); if !raw.is_empty() { - out.push(raw.to_ascii_uppercase()); + out.push(CodeToken { + word: raw.to_ascii_uppercase(), + span: if emit_contributions { + segment_span(node, line_at) + } else { + None + }, + }); } return; } for child in children { - walk(child, out); + walk(child, line_at, emit_contributions, out); } } let mut out = Vec::new(); - walk(node, &mut out); + walk(node, line_at, emit_contributions, &mut out); out } @@ -2077,13 +2271,15 @@ const PROCEDURAL_DEFINITIONS: SyntaxSet = SyntaxSet::new(&[ /// uppercased so case variants collapse to one object. fn collect_touched_objects( root: &ErasedSegment, + line_at: &impl Fn(u32) -> u32, + emit_contributions: bool, ) -> ( - std::collections::BTreeSet, - std::collections::BTreeSet, + std::collections::BTreeMap>, + std::collections::BTreeMap>, ) { - use std::collections::BTreeSet; - let mut read = BTreeSet::new(); - let mut write = BTreeSet::new(); + use std::collections::BTreeMap; + let mut read = BTreeMap::new(); + let mut write = BTreeMap::new(); // CTE names are query-local aliases scoped to their *owning query block*, // not database objects. Scope matters at two levels: @@ -2097,7 +2293,14 @@ fn collect_touched_objects( // identity (`cte_local_refs`), not by a flat name set. for stmt in &top_level_statements(root) { let cte_local = cte_local_refs(stmt); - collect_statement_objects(stmt, &cte_local, &mut read, &mut write); + collect_statement_objects( + stmt, + &cte_local, + &mut read, + &mut write, + line_at, + emit_contributions, + ); } (read, write) @@ -2164,8 +2367,10 @@ fn cte_local_refs(stmt: &ErasedSegment) -> Vec { fn collect_statement_objects( stmt: &ErasedSegment, cte_local: &[ErasedSegment], - read: &mut std::collections::BTreeSet, - write: &mut std::collections::BTreeSet, + read: &mut std::collections::BTreeMap>, + write: &mut std::collections::BTreeMap>, + line_at: &impl Fn(u32) -> u32, + emit_contributions: bool, ) { let is_cte_local = |tr: &ErasedSegment| cte_local.iter().any(|c| c.is(tr)); // The objects a write statement targets are not always `TableReference`s: @@ -2222,11 +2427,11 @@ fn collect_statement_objects( } let name = tr.raw().to_ascii_uppercase(); if i == 0 || all_targets { - write.insert(name); + record_object_occurrence(write, name, tr, line_at, emit_contributions); write_target_nodes.push(tr.clone()); } else { // FROM/USING source tables of a DML statement are reads. - read.insert(name); + record_object_occurrence(read, name, tr, line_at, emit_contributions); } } } @@ -2253,12 +2458,74 @@ fn collect_statement_objects( ) { let is_write_target = write_target_nodes.iter().any(|w| w.is(&tr)); if !is_cte_local(&tr) && !is_write_target { - read.insert(tr.raw().to_ascii_uppercase()); + record_object_occurrence( + read, + tr.raw().to_ascii_uppercase(), + &tr, + line_at, + emit_contributions, + ); } } } } +fn record_object_occurrence( + objects: &mut std::collections::BTreeMap>, + name: String, + segment: &ErasedSegment, + line_at: &impl Fn(u32) -> u32, + emit_contributions: bool, +) { + let candidate = if emit_contributions { + segment_span(segment, line_at) + } else { + None + }; + objects + .entry(name) + .and_modify(|current| match (*current, candidate) { + (None, Some(_)) => *current = candidate, + (Some(existing), Some(new)) if new.start_byte < existing.start_byte => { + *current = candidate; + } + _ => {} + }) + .or_insert(candidate); +} + +fn statement_span(statement: &StatementFacts) -> SourceSpan { + SourceSpan::new( + statement.start_byte, + statement.end_byte, + statement.start_line, + statement.end_line, + ) +} + +fn segment_span(segment: &ErasedSegment, line_at: &impl Fn(u32) -> u32) -> Option { + let marker = segment.get_position_marker()?; + let start_byte = marker.source_slice.start as u32; + let end_byte = marker.source_slice.end as u32; + Some(SourceSpan::new( + start_byte, + end_byte, + line_at(start_byte), + line_at(end_byte.saturating_sub(1)), + )) +} + +fn record_change_risk( + evidence: &mut Vec, + enabled: bool, + span: SourceSpan, + factor: ChangeRiskFactor, +) { + if enabled { + evidence.push(ChangeRiskEvidence { span, factor }); + } +} + // ── Halstead ──────────────────────────────────────────────────────────── fn extract_halstead(root: &ErasedSegment, h: &mut HalsteadFacts) { diff --git a/crates/mehen-sql/src/lib.rs b/crates/mehen-sql/src/lib.rs index 600664d2..2b2c213e 100644 --- a/crates/mehen-sql/src/lib.rs +++ b/crates/mehen-sql/src/lib.rs @@ -34,8 +34,9 @@ mod loc; mod metrics; use mehen_core::{ - AnalysisBackend, AnalysisConfig, Language, LanguageAnalysis, LanguageAnalyzer, MetricSpace, - ParseDiagnostic, Result, SourceFile, SourceSpan, SpaceId, SpaceKind, byte_offset_clamped, + AnalysisBackend, AnalysisConfig, ContributionCollector, Language, LanguageAnalysis, + LanguageAnalyzer, MetricSpace, ParseDiagnostic, Result, SourceFile, SourceSpan, SpaceId, + SpaceKind, byte_offset_clamped, }; use smol_str::SmolStr; @@ -69,7 +70,7 @@ impl LanguageAnalyzer for SqlAnalyzer { AnalysisBackend::Sqruff } - fn analyze(&self, source: &SourceFile, _config: &AnalysisConfig) -> Result { + fn analyze(&self, source: &SourceFile, config: &AnalysisConfig) -> Result { let file_span = SourceSpan { start_byte: 0, end_byte: byte_offset_clamped(source.text.len()), @@ -131,7 +132,7 @@ impl LanguageAnalyzer for SqlAnalyzer { let line_index = &source.line_index; let line_at = |byte: u32| line_index.line_at(byte); - let mut file_facts = facts::extract(&parsed, &dialect, line_at); + let mut file_facts = facts::extract(&parsed, &dialect, line_at, config.emit_contributions); // Lexer errors (malformed tokens) are distinct from unparsable parse // segments. The current sqruff release never populates this vector, but // surface them into parser-health so a future version cannot make @@ -148,6 +149,16 @@ impl LanguageAnalyzer for SqlAnalyzer { metrics::publish(&file_facts, &loc_stats, &resolution, &mut root.metrics); publish_dialect_labels(&mut root, &resolution); + let mut contribution_collector = ContributionCollector::new(config.emit_contributions); + for item in &file_facts.change_risk_evidence { + contribution_collector.record( + "sql.change_risk_score", + item.span, + item.factor.amount(), + item.factor.reason(), + ); + } + // Per-statement spaces so top-offenders / nested reporting can attribute // metrics to a statement's line range (research foundation §4.4). // Space ids start at 1 (the root unit is 0). @@ -203,7 +214,7 @@ impl LanguageAnalyzer for SqlAnalyzer { backend: AnalysisBackend::Sqruff, diagnostics, root, - contributions: Vec::new(), + contributions: contribution_collector.finish(), }) } } diff --git a/crates/mehen-sql/tests/contributions.rs b/crates/mehen-sql/tests/contributions.rs new file mode 100644 index 00000000..6a1041a7 --- /dev/null +++ b/crates/mehen-sql/tests/contributions.rs @@ -0,0 +1,145 @@ +// SPDX-License-Identifier: AGPL-3.0-only +// Copyright (C) 2026 Konstantin Vyatkin + +use mehen_core::{ + AnalysisConfig, Language, LanguageAnalysis, LanguageAnalyzer, MetricKey, SourceFile, +}; +use mehen_sql::SqlAnalyzer; + +fn analyze(sql: &str, config: &AnalysisConfig) -> LanguageAnalysis { + SqlAnalyzer::new() + .analyze( + &SourceFile::new("risk.sql".into(), Language::Sql, sql.to_string()), + config, + ) + .expect("SQL analysis succeeds") +} + +fn metric(analysis: &LanguageAnalysis, key: &str) -> f64 { + analysis + .root + .metrics + .get(&MetricKey::new(key)) + .unwrap_or_else(|| panic!("missing metric {key}")) + .as_f64() +} + +fn assert_risk_sum(analysis: &LanguageAnalysis) { + let sum: f64 = analysis + .contributions + .iter() + .filter(|item| item.metric.as_str() == "sql.change_risk_score") + .map(|item| item.amount) + .sum(); + assert_eq!(sum, metric(analysis, "sql.change_risk_score")); +} + +#[test] +fn change_risk_contributions_are_weighted_spanned_and_complete() { + let sql = "DROP TABLE t;\nTRUNCATE TABLE s;\nDELETE FROM u;"; + let analysis = analyze(sql, &AnalysisConfig::production()); + let contributions = &analysis.contributions; + + assert!(!contributions.is_empty()); + assert!(contributions.iter().all(|item| { + item.metric.as_str() == "sql.change_risk_score" + && item.span.start_byte <= item.span.end_byte + && item.span.end_byte as usize <= sql.len() + && item.span.start_line >= 1 + && item.span.start_line <= item.span.end_line + })); + + assert_risk_sum(&analysis); + + let reasons: Vec<_> = contributions + .iter() + .map(|item| item.reason.as_str()) + .collect(); + assert!(reasons.contains(&"sql.change_risk.drop")); + assert!(reasons.contains(&"sql.change_risk.truncate")); + assert!(reasons.contains(&"sql.change_risk.delete_without_where")); + assert_eq!( + reasons + .iter() + .filter(|reason| **reason == "sql.change_risk.write_object") + .count(), + 3 + ); + + // The collector's public ordering contract is source order. + assert!(contributions.windows(2).all(|pair| { + (pair[0].span.start_byte, pair[0].span.end_byte) + <= (pair[1].span.start_byte, pair[1].span.end_byte) + })); +} + +#[test] +fn distinct_object_evidence_uses_the_first_occurrence() { + let sql = "SELECT * FROM t;\nSELECT id FROM t;"; + let analysis = analyze(sql, &AnalysisConfig::production()); + let reads: Vec<_> = analysis + .contributions + .iter() + .filter(|item| item.reason.as_str() == "sql.change_risk.read_object") + .collect(); + + assert_eq!(reads.len(), 1); + assert_eq!(reads[0].amount, 1.0); + assert_eq!(reads[0].span.start_line, 1); + assert_eq!(metric(&analysis, "sql.object.read_count"), 1.0); + assert_eq!(metric(&analysis, "sql.change_risk_score"), 1.0); +} + +#[test] +fn benchmark_profile_skips_evidence_without_changing_metrics() { + let sql = "UPDATE accounts SET enabled = FALSE"; + let production = analyze(sql, &AnalysisConfig::production()); + let benchmark = analyze(sql, &AnalysisConfig::benchmark()); + + assert!(!production.contributions.is_empty()); + assert!(benchmark.contributions.is_empty()); + assert_eq!( + metric(&production, "sql.change_risk_score"), + metric(&benchmark, "sql.change_risk_score") + ); +} + +#[test] +fn every_implemented_change_risk_term_has_a_stable_reason() { + let cases = [ + ( + "ALTER TABLE customers ADD COLUMN active BOOLEAN", + "sql.change_risk.alter", + ), + ( + "UPDATE accounts SET enabled = FALSE", + "sql.change_risk.update_without_where", + ), + ( + "GRANT SELECT ON customers TO reporting_role", + "sql.change_risk.grant_revoke", + ), + ( + "MERGE INTO accounts a USING updates u ON a.id = u.id WHEN MATCHED THEN UPDATE SET a.balance = u.balance", + "sql.change_risk.merge", + ), + ( + "CREATE OR REPLACE VIEW active_accounts AS SELECT * FROM accounts", + "sql.change_risk.create_or_replace", + ), + ("BEGIN; COMMIT;", "sql.change_risk.transaction_control"), + ]; + + for (sql, expected_reason) in cases { + let analysis = analyze(sql, &AnalysisConfig::production()); + assert!( + analysis + .contributions + .iter() + .any(|item| item.reason.as_str() == expected_reason), + "missing {expected_reason} for {sql:?}: {:?}", + analysis.contributions + ); + assert_risk_sum(&analysis); + } +} diff --git a/docs/commands/metrics.mdx b/docs/commands/metrics.mdx index d41e0b4e..d6bc364e 100644 --- a/docs/commands/metrics.mdx +++ b/docs/commands/metrics.mdx @@ -71,18 +71,35 @@ Markdown Halstead, link debt, table burden, visual scaffold, artifact debt, repo evidence coverage, filler/lazy risk, review criticality, section balance, good scaffold. The opt-in [prose layer](/metrics/markdown/prose/overview) ships behind Cargo features. +When an analyzer supports source-resolved evidence, the default profile also emits a +`contributions` array. Each entry identifies the metric, weighted amount, stable reason code, and +byte/line span responsible for that amount. SQL change risk is the first supported metric: +`sql.change_risk_score` explains destructive DDL/DML and distinct object-touch terms. The +`ci`/`strict` profiles skip this optional evidence to keep automated payloads lean. + ## Profiles `--profile` selects a built-in preset for what gets reported and how thresholds are interpreted: | Profile | Use case | |---|---| -| `default` | Local developer output. No implicit failure policy. | -| `ci` | GitHub Action-friendly defaults. Stable tables, standard metric selector set. | -| `strict` | Same output as `ci`, with tighter built-in threshold suggestions. | +| `default` | Local developer output, including supported contribution evidence. No implicit failure policy. | +| `ci` | GitHub Action-friendly defaults without contribution evidence. Stable tables, standard metric selector set. | +| `strict` | Same evidence behavior as `ci`, with tighter built-in threshold suggestions. | Explicit CLI flags always override profile defaults. +## Inspecting contribution evidence + +Maintainers can print only the contribution array for a file through the developer task: + +```bash +cargo xtask metric-contributions migrations/2026-07-10.sql +``` + +The task runs the production analyzer through the real `mehen metrics` path, so language detection, +profile behavior, reason codes, and spans match the public CLI report. + ## See also - [`mehen diff`](/commands/diff) — compare metrics between revisions. diff --git a/docs/concepts/output-formats.mdx b/docs/concepts/output-formats.mdx index 93e3c815..441fb771 100644 --- a/docs/concepts/output-formats.mdx +++ b/docs/concepts/output-formats.mdx @@ -44,6 +44,19 @@ mehen emits structured output in formats appropriate to each command's role. } ``` +`contributions` is omitted when the selected profile disables evidence or the analyzer has none. +Entries are deterministically ordered by source span. `amount` is the signed, weighted number of +points the source construct contributes to `metric`. For example, a SQL `DROP` contribution is: + +```json +{ + "metric": "sql.change_risk_score", + "span": { "start_byte": 0, "end_byte": 12, "start_line": 1, "end_line": 1 }, + "amount": 8.0, + "reason": "sql.change_risk.drop" +} +``` + Markdown files emit a top-level `markdown:` block with the [documentation suite](/metrics/markdown/overview). diff --git a/xtask/Cargo.toml b/xtask/Cargo.toml index 3ad04735..b3f7b1e6 100644 --- a/xtask/Cargo.toml +++ b/xtask/Cargo.toml @@ -16,6 +16,7 @@ path = "src/main.rs" [dependencies] clap = { workspace = true } askama = "^0.16" +serde_json = { workspace = true } tree-sitter = { workspace = true } # Kind-enum codegen reaches each grammar through the owning analyzer # crate's `__grammar_language()` accessor (see diff --git a/xtask/src/main.rs b/xtask/src/main.rs index 802c1ed1..943417c7 100644 --- a/xtask/src/main.rs +++ b/xtask/src/main.rs @@ -13,6 +13,7 @@ //! - `update-ruff` — Phase 6. mod antlr; +mod metric_contributions; mod tree_sitter; use clap::{Parser, Subcommand}; @@ -124,10 +125,13 @@ fn main() { } } }, - Command::AstDump { .. } - | Command::MetricContributions { .. } - | Command::AuditLicenses - | Command::UpdateRuff { .. } => { + Command::MetricContributions { path } => { + if let Err(err) = metric_contributions::run(&path) { + eprintln!("xtask metric-contributions: {err}"); + std::process::exit(1); + } + } + Command::AstDump { .. } | Command::AuditLicenses | Command::UpdateRuff { .. } => { eprintln!("xtask command not yet implemented"); std::process::exit(1); } diff --git a/xtask/src/metric_contributions.rs b/xtask/src/metric_contributions.rs new file mode 100644 index 00000000..d751a581 --- /dev/null +++ b/xtask/src/metric_contributions.rs @@ -0,0 +1,58 @@ +// SPDX-License-Identifier: AGPL-3.0-only +// Copyright (C) 2026 Konstantin Vyatkin + +use std::process::Command; + +/// Run the production analyzer through the real `mehen` binary, then print +/// only its contribution evidence. Keeping the analyzer graph behind the CLI +/// avoids making every lightweight xtask operation link all parser backends. +pub(crate) fn run(path: &str) -> Result<(), String> { + let workspace = crate::tree_sitter::workspace_root().map_err(|err| err.to_string())?; + let cargo = std::env::var_os("CARGO").unwrap_or_else(|| "cargo".into()); + let output = Command::new(cargo) + .current_dir(workspace) + .args(["run", "--quiet", "-p", "mehen", "--", "metrics"]) + .arg(path) + .args(["--profile", "default"]) + .output() + .map_err(|err| format!("failed to run mehen: {err}"))?; + + if !output.status.success() { + let stderr = String::from_utf8_lossy(&output.stderr); + return Err(format!( + "mehen metrics failed with {}: {}", + output.status, + stderr.trim() + )); + } + + let rendered = contributions_from_metrics_json(&output.stdout)?; + println!("{rendered}"); + Ok(()) +} + +fn contributions_from_metrics_json(bytes: &[u8]) -> Result { + let report: serde_json::Value = serde_json::from_slice(bytes) + .map_err(|err| format!("mehen returned invalid JSON: {err}"))?; + let contributions = report + .get("contributions") + .cloned() + .unwrap_or_else(|| serde_json::Value::Array(Vec::new())); + serde_json::to_string_pretty(&contributions) + .map_err(|err| format!("failed to render contributions: {err}")) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn extracts_contributions_or_an_empty_array() { + let present = br#"{"contributions":[{"metric":"risk","amount":8}]}"#; + let rendered = contributions_from_metrics_json(present).unwrap(); + assert!(rendered.contains("\"metric\": \"risk\"")); + + let absent = br#"{"schema_version":"1.0"}"#; + assert_eq!(contributions_from_metrics_json(absent).unwrap(), "[]"); + } +} From 01ff9a5d1591eeb956519489ff7870fb365c6c38 Mon Sep 17 00:00:00 2001 From: Konstantin Vyatkin Date: Fri, 10 Jul 2026 13:50:48 +0200 Subject: [PATCH 2/2] fix: address PR review comments Remove redundant contribution sort fields, avoid allocation when counting touched SQL objects, skip disabled span work, and resolve xtask input paths before changing directories. Addresses: https://github.com/ophi-dev/mehen/pull/176#discussion_r3558475231 Addresses: https://github.com/ophi-dev/mehen/pull/176#discussion_r3558475236 Addresses: https://github.com/ophi-dev/mehen/pull/176#discussion_r3558475246 Addresses: https://github.com/ophi-dev/mehen/pull/176#discussion_r3558475249 --- crates/mehen-core/src/analysis.rs | 4 ---- crates/mehen-sql/src/facts.rs | 39 ++++++++++++++----------------- xtask/src/metric_contributions.rs | 29 ++++++++++++++++++++++- 3 files changed, 46 insertions(+), 26 deletions(-) diff --git a/crates/mehen-core/src/analysis.rs b/crates/mehen-core/src/analysis.rs index 177406c3..aadab240 100644 --- a/crates/mehen-core/src/analysis.rs +++ b/crates/mehen-core/src/analysis.rs @@ -169,16 +169,12 @@ impl ContributionCollector { ( a.span.start_byte, a.span.end_byte, - a.span.start_line, - a.span.end_line, &a.metric, a.reason.as_str(), ) .cmp(&( b.span.start_byte, b.span.end_byte, - b.span.start_line, - b.span.end_line, &b.metric, b.reason.as_str(), )) diff --git a/crates/mehen-sql/src/facts.rs b/crates/mehen-sql/src/facts.rs index 61e89c18..ce389880 100644 --- a/crates/mehen-sql/src/facts.rs +++ b/crates/mehen-sql/src/facts.rs @@ -2049,11 +2049,11 @@ fn extract_objects( let (read_objects, write_objects) = collect_touched_objects(root, line_at, emit_contributions); obj.read_object_count = read_objects.len() as u32; obj.write_object_count = write_objects.len() as u32; - obj.touch_count = read_objects - .keys() - .chain(write_objects.keys()) - .collect::>() - .len() as u32; + obj.touch_count = (read_objects.len() + + write_objects + .keys() + .filter(|name| !read_objects.contains_key(*name)) + .count()) as u32; if emit_contributions { for span in write_objects.values() { record_change_risk( @@ -2131,22 +2131,19 @@ fn extract_objects( .filter(|w| w[0].word == "CREATE" && w[1].word == "OR" && w[2].word == "REPLACE") .collect::>(); obj.create_or_replace_count = create_or_replace.len() as u32; - for tokens in create_or_replace { - let span = match (tokens[0].span, tokens[2].span) { - (Some(start), Some(end)) => SourceSpan::new( - start.start_byte, - end.end_byte, - start.start_line, - end.end_line, - ), - _ => fallback_span, - }; - record_change_risk( - evidence, - emit_contributions, - span, - ChangeRiskFactor::CreateOrReplace, - ); + if emit_contributions { + for tokens in create_or_replace { + let span = match (tokens[0].span, tokens[2].span) { + (Some(start), Some(end)) => SourceSpan::new( + start.start_byte, + end.end_byte, + start.start_line, + end.end_line, + ), + _ => fallback_span, + }; + record_change_risk(evidence, true, span, ChangeRiskFactor::CreateOrReplace); + } } // RETURNING (Postgres/Oracle) / OUTPUT (T-SQL) DML result clauses, counted diff --git a/xtask/src/metric_contributions.rs b/xtask/src/metric_contributions.rs index d751a581..9c581d3a 100644 --- a/xtask/src/metric_contributions.rs +++ b/xtask/src/metric_contributions.rs @@ -1,18 +1,23 @@ // SPDX-License-Identifier: AGPL-3.0-only // Copyright (C) 2026 Konstantin Vyatkin +use std::path::{Path, PathBuf}; use std::process::Command; /// Run the production analyzer through the real `mehen` binary, then print /// only its contribution evidence. Keeping the analyzer graph behind the CLI /// avoids making every lightweight xtask operation link all parser backends. pub(crate) fn run(path: &str) -> Result<(), String> { + let invocation_dir = std::env::current_dir() + .map_err(|err| format!("failed to read current directory: {err}"))?; + let absolute_path = resolve_input_path_from(Path::new(path), &invocation_dir) + .map_err(|err| format!("failed to resolve path '{path}': {err}"))?; let workspace = crate::tree_sitter::workspace_root().map_err(|err| err.to_string())?; let cargo = std::env::var_os("CARGO").unwrap_or_else(|| "cargo".into()); let output = Command::new(cargo) .current_dir(workspace) .args(["run", "--quiet", "-p", "mehen", "--", "metrics"]) - .arg(path) + .arg(absolute_path) .args(["--profile", "default"]) .output() .map_err(|err| format!("failed to run mehen: {err}"))?; @@ -31,6 +36,15 @@ pub(crate) fn run(path: &str) -> Result<(), String> { Ok(()) } +fn resolve_input_path_from(path: &Path, invocation_dir: &Path) -> std::io::Result { + let path = if path.is_absolute() { + path.to_owned() + } else { + invocation_dir.join(path) + }; + path.canonicalize() +} + fn contributions_from_metrics_json(bytes: &[u8]) -> Result { let report: serde_json::Value = serde_json::from_slice(bytes) .map_err(|err| format!("mehen returned invalid JSON: {err}"))?; @@ -45,6 +59,7 @@ fn contributions_from_metrics_json(bytes: &[u8]) -> Result { #[cfg(test)] mod tests { use super::*; + use std::path::Path; #[test] fn extracts_contributions_or_an_empty_array() { @@ -55,4 +70,16 @@ mod tests { let absent = br#"{"schema_version":"1.0"}"#; assert_eq!(contributions_from_metrics_json(absent).unwrap(), "[]"); } + + #[test] + fn resolves_relative_input_from_the_invocation_directory() { + let invocation_dir = Path::new(env!("CARGO_MANIFEST_DIR")); + let resolved = resolve_input_path_from(Path::new("Cargo.toml"), invocation_dir).unwrap(); + + assert!(resolved.is_absolute()); + assert_eq!( + resolved, + invocation_dir.join("Cargo.toml").canonicalize().unwrap() + ); + } }