Skip to content

Drop the output re-parse and the full pre-run snapshot #11

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
IPerception wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Drop the output re-parse and the full pre-run snapshot #11

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
eb998fc
perf: browse the mutated document instead of re-parsing the output
IPerception Sep 8, 2026
e4771da
test: audit that rule marks account for every change they make
IPerception Sep 8, 2026
1df0cc4
perf: snapshot only the segments a rule changes
IPerception Sep 8, 2026
8a3f1e9
docs: record what items 2 and 3 actually did, and correct the old num…
IPerception Sep 8, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
80 changes: 67 additions & 13 deletions web/EDIWorkbench.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2238,8 +2238,11 @@ <h3>Using it with real data</h3>
const shifted = shiftDate8(value, this.days);
if (shifted === null) return; // malformed date; skip rather than fail the file

// Copied before the write, not after: processText builds its change list
// from these marks, so this is the only record of the original date.
const before = { id: seg.id, elements: seg.elements.slice() };
seg.elements[2] = shifted;
marks.push({ segment: i, elements: [2] });
marks.push({ segment: i, elements: [2], before });
});

return { segments: marks.length, occurrences: marks.length, marks };
Expand Down Expand Up @@ -2315,14 +2318,18 @@ <h3>Using it with real data</h3>
if (this.segmentIds && !this.segmentIds.has(seg.id.toUpperCase())) return;

const hits = [];
let before = null;
seg.elements.forEach((value, j) => {
const [next, count] = this.replaceIn(value);
if (!count) return;
// On the first hit only, and before the write: one copy per changed
// segment is what replaced processText's clone of every segment.
if (!before) before = { id: seg.id, elements: seg.elements.slice() };
seg.elements[j] = next;
occurrences += count;
hits.push(j);
});
if (hits.length) marks.push({ segment: i, elements: hits });
if (hits.length) marks.push({ segment: i, elements: hits, before });
});

return { segments: marks.length, occurrences, marks };
Expand Down Expand Up @@ -2384,10 +2391,12 @@ <h3>Using it with real data</h3>

/* ---- engine -------------------------------------------------------- */

function snapshot(segments) {
return segments.map((seg) => ({ id: seg.id, elements: seg.elements.slice() }));
}

// There was a snapshot(segments) here that cloned every segment in the
// document before the rules ran, so processText could diff against it
// afterwards. It was a second full copy of the working set, built to find
// changes the rules already report: each rule now copies a segment on its
// first write to it and carries that copy in the mark. sameSegment survives
// because the marks say what a rule touched, not what it managed to change.
function sameSegment(a, b) {
if (a.id !== b.id || a.elements.length !== b.elements.length) return false;
return a.elements.every((v, i) => v === b.elements[i]);
Expand All @@ -2402,10 +2411,19 @@ <h3>Using it with real data</h3>
if (typeof rule.check === "function") rule.check(doc);
}

const before = snapshot(doc.segments);
const countBefore = doc.segments.length;
const summary = [];
const ruleKindsBySegment = new Map();
// Segment index -> the segment as it was before any rule touched it. Each
// rule copies a segment on its first write to it and carries that copy in
// the mark, so only segments that actually change are ever copied. This
// replaced a clone of the entire document taken up front, which on a large
// file was a second full working set built to find a handful of changes.
//
// First writer wins. Two rules can hit one segment, and the second one's
// copy has the first one's edit already in it -- the change list has to
// show what arrived, not what the second rule happened to find.
const beforeBySegment = new Map();

