Skip to content

Commit 918adfb

Browse files
committed
refactor(core): name parse memoization keys and centralize LinkCtx build
Replace the positional (u8, u32, ArtifactFormat[, String]) parse/decline memoization tuples with named ParseClaim/Decline structs (+ side_code and ParseClaim::declined_by helpers), and collapse the three duplicated LinkCtx constructions into a single link_ctx() helper. Behavior unchanged (core + vector suites green; clippy clean).
1 parent 9b2ac6f commit 918adfb

1 file changed

Lines changed: 101 additions & 83 deletions

File tree

binoc-core/src/correspondence/driver.rs

Lines changed: 101 additions & 83 deletions
Original file line numberDiff line numberDiff line change
@@ -205,18 +205,12 @@ impl CorrespondenceRunResult {
205205
})?;
206206
let link = self.store.links.link(link_index);
207207
let view = CoreEngineView::new(&self.store, false);
208-
let link_ref = view.link_ref(link_index);
209-
let path = &self.store.right.node(link.right).item.logical_path;
210-
let ctx = LinkCtx {
211-
view: &view,
212-
link: link_ref,
213-
row_keys: config.row_keys.get(path).map(Vec::as_slice).unwrap_or(&[]),
214-
row_identity_policies: config
215-
.row_identity_policies
216-
.get(path)
217-
.copied()
218-
.unwrap_or_default(),
219-
};
208+
let ctx = link_ctx(
209+
&view,
210+
view.link_ref(link_index),
211+
&self.store.right.node(link.right).item.logical_path,
212+
config,
213+
);
220214
let edits = self
221215
.edit_lists
222216
.get(&link_index)
@@ -349,21 +343,15 @@ fn run_inner(
349343
let mut expanded: BTreeSet<(u8, u32)> = BTreeSet::new();
350344
// A *successful* parse memoizes per (node, output format): first claim wins
351345
// for that format on that node, so no other same-format rule re-parses it.
352-
let mut parsed: BTreeSet<(u8, u32, binoc_sdk::ArtifactFormat)> = BTreeSet::new();
346+
let mut parsed: BTreeSet<ParseClaim> = BTreeSet::new();
353347
// A *declined* parse memoizes per (node, output format, RULE) so the same
354348
// rule is not retried, while leaving the format free for a different rule.
355349
// This is what lets a fusing claim decline a bare/invalid group and still
356350
// have the single-input parser (same output format) claim the node (CFM-83).
357-
let mut declined: BTreeSet<(u8, u32, binoc_sdk::ArtifactFormat, String)> = BTreeSet::new();
351+
let mut declined: BTreeSet<Decline> = BTreeSet::new();
358352

359353
fn key(side: TreeSide, index: u32) -> (u8, u32) {
360-
(
361-
match side {
362-
TreeSide::Left => 0,
363-
TreeSide::Right => 1,
364-
},
365-
index,
366-
)
354+
(side_code(side), index)
367355
}
368356

369357
// Per-round rule processing order (CFM-83). Parse rules are attempted
@@ -510,19 +498,13 @@ fn run_inner(
510498
let mut jobs = Vec::new();
511499
for side in [TreeSide::Left, TreeSide::Right] {
512500
for index in 0..frontier_of(side) {
513-
let parsed_key = (key(side, index).0, index, descriptor.output.clone());
514-
if parsed.contains(&parsed_key) {
501+
let claim = ParseClaim::new(side, index, descriptor.output.clone());
502+
if parsed.contains(&claim) {
515503
continue;
516504
}
517505
// This rule already declined this node — don't retry
518506
// it (but a different same-format rule still may).
519-
let declined_key = (
520-
parsed_key.0,
521-
parsed_key.1,
522-
parsed_key.2.clone(),
523-
descriptor.name.clone(),
524-
);
525-
if declined.contains(&declined_key) {
507+
if declined.contains(&claim.declined_by(&descriptor.name)) {
526508
continue;
527509
}
528510
// A node already folded into a fusing result node is
@@ -566,7 +548,7 @@ fn run_inner(
566548
}
567549
jobs.push(ParseJob {
568550
id,
569-
parsed_key,
551+
claim,
570552
beneath,
571553
group,
572554
member_indices,
@@ -595,12 +577,7 @@ fn run_inner(
595577
);
596578
// Rule-scoped: an erroring rule does not block a
597579
// different same-format rule from trying the node.
598-
declined.insert((
599-
result.parsed_key.0,
600-
result.parsed_key.1,
601-
result.parsed_key.2.clone(),
602-
descriptor.name.clone(),
603-
));
580+
declined.insert(result.claim.declined_by(&descriptor.name));
604581
continue;
605582
}
606583
};
@@ -619,12 +596,7 @@ fn run_inner(
619596
// shapefile fusing claim to release a bare/invalid group to
620597
// the single-input parser (CFM-83).
621598
if output.bytes.is_empty() && output.children.is_empty() {
622-
declined.insert((
623-
result.parsed_key.0,
624-
result.parsed_key.1,
625-
result.parsed_key.2.clone(),
626-
descriptor.name.clone(),
627-
));
599+
declined.insert(result.claim.declined_by(&descriptor.name));
628600
continue;
629601
}
630602
// Let the parse rule name the node it just decomposed
@@ -702,7 +674,7 @@ fn run_inner(
702674
.tree_mut(result.id.side)
703675
.subsume(member_index, result.id.index);
704676
}
705-
parsed.insert(result.parsed_key);
677+
parsed.insert(result.claim);
706678
stats.record_fire(
707679
round,
708680
&descriptor.name,
@@ -871,14 +843,7 @@ fn run_inner(
871843
trace.as_deref_mut(),
872844
)?;
873845

874-
compact_edit_lists(
875-
config,
876-
&store,
877-
&mut edit_lists,
878-
&mut stats,
879-
data,
880-
trace.as_deref_mut(),
881-
)?;
846+
compact_edit_lists(config, &store, &mut edit_lists, &mut stats, data, trace)?;
882847

883848
Ok(CorrespondenceRunResult {
884849
store,
@@ -890,6 +855,31 @@ fn run_inner(
890855
})
891856
}
892857

858+
/// Build a `LinkCtx` for one link. Centralizes the per-link row-key and
859+
/// row-identity-policy lookups (keyed on the right node's logical path) that
860+
/// every writer/compaction/extract dispatch needs.
861+
fn link_ctx<'a>(
862+
view: &'a dyn EngineView,
863+
link: LinkRef,
864+
right_path: &str,
865+
config: &'a CorrespondenceEngineConfig,
866+
) -> LinkCtx<'a> {
867+
LinkCtx {
868+
view,
869+
link,
870+
row_keys: config
871+
.row_keys
872+
.get(right_path)
873+
.map(Vec::as_slice)
874+
.unwrap_or(&[]),
875+
row_identity_policies: config
876+
.row_identity_policies
877+
.get(right_path)
878+
.copied()
879+
.unwrap_or_default(),
880+
}
881+
}
882+
893883
fn build_edit_lists(
894884
config: &CorrespondenceEngineConfig,
895885
store: &Store,
@@ -936,20 +926,12 @@ fn build_edit_lists(
936926
edit_lists.insert(index, Vec::new());
937927
continue;
938928
};
939-
let ctx = LinkCtx {
940-
view: &view,
941-
link: link_ref,
942-
row_keys: config
943-
.row_keys
944-
.get(&store.right.node(link.right).item.logical_path)
945-
.map(Vec::as_slice)
946-
.unwrap_or(&[]),
947-
row_identity_policies: config
948-
.row_identity_policies
949-
.get(&store.right.node(link.right).item.logical_path)
950-
.copied()
951-
.unwrap_or_default(),
952-
};
929+
let ctx = link_ctx(
930+
&view,
931+
link_ref,
932+
&store.right.node(link.right).item.logical_path,
933+
config,
934+
);
953935

954936
// Dispatch composes, then selects (CFM-81). The link's edit list is
955937
// the concatenation of:
@@ -1065,20 +1047,12 @@ fn compact_edit_lists(
10651047
continue;
10661048
}
10671049
let link = store.links.link(*link_index);
1068-
let ctx = LinkCtx {
1069-
view: &view,
1070-
link: view.link_ref(*link_index),
1071-
row_keys: config
1072-
.row_keys
1073-
.get(&store.right.node(link.right).item.logical_path)
1074-
.map(Vec::as_slice)
1075-
.unwrap_or(&[]),
1076-
row_identity_policies: config
1077-
.row_identity_policies
1078-
.get(&store.right.node(link.right).item.logical_path)
1079-
.copied()
1080-
.unwrap_or_default(),
1081-
};
1050+
let ctx = link_ctx(
1051+
&view,
1052+
view.link_ref(*link_index),
1053+
&store.right.node(link.right).item.logical_path,
1054+
config,
1055+
);
10821056
// Format-scoped compaction (CFM-81): a rule that declares a
10831057
// `format()` sees and rewrites only the provenance-scoped segment
10841058
// of that format, never the whole mixed list. A rule with no
@@ -1160,12 +1134,56 @@ fn run_expand_jobs(
11601134
}
11611135
}
11621136

1137+
/// Stable side encoding (`Left = 0`, `Right = 1`) used as the leading sort key
1138+
/// for node-scoped memoization sets.
1139+
fn side_code(side: TreeSide) -> u8 {
1140+
match side {
1141+
TreeSide::Left => 0,
1142+
TreeSide::Right => 1,
1143+
}
1144+
}
1145+
1146+
/// Memoization key for a *successful* parse: first claim wins per (node, output
1147+
/// format), so no other same-format rule re-parses that node.
1148+
#[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord)]
1149+
struct ParseClaim {
1150+
side: u8,
1151+
index: u32,
1152+
format: ArtifactFormat,
1153+
}
1154+
1155+
/// Memoization key for a *declined* parse: scoped to the rule, so the same rule
1156+
/// is not retried while the output format stays free for a different rule
1157+
/// (CFM-83).
1158+
#[derive(Clone, PartialEq, Eq, PartialOrd, Ord)]
1159+
struct Decline {
1160+
claim: ParseClaim,
1161+
rule: String,
1162+
}
1163+
1164+
impl ParseClaim {
1165+
fn new(side: TreeSide, index: u32, format: ArtifactFormat) -> Self {
1166+
ParseClaim {
1167+
side: side_code(side),
1168+
index,
1169+
format,
1170+
}
1171+
}
1172+
1173+
fn declined_by(&self, rule: &str) -> Decline {
1174+
Decline {
1175+
claim: self.clone(),
1176+
rule: rule.to_string(),
1177+
}
1178+
}
1179+
}
1180+
11631181
#[derive(Debug, Clone)]
11641182
struct ParseJob {
11651183
/// The anchor node (the required, defining member — e.g. the `.shp`). The
11661184
/// parse result, artifacts, and any subsumption all attribute to this node.
11671185
id: NodeId,
1168-
parsed_key: (u8, u32, ArtifactFormat),
1186+
claim: ParseClaim,
11691187
beneath: bool,
11701188
/// The resolved member group handed to `parse_group`. For a single-input
11711189
/// rule this is just the anchor (`ParseGroup::single`).
@@ -1177,7 +1195,7 @@ struct ParseJob {
11771195

11781196
struct ParseJobResult {
11791197
id: NodeId,
1180-
parsed_key: (u8, u32, ArtifactFormat),
1198+
claim: ParseClaim,
11811199
beneath: bool,
11821200
item: ItemRef,
11831201
member_indices: Vec<u32>,
@@ -1192,7 +1210,7 @@ fn run_parse_jobs(
11921210
) -> Vec<ParseJobResult> {
11931211
let run_one = |job: &ParseJob| ParseJobResult {
11941212
id: job.id,
1195-
parsed_key: job.parsed_key.clone(),
1213+
claim: job.claim.clone(),
11961214
beneath: job.beneath,
11971215
item: job.group.anchor.clone(),
11981216
member_indices: job.member_indices.clone(),

0 commit comments

Comments
 (0)