Add core semantic replay foundation#84
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
✅ Deploy Preview for comptext-v7 canceled.
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a core foundation for token-efficient execution replay, featuring a semantic reference protocol, an append-only event log, and a rubric-based decision quality engine. Feedback focuses on improving the robustness of the stableStringify utility to handle edge cases like undefined and toJSON methods, optimizing the performance of reference deduplication from O(N^2) to O(N), and addressing the computational overhead of deep-cloning events during log retrieval.
| export function stableStringify(value: unknown): string { | ||
| if (value === null || typeof value !== 'object') { | ||
| return JSON.stringify(value); | ||
| } | ||
| if (Array.isArray(value)) { | ||
| return `[${value.map((entry) => stableStringify(entry)).join(',')}]`; | ||
| } | ||
| const entries = Object.entries(value as Record<string, unknown>) | ||
| .filter(([, entry]) => entry !== undefined) | ||
| .sort(([left], [right]) => left.localeCompare(right)); | ||
| return `{${entries.map(([key, entry]) => `${JSON.stringify(key)}:${stableStringify(entry)}`).join(',')}}`; | ||
| } |
There was a problem hiding this comment.
The stableStringify implementation has several issues that affect its robustness and correctness:
- Type Safety & Runtime Error:
JSON.stringify(undefined)returnsundefined(the value), not a string. This violates thestringreturn type and causesstableHashto throw an error when accessing.lengthon line 20. - Array Consistency: It doesn't handle
undefinedvalues inside arrays correctly. Standard JSON serialization convertsundefinedin arrays tonull, but this implementation would produce an empty string segment (e.g.,[undefined]becomes[]instead of[null]). - Missing
toJSONSupport: It doesn't handle objects withtoJSONmethods (likeDate), resulting in empty objects{}instead of the expected serialized string. - Function/Symbol Handling: It should explicitly handle functions and symbols to match
JSON.stringifybehavior (omitted in objects,nullin arrays).
export function stableStringify(value: unknown): string {
if (value === undefined || typeof value === 'function' || typeof value === 'symbol') {
return 'null';
}
if (value === null || typeof value !== 'object') {
return JSON.stringify(value);
}
if (typeof (value as any).toJSON === 'function') {
return stableStringify((value as any).toJSON());
}
if (Array.isArray(value)) {
return `[${value.map((entry) => stableStringify(entry)).join(',')}]`;
}
const entries = Object.entries(value as Record<string, unknown>)
.filter(([, entry]) => entry !== undefined && typeof entry !== 'function' && typeof entry !== 'symbol')
.sort(([left], [right]) => left.localeCompare(right));
return `{${entries.map(([key, entry]) => `${JSON.stringify(key)}:${stableStringify(entry)}`).join(',')}}`;
}| const deduped = new Map<string, SemanticReference>(); | ||
| for (const reference of references) { | ||
| const existing = deduped.get(reference.uri) ?? deduped.get(reference.hash); | ||
| if (!existing) { | ||
| deduped.set(reference.uri, reference); | ||
| deduped.set(reference.hash, reference); | ||
| } | ||
| } | ||
| const candidates = uniqueStable(Array.from(deduped.values()).map((reference) => reference.id)) | ||
| .map((id) => Array.from(deduped.values()).find((reference) => reference.id === id)!) | ||
| .sort((left, right) => right.relevanceScore - left.relevanceScore || left.tokenEstimate - right.tokenEstimate || left.id.localeCompare(right.id)); |
There was a problem hiding this comment.
The current deduplication and candidate selection logic is inefficient. Specifically, lines 143-144 perform an O(N^2) operation because they iterate over unique IDs and then perform a find on an array that contains duplicates for every entry in the deduped map. This can be simplified to O(N) (plus the sort) by using a more direct approach to track seen URIs and Hashes.
const uniqueRefs = new Map<string, SemanticReference>();
const seenUris = new Set<string>();
const seenHashes = new Set<string>();
for (const ref of references) {
if (!seenUris.has(ref.uri) && !seenHashes.has(ref.hash)) {
uniqueRefs.set(ref.id, ref);
seenUris.add(ref.uri);
seenHashes.add(ref.hash);
}
}
const candidates = Array.from(uniqueRefs.values()).sort(
(left, right) =>
right.relevanceScore - left.relevanceScore ||
left.tokenEstimate - right.tokenEstimate ||
left.id.localeCompare(right.id)
);| list(executionId?: string): ExecutionEvent[] { | ||
| return this.events | ||
| .filter((event) => executionId === undefined || event.executionId === executionId) | ||
| .map(cloneEvent); |
There was a problem hiding this comment.
Using JSON.parse(JSON.stringify(...)) for deep cloning on every list() call is computationally expensive and creates significant garbage collection pressure, especially as the event log grows. Since the events are already shallow-frozen upon being appended, consider using structuredClone() for better performance if deep cloning is required, or evaluate if the immutability guarantees provided by Object.freeze are sufficient to avoid cloning on every read.
1 participant
Motivation
Description
SemanticReference,ContextManifest,SemanticReferenceRegistry,ReferenceResolvertyping,TokenBudgetManager,ContextManifestBuilder, andCompactPromptBuilderthat hydrate only on explicit request. (dashboard/app/src/core/foundation/semanticReferences.ts)ExecutionEvent,ExecutionEventLog,InMemoryExecutionEventStore,appendExecutionEvent,getExecutionTimeline,summarizeExecutionEvents, and compact-payload size enforcement. (dashboard/app/src/core/foundation/executionEventLog.ts)ReplaySnapshot,ReplayBranch,ReplayTimeline,ReplayComparator,createReplaySnapshot,branchFromStep, andcompareReplayRuns. (dashboard/app/src/core/foundation/replaySnapshot.ts)QualityRubric,DecisionQualityEngine, andQualityEvalRunthat scores validity, specificity, correctness, traceability, rollbackSafety, and tokenEfficiency using a fixed rubric. (dashboard/app/src/core/foundation/decisionQuality.ts)coreFoundationSampleexposing static sample data for optional lightweight wiring. (dashboard/app/src/core/foundation/shared.ts, sampleData.ts)Testing
npm run build(repo root): unavailable, failed with ENOENT because/workspace/Comptextv7/package.jsonis missing (error: "Could not read package.json: ENOENT: no such file or directory, open '/workspace/Comptextv7/package.json'").npm run lintandnpm test -- --runInBand(repo root): unavailable for the sameENOENTpackage.json error.npm run typecheck(dashboard/app): executed and completed successfully (tsc -bsucceeded).npm run build(dashboard/app): executed and completed successfully (Vite build +tscsucceeded; production build produceddist/assets).pytest tests/test_core_foundation_ts.py -q: passed (4 tests covering the new TypeScript foundation modules).pytest -q: full project test suite passed (51 tests).Codex Task