mirror of
https://github.com/morluto/rea.git
synced 2026-10-31 20:06:14 +00:00
fix(javascript): index repeated graph observations before final sorting
Retain stable live nodes and observation arrays while avoiding quadratic collection rebuilds. Preserve duplicate handling and canonical output. Fixes #1748 Assisted-by: Suika (gensokyo-skills:suika-scatter-gather-planning)
This commit is contained in:
1 parent
713db4dc26
commit
6d5d71fab5
2 files changed
+162
-26
No files matched your search
@@ -0,0 +1,97 @@
|
||||
import { expect, it } from "vitest";
|
||||
import { completeApplicationCoverage } from "../../domain/javascript/javascriptApplicationEvidenceSchemas.js";
|
||||
import { createJavaScriptApplicationNode } from "../../domain/javascript/javascriptApplicationGraph.js";
|
||||
import { JavaScriptArtifactGraphAccumulator } from "./JavaScriptArtifactGraphAccumulator.js";
|
||||
import { artifactObservationEvidence } from "./JavaScriptArtifactGraphEvidence.js";
|
||||
|
||||
const nodeInput = (value: string, sha256 = "a".repeat(64)) => ({
|
||||
kind: "javascript-asset",
|
||||
identity: {
|
||||
strategy: "content-digest",
|
||||
stability: "global-exact",
|
||||
sha256,
|
||||
},
|
||||
observations: [
|
||||
{
|
||||
label: value,
|
||||
properties: { value },
|
||||
evidence: artifactObservationEvidence({
|
||||
sha256,
|
||||
path: "main.js",
|
||||
operation: "accumulator-test",
|
||||
coverage: completeApplicationCoverage(),
|
||||
}),
|
||||
},
|
||||
],
|
||||
});
|
||||
|
||||
it("retains live references, exact deduplication and canonical output across later additions", () => {
|
||||
const accumulator = new JavaScriptArtifactGraphAccumulator();
|
||||
const inputs = ["雪😀", "alpha", "beta"].map((value) => nodeInput(value));
|
||||
const firstInput = inputs[0];
|
||||
if (firstInput === undefined) throw new Error("Missing first input");
|
||||
const node = accumulator.addNode(firstInput);
|
||||
const observations = node.observations;
|
||||
const firstObservation = observations[0];
|
||||
expect(accumulator.addNode(firstInput)).toBe(node);
|
||||
expect(node.observations).toBe(observations);
|
||||
expect(observations).toHaveLength(1);
|
||||
expect(observations[0]).not.toBe(firstObservation);
|
||||
for (const input of inputs.slice(1))
|
||||
expect(accumulator.addNode(input)).toBe(node);
|
||||
const secondNode = accumulator.addNode(nodeInput("other", "b".repeat(64)));
|
||||
const firstSnapshot = accumulator.nodes();
|
||||
const expectedNode = createJavaScriptApplicationNode({
|
||||
...firstInput,
|
||||
observations: inputs.flatMap((input) => input.observations),
|
||||
});
|
||||
expect(firstSnapshot).toEqual([expectedNode, secondNode]);
|
||||
expect(firstSnapshot[0]).toBe(node);
|
||||
expect(node.observations).toBe(observations);
|
||||
const finalizedObservations = [...observations];
|
||||
expect(accumulator.nodes()).not.toBe(firstSnapshot);
|
||||
expect(accumulator.nodes()).toEqual(firstSnapshot);
|
||||
for (const [index, observation] of finalizedObservations.entries())
|
||||
expect(node.observations[index]).toBe(observation);
|
||||
|
||||
const later = nodeInput("later");
|
||||
expect(accumulator.addNode(later)).toBe(node);
|
||||
// Updating a duplicate after final sorting must use its new array index.
|
||||
expect(accumulator.addNode(firstInput)).toBe(node);
|
||||
expect(accumulator.nodes()).toEqual([
|
||||
createJavaScriptApplicationNode({
|
||||
...firstInput,
|
||||
observations: [...inputs, later].flatMap((input) => input.observations),
|
||||
}),
|
||||
secondNode,
|
||||
]);
|
||||
expect(node.observations).toBe(observations);
|
||||
});
|
||||
|
||||
it("preserves first-input duplicates and retains the last occurrence when later merged", () => {
|
||||
const accumulator = new JavaScriptArtifactGraphAccumulator();
|
||||
const first = nodeInput("same");
|
||||
const distinct = nodeInput("distinct");
|
||||
const node = accumulator.addNode({
|
||||
...first,
|
||||
observations: [...first.observations, ...first.observations],
|
||||
});
|
||||
const observations = node.observations;
|
||||
const lastInitial = observations[1];
|
||||
expect(accumulator.nodes()[0]?.observations).toHaveLength(2);
|
||||
expect(accumulator.addNode(distinct)).toBe(node);
|
||||
expect(accumulator.nodes()).toEqual([
|
||||
createJavaScriptApplicationNode({
|
||||
...first,
|
||||
observations: [...first.observations, ...distinct.observations],
|
||||
}),
|
||||
]);
|
||||
expect(node.observations).toBe(observations);
|
||||
expect(observations).toContain(lastInitial);
|
||||
// A repeated observation inside an incoming batch also collapses to one.
|
||||
accumulator.addNode({
|
||||
...distinct,
|
||||
observations: [...distinct.observations, ...distinct.observations],
|
||||
});
|
||||
expect(accumulator.nodes()[0]?.observations).toHaveLength(2);
|
||||
});
|
||||
@@ -6,40 +6,62 @@ import type {
|
||||
ApplicationEdge,
|
||||
ApplicationNode,
|
||||
} from "../../domain/javascript/javascriptApplicationGraphSchemas.js";
|
||||
import { compareUnicodeCodePoints } from "../../domain/unicodeCodePointOrder.js";
|
||||
|
||||
/** Deduplicate graph entities while retaining distinct evidence observations. */
|
||||
/**
|
||||
* Own live construction nodes and deduplicate their evidence observations.
|
||||
* Consumers retain references but must not mutate the owned observations;
|
||||
* nodes() finalizes their canonical order before graph validation/output.
|
||||
*/
|
||||
export class JavaScriptArtifactGraphAccumulator {
|
||||
readonly #nodes = new Map<string, ApplicationNode>();
|
||||
readonly #nodes = new Map<string, AccumulatedNode>();
|
||||
readonly #edges = new Map<string, ApplicationEdge>();
|
||||
|
||||
/** Create or merge one canonical node, retaining a stable live construction reference. */
|
||||
/** Merge observations into a stable live node; order is finalized by nodes(). */
|
||||
addNode<Input extends DisplayableNodeInput>(input: Input): ApplicationNode {
|
||||
const created = createJavaScriptApplicationNode(displayableNode(input));
|
||||
const existing = this.#nodes.get(created.node_id);
|
||||
if (existing === undefined) {
|
||||
this.#nodes.set(created.node_id, created);
|
||||
this.#nodes.set(created.node_id, {
|
||||
node: created,
|
||||
observationIndices: new Map(
|
||||
created.observations.map((observation, index) => [
|
||||
observation.observation_id,
|
||||
index,
|
||||
]),
|
||||
),
|
||||
dirty: false,
|
||||
});
|
||||
return created;
|
||||
}
|
||||
const bySemanticId = new Map(
|
||||
[...existing.observations, ...created.observations].map((observation) => [
|
||||
observation.observation_id,
|
||||
observation,
|
||||
]),
|
||||
);
|
||||
const observations = [...bySemanticId.values()].sort((left, right) =>
|
||||
left.observation_id < right.observation_id
|
||||
? -1
|
||||
: left.observation_id > right.observation_id
|
||||
? 1
|
||||
: 0,
|
||||
);
|
||||
// Both factories already validated and identified these observations.
|
||||
// Replacing the node retained every previous observation array through
|
||||
// path/chunk/IPC indexes when multiple paths shared an entity identity.
|
||||
existing.observations.length = 0;
|
||||
for (const observation of observations)
|
||||
existing.observations.push(observation);
|
||||
return existing;
|
||||
const observations = existing.node.observations;
|
||||
// The first factory input can contain duplicate IDs. As before, a later
|
||||
// merge retains only their last occurrences without replacing the array.
|
||||
if (existing.observationIndices.size !== observations.length) {
|
||||
let retained = 0;
|
||||
for (const [index, observation] of observations.entries()) {
|
||||
if (
|
||||
existing.observationIndices.get(observation.observation_id) !== index
|
||||
)
|
||||
continue;
|
||||
observations[retained] = observation;
|
||||
existing.observationIndices.set(observation.observation_id, retained);
|
||||
retained += 1;
|
||||
}
|
||||
observations.length = retained;
|
||||
}
|
||||
for (const observation of created.observations) {
|
||||
const index = existing.observationIndices.get(observation.observation_id);
|
||||
if (index === undefined) {
|
||||
existing.observationIndices.set(
|
||||
observation.observation_id,
|
||||
observations.length,
|
||||
);
|
||||
observations.push(observation);
|
||||
existing.dirty = true;
|
||||
} else observations[index] = observation;
|
||||
}
|
||||
return existing.node;
|
||||
}
|
||||
|
||||
/** Create or replace one canonical edge by semantic identifier. */
|
||||
@@ -49,9 +71,20 @@ export class JavaScriptArtifactGraphAccumulator {
|
||||
return edge;
|
||||
}
|
||||
|
||||
/** Return all accumulated canonical nodes. */
|
||||
/** Finalize observation order in place and return all canonical nodes. */
|
||||
nodes(): readonly ApplicationNode[] {
|
||||
return [...this.#nodes.values()];
|
||||
return [...this.#nodes.values()].map((entry) => {
|
||||
if (entry.dirty) {
|
||||
entry.node.observations.sort((left, right) =>
|
||||
compareUnicodeCodePoints(left.observation_id, right.observation_id),
|
||||
);
|
||||
entry.observationIndices.clear();
|
||||
for (const [index, observation] of entry.node.observations.entries())
|
||||
entry.observationIndices.set(observation.observation_id, index);
|
||||
entry.dirty = false;
|
||||
}
|
||||
return entry.node;
|
||||
});
|
||||
}
|
||||
|
||||
/** Return all accumulated canonical edges. */
|
||||
@@ -60,6 +93,12 @@ export class JavaScriptArtifactGraphAccumulator {
|
||||
}
|
||||
}
|
||||
|
||||
interface AccumulatedNode {
|
||||
readonly node: ApplicationNode;
|
||||
readonly observationIndices: Map<string, number>;
|
||||
dirty: boolean;
|
||||
}
|
||||
|
||||
/** Node fields whose display form is normalized before validation. */
|
||||
interface DisplayableNodeInput {
|
||||
readonly observations: readonly { readonly label: string | null }[];
|
||||
|
||||
Reference in new issue
Block a user