From bc2b9bb8b54cb5143b59a0b5fa4dfed66c232cb8 Mon Sep 17 00:00:00 2001 From: forhappy Date: Fri, 2 Oct 2026 07:50:55 -0700 Subject: [PATCH] Fix verified graph identity and clustering bugs --- CHANGELOG.md | 8 + COMPATIBILITY.md | 36 ++++ crates/compass-files/src/cache.rs | 2 +- crates/compass-graph/src/community/build.rs | 51 ++++++ .../compass-graph/src/community/hierarchy.rs | 165 +++++++++++++++--- .../src/community/incremental.rs | 27 ++- crates/compass-graph/src/community/quality.rs | 2 +- crates/compass-graph/src/v1.rs | 61 +++++-- .../tests/community_hierarchy.rs | 95 ++++++++++ .../tests/extractor_regressions.rs | 138 +++++++++++++++ crates/compass-graph/tests/html_identity.rs | 137 +++++++++++++++ .../compass-graph/tests/markdown_identity.rs | 54 ++++++ crates/compass-languages/src/bash.rs | 3 + crates/compass-languages/src/html.rs | 70 ++------ crates/compass-languages/src/markdown.rs | 16 +- crates/compass-languages/src/r.rs | 29 ++- 16 files changed, 779 insertions(+), 115 deletions(-) create mode 100644 crates/compass-graph/tests/extractor_regressions.rs create mode 100644 crates/compass-graph/tests/html_identity.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index eee21ef35..5517e3d9a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,14 @@ limits, profiles and semantic mode, with bounded disposable storage and corruption fallback. Reuse one pinned engine across CLI page widening. +- Fix six verified graph edge cases: unchanged communities are reclustered + after topology splits; hierarchy reconciliation keeps IDs unique; Markdown + and R identities preserve distinct rows, names and lexical scopes; empty + shell files publish without a zero-range entrypoint; and HTML table/link + occurrences retain their distinct identities and edge sources. Invalidate + old extraction and graph-publication caches while keeping known hierarchy + signature-v1 artifacts readable. + ## 0.4.1 - 2026-10-01 - Match engineering business vocabulary through bounded synonyms and witnessed diff --git a/COMPATIBILITY.md b/COMPATIBILITY.md index 16046c79b..3cbd2de1b 100644 --- a/COMPATIBILITY.md +++ b/COMPATIBILITY.md @@ -74,6 +74,42 @@ and AST cache semantics are unchanged. Content reads use the configured graph byte cap; this correctness correction makes no latency or memory improvement claim. +## Extractor identity and community reconciliation corrections + +The AST extraction cache identity advances from 15 to 16, and graph publication +semantics advance from compass.graph.publication/1 to /2. Existing current +builds invalidate their cached extraction/publication state and rebuild +coherently. The compass.graph/1 schema and universal evidence schema do not +change; published historical realizations remain immutable. + +An empty Bash script publishes its file inventory without a synthetic +zero-range entrypoint. R function identities now include lexical owners for +nested functions, and exact case and punctuation remain part of the function +name. Top-level R qualified names keep their existing spelling. Markdown table +row identities append a digest of the full, exact identity key to the readable +qualified-name prefix; row and cell IDs remain stable across line shifts and +non-identity edits. Existing Markdown table-row and cell IDs change once on +rebuild. + +HTML table and row nodes use source-occurrence identity, so equal visible text +does not merge different elements. Local HTML references originate at their + or node and point to the uniquely resolved target. A link to its +own fragment remains part of the link count but does not publish a self-loop. +Existing HTML table and row IDs change once on rebuild. + +Incremental community clustering rechecks whether each prior community remains +connected in the current topology, even when its source files are unchanged. +Disconnected prior communities join the bounded affected region and are +reclustered; the existing affected-region limit can still select the full +detector fallback. + +Hierarchy reconciliation uses hierarchy-signature/v2. If a fresh group ID +collides with an inherited ID, it receives the first available deterministic +digest ID within a bounded retry count, and reconciliation events name the +final IDs. Known hierarchy-signature/v1 artifacts remain readable. The +hierarchy schema stays at /1, and existing historical sidecars are not +rewritten. + ## Go struct field declarations The universal Go producer now publishes one `field` declaration per explicitly diff --git a/crates/compass-files/src/cache.rs b/crates/compass-files/src/cache.rs index 7cf228ba4..aafccebca 100644 --- a/crates/compass-files/src/cache.rs +++ b/crates/compass-files/src/cache.rs @@ -13,7 +13,7 @@ use sha2::{Digest, Sha256}; use crate::{FileError, StatHashIndex, file_hash, io_error, write_bytes_atomic, write_json_atomic}; /// Changes whenever cached extraction semantics change, even if the wire encoding does not. -pub const AST_CACHE_VERSION: &str = "15"; +pub const AST_CACHE_VERSION: &str = "16"; /// Portable cache encoding version used in the on-disk namespace. pub const CACHE_ENCODING_VERSION: u32 = 1; const MESSAGEPACK_EXTENSION: &str = "msgpack"; diff --git a/crates/compass-graph/src/community/build.rs b/crates/compass-graph/src/community/build.rs index d8418f00c..21bb14adc 100644 --- a/crates/compass-graph/src/community/build.rs +++ b/crates/compass-graph/src/community/build.rs @@ -1022,6 +1022,57 @@ mod tests { Ok(()) } + #[test] + fn quality_incremental_run_reclusters_a_prior_community_split_by_unchanged_nodes() + -> Result<(), Box> { + let document = planted_graph(); + let no_changes = BTreeSet::new(); + let initial = build_communities( + &document, + &CommunityRequest { + profile: CommunityProfile::QualityV1, + resolution: ResolutionPolicy::Fixed(1.0), + exclude_hubs_percentile: None, + previous: None, + incremental: false, + changed_sources: &no_changes, + limits: CommunityLimits::default(), + }, + )?; + let previous = previous_from(&initial); + assert_eq!( + canonical_memberships(&initial.communities), + [["a", "b", "c", "d"], ["w", "x", "y", "z"]] + ); + + let mut split = document; + split.links.retain(|edge| { + !matches!( + (edge.source.as_str(), edge.target.as_str()), + ("a", "c") | ("a", "d") | ("b", "c") | ("b", "d") + ) + }); + let mut limits = CommunityLimits::default(); + limits.incremental.max_affected_fraction = 1.0; + for changed_sources in [BTreeSet::new(), BTreeSet::from(["src/z.rs".to_owned()])] { + let request = CommunityRequest { + profile: CommunityProfile::QualityV1, + resolution: ResolutionPolicy::Fixed(1.0), + exclude_hubs_percentile: None, + previous: Some(&previous), + incremental: true, + changed_sources: &changed_sources, + limits, + }; + let first = build_communities(&split, &request)?; + let second = build_communities(&split, &request)?; + assert_eq!(first.quality.disconnected_community_count, 0); + assert_eq!(first.quality.assigned_node_count, split.nodes.len()); + assert_eq!(first.communities, second.communities); + } + Ok(()) + } + #[test] fn quality_profile_recovers_planted_groups_and_is_permutation_invariant() -> Result<(), Box> { diff --git a/crates/compass-graph/src/community/hierarchy.rs b/crates/compass-graph/src/community/hierarchy.rs index c05d318b8..90144f762 100644 --- a/crates/compass-graph/src/community/hierarchy.rs +++ b/crates/compass-graph/src/community/hierarchy.rs @@ -43,10 +43,12 @@ pub const COMMUNITY_HIERARCHY_BUDGET: &str = "community-hierarchy-budget/v1"; pub const COMMUNITY_HIERARCHY_MERGE_POLICY: &str = "relationship-then-location-affinity/v1"; /// Identity of the rule that derives group signatures and durable ids. -pub const COMMUNITY_HIERARCHY_SIGNATURE_ALGORITHM: &str = "hierarchy-signature/v1"; +pub const COMMUNITY_HIERARCHY_SIGNATURE_ALGORITHM: &str = "hierarchy-signature/v2"; +const LEGACY_COMMUNITY_HIERARCHY_SIGNATURE_ALGORITHM: &str = "hierarchy-signature/v1"; /// Length of a group signature in hex characters. const GROUP_SIGNATURE_LENGTH: usize = 16; +const GROUP_ID_DISAMBIGUATION_ATTEMPTS: usize = 1_024; /// Root groups a hierarchy aims for. pub const DEFAULT_ROOT_TARGET: usize = 24; @@ -446,7 +448,9 @@ impl CommunityHierarchy { if self.merge_policy != COMMUNITY_HIERARCHY_MERGE_POLICY { return Err(invalid("unexpected merge policy")); } - if self.signature_algorithm != COMMUNITY_HIERARCHY_SIGNATURE_ALGORITHM { + if self.signature_algorithm != COMMUNITY_HIERARCHY_SIGNATURE_ALGORITHM + && self.signature_algorithm != LEGACY_COMMUNITY_HIERARCHY_SIGNATURE_ALGORITHM + { return Err(invalid("unexpected signature algorithm")); } if self.budget.max_levels == 0 @@ -1067,7 +1071,7 @@ pub fn reconcile_hierarchy( .iter() .map(|group| group.member_count) .collect::>(); - let next_ids = next.levels[position] + let mut next_ids = next.levels[position] .groups .iter() .map(|group| group.id.clone()) @@ -1116,9 +1120,9 @@ pub fn reconcile_hierarchy( if predecessors.get(*next_index).map_or(0, Vec::len) > 1 { continue; } - let Some(next_id) = next_ids.get(*next_index) else { + if next_ids.get(*next_index).is_none() { continue; - }; + } if position > 0 && let (Some(previous_parent), Some(next_parent)) = ( previous_parent_of(previous_index), @@ -1135,33 +1139,48 @@ pub fn reconcile_hierarchy( { group.id = previous_id.clone(); } - let _ = next_id; matched_here.insert(previous_index, *next_index); stable += 1; } - [(best, best_index), (runner_up, _), ..] => { + [(best, _), (runner_up, _), ..] => { // Two successors this close mean the evidence does not name // a single heir, so no id is inherited. if best - runner_up <= policy.ambiguity_margin { ambiguous += 1; - events.push(HierarchyEvent { - kind: HierarchyEventKind::Ambiguous, - level: next_level_number, - previous_ids: vec![previous_id.clone()], - next_ids: row - .iter() - .filter_map(|(_, next_index)| next_ids.get(*next_index).cloned()) - .collect(), - overlap: *best, - member_count: next_members - .get(*best_index) - .copied() - .unwrap_or_default(), - }); } } } } + let claimed = matched_here.values().copied().collect::>(); + disambiguate_reconciled_group_ids( + next, + position, + next_level_number, + &mut next_ids, + &claimed, + )?; + for (previous_index, row) in rows.iter().enumerate() { + let [(best, best_index), (runner_up, _), ..] = row.as_slice() else { + continue; + }; + if best - runner_up > policy.ambiguity_margin { + continue; + } + let Some(previous_id) = previous_ids.get(previous_index) else { + continue; + }; + events.push(HierarchyEvent { + kind: HierarchyEventKind::Ambiguous, + level: next_level_number, + previous_ids: vec![previous_id.clone()], + next_ids: row + .iter() + .filter_map(|(_, next_index)| next_ids.get(*next_index).cloned()) + .collect(), + overlap: *best, + member_count: next_members.get(*best_index).copied().unwrap_or_default(), + }); + } for (previous_index, row) in rows.iter().enumerate() { if row.len() < 2 { continue; @@ -1230,7 +1249,6 @@ pub fn reconcile_hierarchy( }); } } - let claimed = matched_here.values().copied().collect::>(); for (next_index, next_id) in next_ids.iter().enumerate() { if claimed.contains(&next_index) || !predecessors[next_index].is_empty() { continue; @@ -1286,6 +1304,100 @@ pub fn reconcile_hierarchy( }) } +fn disambiguate_reconciled_group_ids( + next: &mut CommunityHierarchy, + level_index: usize, + level_number: usize, + next_ids: &mut [String], + retained_indices: &BTreeSet, +) -> Result<(), CommunityHierarchyArtifactError> { + let mut occupied = BTreeSet::::new(); + for index in retained_indices { + let id = next + .levels + .get(level_index) + .and_then(|level| level.groups.get(*index)) + .map(|group| group.id.clone()) + .ok_or_else(|| { + CommunityHierarchyArtifactError::InvalidEvidence( + "retained group is missing from its level".to_owned(), + ) + })?; + if !occupied.insert(id.clone()) { + return Err(CommunityHierarchyArtifactError::InvalidEvidence( + "retained group ids are not unique within a level".to_owned(), + )); + } + let Some(slot) = next_ids.get_mut(*index) else { + return Err(CommunityHierarchyArtifactError::InvalidEvidence( + "retained group id is missing its level entry".to_owned(), + )); + }; + slot.clone_from(&id); + } + + let mut collisions = Vec::new(); + for (index, id) in next_ids.iter().enumerate() { + if retained_indices.contains(&index) { + continue; + } + if !occupied.insert(id.clone()) { + collisions.push(index); + } + } + + for index in collisions { + let signature = next + .levels + .get(level_index) + .and_then(|level| level.groups.get(index)) + .map(|group| group.signature.clone()) + .ok_or_else(|| { + CommunityHierarchyArtifactError::InvalidEvidence( + "colliding group is missing its signature".to_owned(), + ) + })?; + let mut replacement = None; + for attempt in 0..GROUP_ID_DISAMBIGUATION_ATTEMPTS { + let seed = format!( + "{COMMUNITY_HIERARCHY_SIGNATURE_ALGORITHM}:collision:{level_number}:{signature}:{attempt}" + ); + let digest = format!("{:x}", Sha256::digest(seed.as_bytes())); + let suffix = digest + .chars() + .take(GROUP_SIGNATURE_LENGTH) + .collect::(); + let candidate = format!("h{level_number}-{suffix}"); + if occupied.insert(candidate.clone()) { + replacement = Some(candidate); + break; + } + } + let Some(id) = replacement else { + return Err(CommunityHierarchyArtifactError::InvalidEvidence( + "bounded group id collision disambiguation was exhausted".to_owned(), + )); + }; + let Some(slot) = next_ids.get_mut(index) else { + return Err(CommunityHierarchyArtifactError::InvalidEvidence( + "colliding group id is missing its level entry".to_owned(), + )); + }; + slot.clone_from(&id); + let Some(group) = next + .levels + .get_mut(level_index) + .and_then(|level| level.groups.get_mut(index)) + else { + return Err(CommunityHierarchyArtifactError::InvalidEvidence( + "colliding group is missing from its level".to_owned(), + )); + }; + group.id = id; + } + Ok(()) +} + struct LevelState { /// The graph this level partitions: the typed projection for the finest /// level, and the previous level's group graph above it. @@ -2604,6 +2716,15 @@ mod tests { Ok(()) } + #[test] + fn artifact_accepts_the_previous_known_signature_algorithm() -> TestResult { + let mut artifact = artifact(4, HierarchyBudget::default())?; + artifact.signature_algorithm = LEGACY_COMMUNITY_HIERARCHY_SIGNATURE_ALGORITHM.to_owned(); + artifact.result_digest = artifact.calculate_digest()?; + artifact.validate()?; + Ok(()) + } + #[test] fn artifact_rejects_unknown_major_and_wrong_graph() -> TestResult { let mut artifact = artifact(2, HierarchyBudget::default())?; diff --git a/crates/compass-graph/src/community/incremental.rs b/crates/compass-graph/src/community/incremental.rs index 990942490..9c0e6f4f9 100644 --- a/crates/compass-graph/src/community/incremental.rs +++ b/crates/compass-graph/src/community/incremental.rs @@ -3,6 +3,7 @@ use std::collections::{BTreeMap, BTreeSet, HashMap}; use compass_model::code_graph::GraphDocument; use super::leiden::{CommunityDetectorError, leiden_anchored}; +use super::quality::connected_component_count; use crate::cluster::{Communities, IncrementalClusterLimits, WeightedGraph}; #[derive(Clone, Copy, Debug, Eq, PartialEq)] @@ -150,6 +151,21 @@ pub(crate) fn prepare_anchored_topology( ) }) .collect::>(); + let mut previous_members = BTreeMap::>::new(); + for (position, id) in graph.ids.iter().enumerate() { + if let Some(community) = previous.get(id) { + previous_members + .entry(*community) + .or_default() + .insert(position); + } + } + let mut touched = previous_members + .iter() + .filter_map(|(community, members)| { + (connected_component_count(graph, members) > 1).then_some(*community) + }) + .collect::>(); let mut affected = graph .ids .iter() @@ -162,13 +178,14 @@ pub(crate) fn prepare_anchored_topology( (!previous.contains_key(id) || changed).then_some(position) }) .collect::>(); - if affected.is_empty() { + touched.extend( + affected + .iter() + .filter_map(|position| previous.get(&graph.ids[*position]).copied()), + ); + if affected.is_empty() && touched.is_empty() { return IncrementalPreparation::Unchanged(communities_from_previous(graph, previous)); } - let touched = affected - .iter() - .filter_map(|position| previous.get(&graph.ids[*position]).copied()) - .collect::>(); for (position, id) in graph.ids.iter().enumerate() { if previous .get(id) diff --git a/crates/compass-graph/src/community/quality.rs b/crates/compass-graph/src/community/quality.rs index 0813f3c51..a68884953 100644 --- a/crates/compass-graph/src/community/quality.rs +++ b/crates/compass-graph/src/community/quality.rs @@ -680,7 +680,7 @@ fn combination_two(count: usize) -> f64 { count.saturating_mul(count.saturating_sub(1)) as f64 / 2.0 } -fn connected_component_count(graph: &WeightedGraph, members: &BTreeSet) -> usize { +pub(super) fn connected_component_count(graph: &WeightedGraph, members: &BTreeSet) -> usize { let mut remaining = members.clone(); let mut components = 0usize; while let Some(start) = remaining.pop_first() { diff --git a/crates/compass-graph/src/v1.rs b/crates/compass-graph/src/v1.rs index 1faaf1b0f..b11ba4f8a 100644 --- a/crates/compass-graph/src/v1.rs +++ b/crates/compass-graph/src/v1.rs @@ -38,7 +38,7 @@ use crate::inference::{InferenceLevel, prefilter_extraction_inference}; use crate::quarantine::{PublicationOutcome, QuarantineCollector}; /// Normalization/publication semantics used by `compass.graph/1`. -pub const V1_PUBLICATION_SEMANTICS_VERSION: &str = "compass.graph.publication/1"; +pub const V1_PUBLICATION_SEMANTICS_VERSION: &str = "compass.graph.publication/2"; const MAX_DOCUMENT_REFERENCE_CANDIDATES: usize = 20; const MAX_DOCUMENT_REFERENCE_PROBES: usize = 100_000; use sha2::{Digest, Sha256}; @@ -1991,6 +1991,21 @@ fn ensure_external_placeholder_details(nodes: &mut [NodeRecord]) { fn normalize_trusted_node(value: Value, raw_id: &str) -> Result { let mut node = serde_json::from_value::(value) .map_err(|error| raw_error(raw_id, &error.to_string()))?; + if node.kind == NodeKind::Resource + && matches!( + node.details.as_ref(), + Some(NodeDetails::Resource(ResourceNodeDetails { + resource_kind: ResourceKind::Document, + media_type: Some(media_type), + .. + })) if media_type == "text/html" + ) + { + // HTML block identity is occurrence-based. The trusted graph record + // already carries the validated ID selected from that occurrence, and + // the resource projection no longer carries its original document role. + return Ok(node); + } // Trusted records already carry typed semantics. Recompute document IDs // from the same semantic/occurrence rules used for raw normalization so a // legacy producer's global document ID cannot collapse repeated blocks. @@ -1998,21 +2013,24 @@ fn normalize_trusted_node(value: Value, raw_id: &str) -> Result Some(matches!( - details.role, - DocumentRole::Document - | DocumentRole::Heading - | DocumentRole::Table - | DocumentRole::TableRow - )), + Some(NodeDetails::Document(details)) => Some( + details.role == DocumentRole::Document + || (details.format == DocumentFormat::Markdown + && matches!( + details.role, + DocumentRole::Heading | DocumentRole::Table | DocumentRole::TableRow + )), + ), Some(NodeDetails::Resource(ResourceNodeDetails { resource_kind: ResourceKind::Document, uri, + media_type, .. })) => Some( - uri.as_deref().is_some_and(|value| value.starts_with('#')) - || graph_v1_table_qualified_name(&node.qualified_name) - || (site.start_byte == 0 && node.name == node.qualified_name), + media_type.as_deref() != Some("text/html") + && (uri.as_deref().is_some_and(|value| value.starts_with('#')) + || graph_v1_table_qualified_name(&node.qualified_name) + || (site.start_byte == 0 && node.name == node.qualified_name)), ), _ => None, }; @@ -5585,7 +5603,14 @@ fn node_identity( let semantic = matches!( details, Some(NodeDetails::Document(DocumentNodeDetails { - role: DocumentRole::Document | DocumentRole::Table | DocumentRole::TableRow, + role: DocumentRole::Document, + .. + })) + ) || matches!( + details, + Some(NodeDetails::Document(DocumentNodeDetails { + format: DocumentFormat::Markdown, + role: DocumentRole::Heading | DocumentRole::Table | DocumentRole::TableRow, .. })) ) || raw_markdown_table_structure(attributes); @@ -5723,11 +5748,13 @@ fn raw_markdown_heading(attributes: &Map) -> bool { } fn raw_markdown_table_structure(attributes: &Map) -> bool { - matches!( - optional_string(attributes, "document_kind").as_deref(), - Some("pipe_table" | "pipe_table_header" | "pipe_table_row" | "pipe_table_cell") - ) && optional_string(attributes, "qualified_name") - .is_some_and(|name| name.contains("::pipe_table")) + document_format(attributes) == DocumentFormat::Markdown + && matches!( + optional_string(attributes, "document_kind").as_deref(), + Some("pipe_table" | "pipe_table_header" | "pipe_table_row" | "pipe_table_cell") + ) + && optional_string(attributes, "qualified_name") + .is_some_and(|name| name.contains("::pipe_table")) } fn raw_anchor( diff --git a/crates/compass-graph/tests/community_hierarchy.rs b/crates/compass-graph/tests/community_hierarchy.rs index 5ef24249b..1b0f5c4af 100644 --- a/crates/compass-graph/tests/community_hierarchy.rs +++ b/crates/compass-graph/tests/community_hierarchy.rs @@ -1013,6 +1013,101 @@ fn an_edit_sequence_reports_appeared_and_disappeared_groups() -> TestResult { Ok(()) } +#[test] +fn reconciliation_does_not_reuse_a_retained_id_for_reappearing_membership() -> TestResult { + let budget = HierarchyBudget { + max_levels: 1, + ..HierarchyBudget::default() + }; + let single = |size| { + Communities::from([( + 0usize, + (0..size) + .map(|index| format!("cluster0_symbol{index}")) + .collect::>(), + )]) + }; + let mut previous_communities = single(2); + let mut previous = hierarchy_of( + &clustered_document(1, 2, false), + &previous_communities, + 1.0, + budget, + )?; + let original_id = previous.levels[0].groups[0].id.clone(); + for size in [4, 8] { + let next_communities = single(size); + let mut next = hierarchy_of( + &clustered_document(1, size, false), + &next_communities, + 1.0, + budget, + )?; + reconcile_hierarchy( + &previous, + &previous_communities, + &mut next, + &next_communities, + &ReconcilePolicy::default(), + )?; + assert_eq!(next.levels[0].groups[0].id, original_id); + previous = next; + previous_communities = next_communities; + } + + let mut initial_members = single(2); + let split = Communities::from([ + ( + 0usize, + initial_members + .remove(&0) + .ok_or("missing initial members")?, + ), + ( + 1usize, + (2..8) + .map(|index| format!("cluster0_symbol{index}")) + .collect::>(), + ), + ]); + let document = clustered_document(1, 8, false); + let mut next = hierarchy_of(&document, &split, 1.0, budget)?; + let report = reconcile_hierarchy( + &previous, + &previous_communities, + &mut next, + &split, + &ReconcilePolicy::default(), + )?; + next.validate()?; + assert_eq!(report.stable, 1); + assert_eq!(next.levels[0].groups[1].id, original_id); + assert_ne!(next.levels[0].groups[0].id, original_id); + let final_ids = next.levels[0] + .groups + .iter() + .map(|group| group.id.as_str()) + .collect::>(); + assert!( + report + .events + .iter() + .flat_map(|event| &event.next_ids) + .all(|id| { final_ids.contains(id.as_str()) }) + ); + + let mut repeated = hierarchy_of(&document, &split, 1.0, budget)?; + reconcile_hierarchy( + &previous, + &previous_communities, + &mut repeated, + &split, + &ReconcilePolicy::default(), + )?; + assert_eq!(serde_json::to_vec(&next)?, serde_json::to_vec(&repeated)?); + Ok(()) +} + #[test] fn limits_fail_with_a_typed_error_instead_of_truncating() -> TestResult { let document = clustered_document(4, 3, true); diff --git a/crates/compass-graph/tests/extractor_regressions.rs b/crates/compass-graph/tests/extractor_regressions.rs new file mode 100644 index 000000000..cfe76c771 --- /dev/null +++ b/crates/compass-graph/tests/extractor_regressions.rs @@ -0,0 +1,138 @@ +use std::collections::BTreeMap; +use std::error::Error; +use std::fs; + +use compass_graph::{build_from_extraction, normalize_document_v1}; +use compass_languages::Engine; +use compass_model::code_graph::{EdgeKind, NodeKind}; + +type TestResult = Result<(), Box>; + +fn publish( + path: &std::path::Path, + root: &std::path::Path, +) -> Result> { + let extraction = Engine::default().extract(path)?; + let flexible = build_from_extraction(&extraction, true, Some(root)); + Ok(normalize_document_v1(&flexible, root, "sha256:test", None)?) +} + +fn has_no_publication_diagnostics(graph: &compass_model::code_graph::GraphDocument) { + assert!( + graph + .graph + .diagnostics + .iter() + .all(|diagnostic| { !diagnostic.code.starts_with("publication_") }) + ); +} + +#[test] +fn empty_bash_script_publishes_inventory_only_and_nonempty_scripts_keep_calls() -> TestResult { + let directory = tempfile::tempdir()?; + let root = directory.path(); + let path = root.join("script.sh"); + fs::write(&path, "")?; + + let graph = publish(&path, root)?; + has_no_publication_diagnostics(&graph); + assert_eq!(graph.nodes.len(), 1); + assert_eq!(graph.nodes[0].kind, NodeKind::File); + assert!(graph.links.is_empty()); + + fs::write(&path, "hello() { echo hello; }\nhello\n")?; + let graph = publish(&path, root)?; + has_no_publication_diagnostics(&graph); + let function = graph + .nodes + .iter() + .find(|node| node.kind == NodeKind::Function && node.name == "hello()") + .ok_or("non-empty script function was not published")?; + assert!( + graph + .links + .iter() + .any(|edge| { edge.kind == EdgeKind::Calls && edge.target == function.id }) + ); + Ok(()) +} + +#[test] +fn r_nested_function_identity_preserves_exact_names_scopes_calls_and_stability() -> TestResult { + let directory = tempfile::tempdir()?; + let root = directory.path(); + let path = root.join("helpers.R"); + let source = r#"left <- function() { + helper <- function() { 1 } + helper() +} +right <- function() { + helper <- function() { 2 } + helper() +} +outer <- function() { + Foo <- function() { 3 } + foo <- function() { 4 } + format.result <- function() { 5 } + format_result <- function() { 6 } + Foo() + foo() + format.result() + format_result() +} +"#; + + let graph_for = |contents: &str| -> Result<_, Box> { + fs::write(&path, contents)?; + let graph = publish(&path, root)?; + has_no_publication_diagnostics(&graph); + let functions = graph + .nodes + .iter() + .filter(|node| node.kind == NodeKind::Function) + .map(|node| (node.qualified_name.clone(), node.id.clone())) + .collect::>(); + assert_eq!(functions.len(), 9); + Ok((graph, functions)) + }; + + let (graph, functions) = graph_for(source)?; + for qualified_name in [ + "left()", + "left()::helper()", + "right()", + "right()::helper()", + "outer()", + "outer()::Foo()", + "outer()::foo()", + "outer()::format.result()", + "outer()::format_result()", + ] { + assert!( + functions.contains_key(qualified_name), + "missing {qualified_name}" + ); + } + for (caller, callee) in [ + ("left()", "left()::helper()"), + ("right()", "right()::helper()"), + ("outer()", "outer()::Foo()"), + ("outer()", "outer()::foo()"), + ("outer()", "outer()::format.result()"), + ("outer()", "outer()::format_result()"), + ] { + assert!( + graph.links.iter().any(|edge| { + edge.kind == EdgeKind::Calls + && edge.source == functions[caller] + && edge.target == functions[callee] + }), + "missing exact call {caller} -> {callee}" + ); + } + + let shifted = format!("# stable identity\n{source}"); + let (_, shifted_functions) = graph_for(&shifted)?; + assert_eq!(shifted_functions, functions); + Ok(()) +} diff --git a/crates/compass-graph/tests/html_identity.rs b/crates/compass-graph/tests/html_identity.rs new file mode 100644 index 000000000..3ddaec266 --- /dev/null +++ b/crates/compass-graph/tests/html_identity.rs @@ -0,0 +1,137 @@ +use std::error::Error; +use std::fs; + +use compass_graph::{build_from_extraction, extraction_from_v1, normalize_document_v1}; +use compass_languages::Engine; +use compass_model::code_graph::{EdgeKind, NodeKind}; + +type TestResult = Result<(), Box>; + +fn extract_and_publish( + path: &std::path::Path, + root: &std::path::Path, + source: &str, +) -> Result< + ( + compass_languages::Extraction, + compass_model::code_graph::GraphDocument, + ), + Box, +> { + fs::write(path, source)?; + let extraction = Engine::default().extract(path)?; + let flexible = build_from_extraction(&extraction, true, Some(root)); + let graph = normalize_document_v1(&flexible, root, "sha256:test", None)?; + Ok((extraction, graph)) +} + +fn assert_no_publication_diagnostics(graph: &compass_model::code_graph::GraphDocument) { + assert!( + graph + .graph + .diagnostics + .iter() + .all(|diagnostic| { !diagnostic.code.starts_with("publication_") }) + ); +} + +#[test] +fn html_tables_with_equal_text_keep_every_occurrence() -> TestResult { + let directory = tempfile::tempdir()?; + let root = directory.path(); + let path = root.join("index.html"); + for source in [ + "
NameStatus
", + "
same
same
", + "
NameStatus
", + ] { + let (extraction, graph) = extract_and_publish(&path, root, source)?; + assert_no_publication_diagnostics(&graph); + assert_eq!(graph.nodes.len(), extraction.nodes.len()); + assert_eq!(graph.links.len(), extraction.edges.len()); + let mut ids = graph + .nodes + .iter() + .map(|node| node.id.as_str()) + .collect::>(); + ids.sort_unstable(); + ids.dedup(); + assert_eq!(ids.len(), extraction.nodes.len()); + } + + let (extraction, graph) = extract_and_publish( + &path, + root, + "
NameStatus
", + )?; + let repeated = extraction_from_v1(&graph); + let repeated = build_from_extraction(&repeated, true, Some(root)); + let repeated = normalize_document_v1(&repeated, root, "sha256:test", None)?; + assert_eq!( + repeated + .nodes + .iter() + .map(|node| node.id.clone()) + .collect::>(), + graph + .nodes + .iter() + .map(|node| node.id.clone()) + .collect::>() + ); + assert_eq!(extraction.nodes.len(), graph.nodes.len()); + Ok(()) +} + +#[test] +fn html_permalink_references_start_at_the_link_occurrence_and_skip_self_links() -> TestResult { + let directory = tempfile::tempdir()?; + let root = directory.path(); + let path = root.join("index.html"); + let source = "