for (const rule of rules) {
const result = rule.apply(doc.segments);
Expand All @@ -2417,6 +2435,9 @@ <h3>Using it with real data</h3>
occurrences: result.occurrences,
});
for (const mark of result.marks) {
if (mark.before && !beforeBySegment.has(mark.segment)) {
beforeBySegment.set(mark.segment, mark.before);
}
let entry = ruleKindsBySegment.get(mark.segment);
if (!entry) { entry = { kinds: [], elements: new Map() }; ruleKindsBySegment.set(mark.segment, entry); }
if (!entry.kinds.includes(rule.kind)) entry.kinds.push(rule.kind);
Expand All @@ -2432,25 +2453,44 @@ <h3>Using it with real data</h3>
//
// The change list below pairs segments by index, which only holds while
// the count is unchanged; a rule that changes it will need that revisited.
// Note the counts this repairs are not themselves listed as changes: the
// list is built from what the rules marked, and this repair is not a rule.
// That is moot while no rule changes the count, and is the second thing to
// revisit on the day one does.
const countsRepaired = doc.segments.length === countBefore
? 0
: repairControlCounts(doc);

// Only the segments a rule marked can appear here, so this walks those
// rather than sweeping the whole document. sameSegment stays as a filter
// over them: a rule may mark a write that turned out to change nothing --
// StringReplaceRule counts a match even when the replacement equals it --
// and such a segment must not reach the Changes tab as a change with an
// identical before and after. What the filter no longer catches is the
// opposite case, a rule editing a segment without marking it; that is
// covered directly now, by the mark audits in parity.mjs and deid.mjs.
const changes = [];
doc.segments.forEach((seg, i) => {
if (sameSegment(before[i], seg)) return;
for (const i of [...beforeBySegment.keys()].sort((a, b) => a - b)) {
const was = beforeBySegment.get(i);
const seg = doc.segments[i];
if (sameSegment(was, seg)) continue;
const entry = ruleKindsBySegment.get(i) || { kinds: [], elements: new Map() };
changes.push({
index: i,
before: before[i],
before: was,
after: { id: seg.id, elements: seg.elements.slice() },
kinds: entry.kinds,
elementKinds: entry.elements,
});
});
}

return {
output: serialize(doc),
// The mutated document itself, so the UI can browse the result without
// parsing `output` back into a second copy of what is already here.
// Callers must treat it as owned by the result: the segment objects are
// live, not clones, and `changes` holds the only record of what they were.
doc,
segmentCount: doc.segments.length,
delimiters: {
element: doc.elementSep,
Expand Down Expand Up @@ -4137,13 +4177,21 @@ <h3>Using it with real data</h3>

apply(segments) {
const marks = new Map(); // segment index -> Set of element indices
const befores = new Map(); // segment index -> the segment as it arrived
const mark = (i, el) => {
let set = marks.get(i);
if (!set) { set = new Set(); marks.set(i, set); }
set.add(el);
};
const put = (i, el, value) => {
if (segments[i].elements[el] === value) return; // no-op: don't claim a change
// Copy the segment before the first write lands on it, and only then --
// processText no longer clones every segment up front, so this is the
// only chance to record what a rewritten value used to be. It has to
// happen here rather than in mark(), which runs after the assignment.
if (!befores.has(i)) {
befores.set(i, { id: segments[i].id, elements: segments[i].elements.slice() });
}
segments[i].elements[el] = value;
mark(i, el);
};
Expand Down Expand Up @@ -4284,7 +4332,7 @@ <h3>Using it with real data</h3>
const out = [];
for (const [segment, elements] of [...marks].sort((a, b) => a[0] - b[0])) {
occurrences += elements.size;
out.push({ segment, elements: [...elements] });
out.push({ segment, elements: [...elements], before: befores.get(segment) });
}
return { segments: out.length, occurrences, marks: out };
}
Expand Down Expand Up @@ -6086,7 +6134,13 @@ <h3>Using it with real data</h3>
`<div class="tile ${t.cls}"><b>${t.value.toLocaleString()}</b><span class="label">${t.label}</span></div>`
).join("");

state.doc = parse(result.output); // browse the edited document, not the original
// Browse the edited document, not the original. processText already holds
// the mutated document, so this takes it rather than re-parsing the output
// string back into a second full set of segment objects -- which cost a
// whole extra parse and about a third of peak heap for a document it had
// in hand the entire time. The segments here are the ones the rules
// mutated, which is exactly what the re-parse was reconstructing.
state.doc = result.doc;
state.outline = buildOutline(state.doc.segments);
state.claims = null; // stale the moment the document changes; rebuilt on demand
state.focus = null;
Expand Down
Loading