From b7ded180c86d011c14b5ade399d886a51fb0f6d6 Mon Sep 17 00:00:00 2001 From: Tom Softreck Date: Mon, 19 Oct 2026 22:06:35 +0200 Subject: [PATCH] perf(schema): validate each intent graph object once (ticket-109, Closes #135) Co-Authored-By: Claude Opus 5.5 --- project/TICKETS.md | 1 + project/ticket-109/README.md | 33 ++++ .../ticket-109/evidence/validate-once-ab.json | 159 ++++++++++++++++++ project/ticket-109/intent.json | 83 +++++++++ project/ticket-109/performance-plan.json | 120 +++++++++++++ src/core/schema-intent-validation.ts | 39 +++++ test/schema-validate-once.test.ts | 56 ++++++ 7 files changed, 491 insertions(+) create mode 100644 project/ticket-109/README.md create mode 100644 project/ticket-109/evidence/validate-once-ab.json create mode 100644 project/ticket-109/intent.json create mode 100644 project/ticket-109/performance-plan.json create mode 100644 test/schema-validate-once.test.ts diff --git a/project/TICKETS.md b/project/TICKETS.md index 23a289bb..1b8d0084 100644 --- a/project/TICKETS.md +++ b/project/TICKETS.md @@ -84,4 +84,5 @@ of `project/README.md`, which remains a generated technical-analysis artifact. | **ticket-104** | [`README.md`](./ticket-104/README.md) | - | - | - | - | - | | **ticket-105** | [`README.md`](./ticket-105/README.md) | - | - | - | - | - | | **ticket-106** | [`README.md`](./ticket-106/README.md) | - | - | - | - | - | +| **ticket-108** | [`README.md`](./ticket-108/README.md) | - | - | - | - | - | diff --git a/project/ticket-109/README.md b/project/ticket-109/README.md new file mode 100644 index 00000000..6194bdd6 --- /dev/null +++ b/project/ticket-109/README.md @@ -0,0 +1,33 @@ +# Ticket 109: Validate each intent graph once per object (#135) + +- **ID**: ticket-109 +- **Owner**: agent:claude +- **Status**: IN_PROGRESS +- **Workflow state**: EDIT +- **Created**: 2026-10-19 + +## Goal and scope + +Closes autogrammar/todo2code#135. `assertIntentGraph` re-validated the same graph +object ~9 times per pipeline and recomputed its canonical-JSON fingerprint each +time (18 calls / 98 s and 20 fingerprints / 74 s of a 290 s `compare-workspace` +on subactor/platform). A `WeakMap` memo now skips a repeat full validation when +the same object still has the same fingerprint string and the same `records` / +`relations` array identities and lengths; failures are never cached. + +Implementation drafted by Willman (koru.develop_ticket, glm-5.3) from an exact +spec; reviewed without changes. + +## Acceptance criteria + +- [x] AC-02: compare-workspace on subactor/platform, 3 interleaved pairs: CPU median + 360.7 s -> 255.3 s (pairs 1.24-1.41x), outputs identical. Evidence + `evidence/validate-once-ab.json`, validated `performance-plan.json`. +- [x] AC-01: Repeated assertions on one unchanged graph validate once; clones, + replaced relation arrays and changed fingerprints are revalidated + (`node --test dist/test/schema-validate-once.test.js`: 4 pass; schema suites 10 pass). + +## Tracking boundary + +This directory contains the minimal reviewed intent. Optional participant prose +and raw command logs are not required delivery output. diff --git a/project/ticket-109/evidence/validate-once-ab.json b/project/ticket-109/evidence/validate-once-ab.json new file mode 100644 index 00000000..b1301e21 --- /dev/null +++ b/project/ticket-109/evidence/validate-once-ab.json @@ -0,0 +1,159 @@ +{ + "schema": "todo2code.validate-once-ab/v1", + "measuredAt": "2026-10-19", + "host": "developer workstation x86_64 Linux 6.17, node 20.19.5, 16 cores, shared host (load1 23-56)", + "workload": "t2c compare-workspace on a clone of subactor/platform (base HEAD~3), CI deterministic environment; graph 50k records / 415k relations", + "method": "same build; old arm = memo disabled via a temporary T2C_REVALIDATE_GRAPHS=1 switch (removed before merge), new arm = memo; arms interleaved ABABAB, 3 pairs; /usr/bin/time user+sys CPU and wall", + "cpu_series": [ + { + "pair": 1, + "arm": "old_revalidate", + "wall_s": 339.15, + "rss_kb": 3058528, + "load1": 28.12, + "fingerprints": [ + "5ac0a4e247d4", + "01d6361de665" + ], + "cpu_s": 360.68 + }, + { + "pair": 1, + "arm": "new_memo", + "wall_s": 239.69, + "rss_kb": 2916300, + "load1": 35.84, + "fingerprints": [ + "5ac0a4e247d4", + "01d6361de665" + ], + "cpu_s": 255.29 + }, + { + "pair": 2, + "arm": "old_revalidate", + "wall_s": 392.57, + "rss_kb": 3248764, + "load1": 55.8, + "fingerprints": [ + "5ac0a4e247d4", + "01d6361de665" + ], + "cpu_s": 373.7 + }, + { + "pair": 2, + "arm": "new_memo", + "wall_s": 335.38, + "rss_kb": 3221788, + "load1": 50.84, + "fingerprints": [ + "5ac0a4e247d4", + "01d6361de665" + ], + "cpu_s": 300.35 + }, + { + "pair": 3, + "arm": "old_revalidate", + "wall_s": 286.49, + "rss_kb": 3152400, + "load1": 23.41, + "fingerprints": [ + "5ac0a4e247d4", + "01d6361de665" + ], + "cpu_s": 326.74 + }, + { + "pair": 3, + "arm": "new_memo", + "wall_s": 209.2, + "rss_kb": 3199796, + "load1": 30.32, + "fingerprints": [ + "5ac0a4e247d4", + "01d6361de665" + ], + "cpu_s": 233.96 + } + ], + "wall_series_earlier_run": [ + { + "pair": 1, + "arm": "old_revalidate", + "wall_s": 420.64, + "rss_kb": 3174240, + "load1": 53.25, + "fingerprints": [ + "5ac0a4e247d4", + "01d6361de665" + ] + }, + { + "pair": 1, + "arm": "new_memo", + "wall_s": 369.45, + "rss_kb": 3297088, + "load1": 39.85, + "fingerprints": [ + "5ac0a4e247d4", + "01d6361de665" + ] + }, + { + "pair": 2, + "arm": "old_revalidate", + "wall_s": 383.85, + "rss_kb": 3126132, + "load1": 38.64, + "fingerprints": [ + "5ac0a4e247d4", + "01d6361de665" + ] + }, + { + "pair": 2, + "arm": "new_memo", + "wall_s": 343.2, + "rss_kb": 3057708, + "load1": 46.41, + "fingerprints": [ + "5ac0a4e247d4", + "01d6361de665" + ] + }, + { + "pair": 3, + "arm": "old_revalidate", + "wall_s": 348.68, + "rss_kb": 3223544, + "load1": 28.77, + "fingerprints": [ + "5ac0a4e247d4", + "01d6361de665" + ] + }, + { + "pair": 3, + "arm": "new_memo", + "wall_s": 207.67, + "rss_kb": 3505540, + "load1": 26.55, + "fingerprints": [ + "5ac0a4e247d4", + "01d6361de665" + ] + } + ], + "summary": { + "cpu_s_median_old": 360.68, + "cpu_s_median_new": 255.29, + "cpu_ratio_pairs": [ + 1.41, + 1.24, + 1.4 + ], + "outputs_identical": true + } +} diff --git a/project/ticket-109/intent.json b/project/ticket-109/intent.json new file mode 100644 index 00000000..6d24617b --- /dev/null +++ b/project/ticket-109/intent.json @@ -0,0 +1,83 @@ +{ + "schema": "new-project.intent/v3", + "ticket": "ticket-109", + "summary": "Validate each intent graph once per object (#135)", + "workstream": "core-dsl", + "classification": { + "kind": "SERVICE", + "priority": "P2", + "origin": "health" + }, + "allowedPaths": [ + "project/ticket-109/**", + "TODO.md", + "project/TICKETS.md", + "src/core/schema-intent-validation.ts", + "test/schema-validate-once.test.ts" + ], + "forbiddenPaths": [ + "project/ticket-*/user-*.md" + ], + "stacks": [], + "dependsOn": [], + "conflictsWith": [], + "integrationTicket": null, + "delivery": { + "acceptedBaseSha": "a85da84e72c6c3fc7d2ea1c8b6eaf790f972a63f", + "targetBranch": "main", + "outcome": "assertIntentGraph validates each unchanged graph object once.", + "nonGoals": [ + "No change to validation rules, messages or graph output." + ], + "complexity": "S", + "estimatedMinutes": 30, + "budgets": { + "maxImplementationFiles": 2, + "maxAffectedComponents": 1, + "maxPublicInterfaceChanges": 1, + "maxRuntimeDependencies": 0 + }, + "architecture": { + "status": "accepted", + "decision": "Identity-keyed WeakMap memo in assertIntentGraph guarded by fingerprint and array identity.", + "components": [ + { + "name": "core-schema", + "paths": [ + "src/core/schema-intent-validation.ts", + "test/schema-validate-once.test.ts" + ] + } + ], + "responsibilityChanges": false, + "interfaceChanges": [ + "export intentGraphValidationCount()" + ], + "dataChanges": [], + "ui": { + "impact": "none", + "states": [], + "evidence": [] + }, + "rollback": "Revert the commit." + }, + "runtimeDependencies": [], + "validation": [ + { + "criterion": "AC-01", + "commands": [ + "npm run build", + "T2C_USE_SUBLLM=false node --test dist/test/schema-validate-once.test.js dist/test/schema-validation.test.js dist/test/structured-schema.test.js" + ], + "evidence": "project/ticket-109/README.md" + }, + { + "criterion": "AC-02", + "commands": [ + "python3 /src/performance.py validate project/ticket-109/performance-plan.json" + ], + "evidence": "project/ticket-109/performance-plan.json" + } + ] + } +} diff --git a/project/ticket-109/performance-plan.json b/project/ticket-109/performance-plan.json new file mode 100644 index 00000000..5abb6a20 --- /dev/null +++ b/project/ticket-109/performance-plan.json @@ -0,0 +1,120 @@ +{ + "schema": "performance.plan/v1", + "subject": { + "repository": "autogrammar/todo2code", + "revision": "a85da84e72c6c3fc7d2ea1c8b6eaf790f972a63f", + "component": "core/schema-intent-validation assertIntentGraph" + }, + "profiles": [ + "repository" + ], + "workload": { + "id": "compare-workspace-platform", + "description": "compare-workspace deterministic run on subactor/platform", + "method": "user+sys CPU via /usr/bin/time, interleaved arms", + "warmup": "none; OS page cache warm after first run", + "samples": 3, + "evidence": { + "uri": "repo:autogrammar/todo2code/project/ticket-109/evidence/validate-once-ab.json", + "sha256": "da50602794bb6df6602189a3b5aa68139956508d280d86793542ce7e3766e292" + } + }, + "baseline": { + "workloadId": "compare-workspace-platform", + "metrics": [ + { + "name": "cpu_time", + "unit": "ms", + "value": 360680 + } + ], + "evidence": { + "uri": "repo:autogrammar/todo2code/project/ticket-109/evidence/validate-once-ab.json", + "sha256": "da50602794bb6df6602189a3b5aa68139956508d280d86793542ce7e3766e292" + } + }, + "candidate": { + "workloadId": "compare-workspace-platform", + "metrics": [ + { + "name": "cpu_time", + "unit": "ms", + "value": 255290 + } + ], + "evidence": { + "uri": "repo:autogrammar/todo2code/project/ticket-109/evidence/validate-once-ab.json", + "sha256": "da50602794bb6df6602189a3b5aa68139956508d280d86793542ce7e3766e292" + } + }, + "budgets": [ + { + "metric": "cpu_time", + "unit": "ms", + "comparison": "at_most", + "threshold": 360680 + } + ], + "controls": [ + { + "kind": "repository_working_set", + "target": "compare-workspace analysed tree", + "limit": 1, + "unit": "count", + "mechanism": "unchanged, the validation memo only removes repeated work on identical graph objects", + "verification": "identical fingerprints across all six runs", + "exception": "" + } + ], + "invariants": [ + { + "id": "functional", + "kind": "functional", + "verification": "base and workspace graph fingerprints identical across arms; schema tests pass", + "required": true + }, + { + "id": "security", + "kind": "security", + "verification": "failed validations are never cached; changed fingerprint or replaced arrays are revalidated", + "required": true + }, + { + "id": "resource", + "kind": "resource", + "verification": "peak RSS within the same range (2.9-3.5 GB both arms)", + "required": true + }, + { + "id": "authority", + "kind": "authority", + "verification": "plan grants no execution authority", + "required": true + } + ], + "strategy": { + "outcome": "same_language", + "rationale": "assertIntentGraph re-validated and re-fingerprinted the same graph objects many times per run", + "changedPaths": [ + "src/core/schema-intent-validation.ts", + "test/schema-validate-once.test.ts" + ], + "compatibility": "identical outputs" + }, + "rollout": { + "mechanism": "release and bump the todo2code pin in subactor/platform", + "scope": "platform intent-conformance gate", + "observationSeconds": 86400, + "abortThreshold": "gate verdict differs for the same PR" + }, + "rollback": { + "mechanism": "revert the commit", + "scope": "same package", + "observationSeconds": 3600, + "abortThreshold": "verdict mismatch" + }, + "authority": { + "requestOnly": true, + "executionGrantEmbedded": false + } +} diff --git a/src/core/schema-intent-validation.ts b/src/core/schema-intent-validation.ts index 2ab25698..9dcc5b4d 100644 --- a/src/core/schema-intent-validation.ts +++ b/src/core/schema-intent-validation.ts @@ -31,6 +31,37 @@ import { assertIntentGenerationMetadata, } from './schema-generation-validation.js'; +const validatedGraphs = new WeakMap(); +let fullGraphValidations = 0; + +export function intentGraphValidationCount(): number { + return fullGraphValidations; +} + +type ValidatedGraphSnapshot = { fingerprint: string; records: unknown; relations: unknown; recordCount: number; relationCount: number }; + +/** Same object, same fingerprint, same record/relation arrays and lengths as when it last validated. */ +function isUnchangedValidatedGraph(value: unknown): boolean { + if (typeof value !== 'object' || value === null) return false; + const cached = validatedGraphs.get(value); + if (!cached) return false; + const candidate = value as { fingerprint?: unknown; records?: unknown; relations?: unknown }; + return candidate.fingerprint === cached.fingerprint + && candidate.records === cached.records + && candidate.relations === cached.relations + && sameLength(candidate.records, cached.recordCount) + && sameLength(candidate.relations, cached.relationCount); +} + +function sameLength(value: unknown, length: number): boolean { + return Array.isArray(value) && value.length === length; +} + +function rememberValidatedGraph(value: object, snapshot: ValidatedGraphSnapshot): void { + fullGraphValidations += 1; + validatedGraphs.set(value, snapshot); +} + export function assertIntentRecord(value: unknown): asserts value is IntentRecord { // #lizard forgives const record = objectValue(value, 'Intent record'); @@ -107,6 +138,7 @@ export function assertIntentRecords(values: unknown): asserts values is IntentRe } export function assertIntentGraph(value: unknown): asserts value is IntentGraph { + if (isUnchangedValidatedGraph(value)) return; const graph = objectValue(value, 'Intent graph'); exactKeys(graph, ['schemaVersion', 'generatedAt', 'fingerprint', 'records', 'relations', 'stats'], 'Intent graph'); if (graph.schemaVersion !== 't2c.graph/v1') throw new Error('Unsupported graph schemaVersion'); @@ -133,6 +165,13 @@ export function assertIntentGraph(value: unknown): asserts value is IntentGraph exactCounts(stats.byStatus, countRecords(records, (record) => record.lifecycle.status), 'Graph stats.byStatus'); const expectedFingerprint = graphFingerprint(records, graph.relations as IntentRelation[]); if (graph.fingerprint !== expectedFingerprint) throw new Error('Graph fingerprint does not match records and relations'); + rememberValidatedGraph(value as object, { + fingerprint: graph.fingerprint as string, + records: graph.records, + relations: graph.relations, + recordCount: records.length, + relationCount: (graph.relations as IntentRelation[]).length, + }); } export function assertIntentGraphDiff(value: unknown): asserts value is IntentGraphDiff { diff --git a/test/schema-validate-once.test.ts b/test/schema-validate-once.test.ts new file mode 100644 index 00000000..11815390 --- /dev/null +++ b/test/schema-validate-once.test.ts @@ -0,0 +1,56 @@ +import assert from 'node:assert/strict'; +import test from 'node:test'; +import { buildRecord } from '../src/core/record.js'; +import { linkIntentRecords } from '../src/graph/linker.js'; +import { assertIntentGraph, intentGraphValidationCount } from '../src/core/schema-intent-validation.js'; +import type { IntentGraph, IntentRecord } from '../src/core/types.js'; + +function makeGraph(): IntentGraph { + const records: IntentRecord[] = [ + buildRecord({ + kind: 'declared_intent', action: 'add', object: 'validation', text: 'Add validation', + lifecycle: 'proposed', sourceKind: 'test', sourcePath: 'TASK.md', sourceLines: { start: 1, end: 1 }, + extractor: 'test/schema', epistemicClass: 'declaration', confidence: 1, basis: ['fixture'], + }), + buildRecord({ + kind: 'declared_intent', action: 'add', object: 'parser', text: 'Add parser', + lifecycle: 'proposed', sourceKind: 'test', sourcePath: 'TASK.md', sourceLines: { start: 1, end: 1 }, + extractor: 'test/schema', epistemicClass: 'declaration', confidence: 1, basis: ['fixture'], + }), + ]; + return linkIntentRecords(records); +} + +test('An unchanged graph object is validated once even across repeated assertions', () => { + const graph = makeGraph(); + const before = intentGraphValidationCount(); + assert.doesNotThrow(() => assertIntentGraph(graph)); + assert.doesNotThrow(() => assertIntentGraph(graph)); + assert.doesNotThrow(() => assertIntentGraph(graph)); + assert.equal(intentGraphValidationCount(), before + 1); +}); + +test('A structuredClone graph is a different object and is fully validated again', () => { + const graph = makeGraph(); + assert.doesNotThrow(() => assertIntentGraph(graph)); + const before = intentGraphValidationCount(); + const clone = structuredClone(graph); + assert.doesNotThrow(() => assertIntentGraph(clone)); + assert.equal(intentGraphValidationCount(), before + 1); +}); + +test('A mutated fingerprint on an already validated graph is not hidden by the cache', () => { + const graph = makeGraph(); + assert.doesNotThrow(() => assertIntentGraph(graph)); + graph.fingerprint = '0'.repeat(64); + assert.throws(() => assertIntentGraph(graph), /fingerprint/); +}); + +test('Replacing the relations array forces a fresh full validation', () => { + const graph = makeGraph(); + assert.doesNotThrow(() => assertIntentGraph(graph)); + const before = intentGraphValidationCount(); + graph.relations = [...graph.relations]; + assert.doesNotThrow(() => assertIntentGraph(graph)); + assert.equal(intentGraphValidationCount(), before + 1); +});