Title##

self"; + let (_, graph) = extract_and_publish(&path, root, source)?; + assert_no_publication_diagnostics(&graph); + + let heading = graph + .nodes + .iter() + .find(|node| { + node.source.as_ref().is_some_and(|anchor| { + source + .find(" Result<_, Box> { + fs::write(&path, contents)?; + let extraction = Engine::default().extract(&path)?; + let flexible = build_from_extraction(&extraction, true, Some(root)); + let graph = normalize_document_v1(&flexible, root, "sha256:test", None)?; + assert!( + graph + .graph + .diagnostics + .iter() + .all(|diagnostic| { !diagnostic.code.starts_with("publication_") }) + ); + Ok(graph) + }; + let identities_for = |contents: &str| -> Result, Box> { + let graph = graph_for(contents)?; + let rows = graph + .nodes + .iter() + .filter(|node| node.qualified_name.contains("::pipe_table_row#")) + .map(|node| (node.qualified_name.clone(), node.id.clone())) + .collect::>(); + assert_eq!(rows.len(), 6, "both rows and all four cells must survive"); + Ok(rows) + }; + + let before = identities_for(&source)?; + let shifted = format!( + "Introductory prose.\n\n{}", + source.replace("active", "planned") + ); + assert_eq!(identities_for(&shifted)?, before); + } + Ok(()) +} + #[test] fn markdown_document_references_resolve_only_unique_exact_targets() -> Result<(), Box> { let directory = tempfile::tempdir()?; diff --git a/crates/compass-languages/src/bash.rs b/crates/compass-languages/src/bash.rs index 93c5f6a1d..d7b37d284 100644 --- a/crates/compass-languages/src/bash.rs +++ b/crates/compass-languages/src/bash.rs @@ -54,6 +54,9 @@ impl<'source, 'tree> BashState<'source, 'tree> { .and_then(|name| name.to_str()) .unwrap_or_default(); self.add_node(&self.file_id.clone(), label, 1, "file"); + if self.source.is_empty() { + return self.extraction; + } self.add_node( &self.entry_id.clone(), &format!("{label} script"), diff --git a/crates/compass-languages/src/html.rs b/crates/compass-languages/src/html.rs index d8a1e9fb6..14228debf 100644 --- a/crates/compass-languages/src/html.rs +++ b/crates/compass-languages/src/html.rs @@ -108,6 +108,7 @@ pub(crate) fn extract_source( unresolved_links: Vec::new(), external_links: Vec::new(), diagnostics: Vec::new(), + self_link_count: 0, next_index: 1, hidden_depth: 0, base_href: None, @@ -147,6 +148,7 @@ struct State<'source, 'path> { unresolved_links: Vec, external_links: Vec, diagnostics: Vec, + self_link_count: usize, next_index: usize, hidden_depth: usize, base_href: Option, @@ -304,9 +306,9 @@ impl State<'_, '_> { self.title = bounded_string(&label); } if tag == "a" { - self.add_pending_link(&attributes, node, "anchor", parent); + self.add_pending_link(&attributes, node, "anchor", &id); } else if tag == "link" { - self.add_pending_link(&attributes, node, "link", parent); + self.add_pending_link(&attributes, node, "link", &id); } self.collect_metadata(&tag, &attributes); if tag == "br" { @@ -376,7 +378,7 @@ impl State<'_, '_> { attributes: &Map, node: Node<'_>, kind: &'static str, - parent: Option<&str>, + owner_id: &str, ) { let Some(raw) = attributes .get("href") @@ -389,10 +391,9 @@ impl State<'_, '_> { self.add_diagnostic("HTML link limit exceeded"); return; } - let owner_id = self.containing_owner(parent); self.pending_links.push(PendingLink { raw: raw.to_owned(), - owner_id, + owner_id: owner_id.to_owned(), site: self.link_site(node.start_byte(), node.end_byte()), kind, rel: attributes @@ -402,42 +403,6 @@ impl State<'_, '_> { }); } - fn containing_owner(&self, parent: Option<&str>) -> String { - let mut candidate = parent.map(str::to_owned); - let mut visited = HashSet::new(); - while let Some(id) = candidate { - if !visited.insert(id.clone()) { - break; - } - if id == self.file_id { - return id; - } - if self - .extraction - .nodes - .iter() - .find(|node| node.id == id) - .and_then(|node| node.attributes.get("document_kind")) - .and_then(Value::as_str) - .is_some_and(is_html_block_kind) - { - return id; - } - candidate = self - .extraction - .edges - .iter() - .rev() - .find(|edge| { - edge.target == id - && edge.attributes.get("relation").and_then(Value::as_str) - == Some("contains") - }) - .map(|edge| edge.source.clone()); - } - self.file_id.clone() - } - fn collect_metadata(&mut self, tag: &str, attributes: &Map) { if tag == "meta" { let key = attributes @@ -513,6 +478,7 @@ impl State<'_, '_> { .count() .saturating_add(self.external_links.len()) .saturating_add(self.unresolved_links.len()) + .saturating_add(self.self_link_count) ), ); if !self.diagnostics.is_empty() { @@ -600,7 +566,11 @@ impl State<'_, '_> { let key = fragment.to_ascii_lowercase(); match self.anchor_targets.get(&key) { Some(candidates) if candidates.len() == 1 => { - self.add_link_edge(&link, candidates[0].clone(), Some(fragment)); + if candidates[0] == link.owner_id { + self.self_link_count = self.self_link_count.saturating_add(1); + } else { + self.add_link_edge(&link, candidates[0].clone(), Some(fragment)); + } } Some(_) => self.add_unresolved(&link, "ambiguous_fragment", fragment), None => self.add_unresolved(&link, "missing_fragment", fragment), @@ -1148,22 +1118,6 @@ fn html_kind(tag: &str) -> &'static str { } } -fn is_html_block_kind(kind: &str) -> bool { - matches!( - kind, - "heading" - | "paragraph" - | "landmark" - | "list" - | "list_item" - | "blockquote" - | "preformatted" - | "table" - | "table_row" - | "table_cell" - ) -} - fn heading_level(tag: &str) -> Option { match tag { "h1" => Some(1), diff --git a/crates/compass-languages/src/markdown.rs b/crates/compass-languages/src/markdown.rs index ae23ab7d6..ed5c5002b 100644 --- a/crates/compass-languages/src/markdown.rs +++ b/crates/compass-languages/src/markdown.rs @@ -4,6 +4,7 @@ use std::path::{Component, Path, PathBuf}; use crate::facts::stamp_source_range; use crate::{RawEdgeRecord as EdgeRecord, RawNodeRecord as NodeRecord}; use serde_json::{Map, Value, json}; +use sha2::{Digest, Sha256}; use tree_sitter::{Node, Parser}; use tree_sitter_language_pack::{DataNode, DataNodeKind, ProcessConfig}; use tree_sitter_md::{INLINE_LANGUAGE, LANGUAGE}; @@ -412,12 +413,13 @@ fn compact_identity(value: &str) -> String { output.push('-'); } } - let trimmed = output.trim_matches('-'); - if trimmed.is_empty() { + let readable = if output.trim_matches('-').is_empty() { "row".to_owned() } else { - trimmed.to_owned() - } + output.trim_matches('-').to_owned() + }; + let digest = format!("{:x}", Sha256::digest(value.as_bytes())); + format!("{readable}-{digest}") } fn truncate_utf8(text: &str, limit: usize) -> String { @@ -1188,15 +1190,15 @@ impl State<'_, '_> { ); let identity_occurrence = row_occurrences.entry(identity_key.clone()).or_default(); *identity_occurrence = identity_occurrence.saturating_add(1); + let compact_key = compact_identity(&identity_key); let row_qualified_name = format!( "{table_qualified_name}::pipe_table_row#{}-{}", - compact_identity(&identity_key), - *identity_occurrence + compact_key, *identity_occurrence ); let row_id = crate::make_id(&[ &table_id, "markdown_table_row", - &identity_key, + &compact_key, &identity_occurrence.to_string(), ]); let mut row_extra = Map::new(); diff --git a/crates/compass-languages/src/r.rs b/crates/compass-languages/src/r.rs index dd79bd190..8dc0acf44 100644 --- a/crates/compass-languages/src/r.rs +++ b/crates/compass-languages/src/r.rs @@ -7,6 +7,7 @@ use serde_json::{Map, Value}; use crate::facts::source_range; use crate::{Extraction, RawCall, file_stem, make_id}; +use sha2::{Digest, Sha256}; const NON_CALLS: &[&str] = &[ "function", "if", "for", "while", "repeat", "switch", "return", @@ -22,6 +23,7 @@ struct Function { end: usize, id: String, name: String, + qualified_name: String, parent: Option, } @@ -59,13 +61,18 @@ impl<'a> State<'a> { .file_name() .and_then(|name| name.to_str()) .unwrap_or_default(); - self.add_node(&self.file_id.clone(), label, 1); + self.add_node(&self.file_id.clone(), label, label, 1); self.add_imports(); let functions = self.functions(); for function in &functions { let at = self.line_at(function.start); - self.add_node(&function.id, &format!("{}()", function.name), at); + self.add_node( + &function.id, + &format!("{}()", function.name), + &function.qualified_name, + at, + ); let container = function .parent .as_deref() @@ -168,12 +175,22 @@ impl<'a> State<'a> { .filter(|function| function.start < start && end <= function.end) .min_by_key(|function| function.end - function.start) .map(|function| function.id.clone()); - let id = make_id(&[parent.as_deref().unwrap_or(&self.stem), &name]); + let qualified_name = parent + .as_deref() + .and_then(|parent_id| functions.iter().find(|function| function.id == parent_id)) + .map_or_else( + || format!("{name}()"), + |parent| format!("{}::{name}()", parent.qualified_name), + ); + let identity = format!("{}::{qualified_name}", self.stem); + let digest = format!("{:x}", Sha256::digest(identity.as_bytes())); + let id = make_id(&[&self.stem, &qualified_name, &digest]); functions.push(Function { start, end, id, name, + qualified_name, parent, }); } @@ -242,12 +259,16 @@ impl<'a> State<'a> { } } - fn add_node(&mut self, id: &str, label: &str, at: usize) { + fn add_node(&mut self, id: &str, label: &str, qualified_name: &str, at: usize) { if !self.seen.insert(id.to_owned()) { return; } let mut attributes = Map::new(); attributes.insert("label".into(), Value::String(label.to_owned())); + attributes.insert( + "qualified_name".into(), + Value::String(qualified_name.to_owned()), + ); attributes.insert("file_type".into(), Value::String("code".into())); attributes.insert( "source_file".into(),