Repository navigation
fix(check): bound crossing analysis work #876
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.
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
Changes from all commits
2f30539
bca9b0e
6edf8cb
3623afb
d7b6550
caa1105
0277caf
6946f12
fbf8f8a
1866558
5ba198f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -19,6 +19,16 @@ import { | |
| } from '../renderers/shared/desktop-readability.mjs'; | ||
|
|
||
| const input = process.argv[2]; | ||
| const DEFAULT_MAX_CROSSING_PAIR_CHECKS = 50_000; | ||
| const DEFAULT_MAX_CROSSING_SEGMENT_PAIR_CHECKS = 3_000_000; | ||
| const maxCrossingPairChecks = positiveIntegerEnvironment( | ||
| 'ARCHIFY_CHECK_MAX_CROSSING_PAIR_CHECKS', | ||
| DEFAULT_MAX_CROSSING_PAIR_CHECKS, | ||
| ); | ||
| const maxCrossingSegmentPairChecks = positiveIntegerEnvironment( | ||
| 'ARCHIFY_CHECK_MAX_CROSSING_SEGMENT_PAIR_CHECKS', | ||
| DEFAULT_MAX_CROSSING_SEGMENT_PAIR_CHECKS, | ||
| ); | ||
|
|
||
| if (!input || input === '-h' || input === '--help') { | ||
| console.error('Usage: node scripts/check-render-output.mjs <diagram.html>'); | ||
|
|
@@ -142,7 +152,13 @@ if (svgMatches.length === 1) { | |
| diagonal.length === 0, | ||
| diagonal.map(({ arrow, segmentIndex }) => `${arrow.kind} ${arrow.index} segment ${segmentIndex + 1}: expected an orthogonal segment or an explicitly authored direct straight route; ${arrow.raw}`), | ||
| ); | ||
| const measuredRelationshipCrossings = collectRelationshipCrossings(arrows, workflowV2); | ||
| const crossingAnalysis = collectRelationshipCrossings( | ||
| arrows, | ||
| workflowV2, | ||
| maxCrossingPairChecks, | ||
| maxCrossingSegmentPairChecks, | ||
| ); | ||
| const measuredRelationshipCrossings = crossingAnalysis.crossings; | ||
| const resolvedCrossovers = measuredRelationshipCrossings.filter((hit) => ( | ||
| hit.left.crossoverHalo && hit.right.crossoverHalo | ||
| )); | ||
|
|
@@ -213,7 +229,8 @@ if (svgMatches.length === 1) { | |
| const desktopReadabilityIsError = qualityProfile === 'showcase'; | ||
| // Certain geometry, but new: surface it as evidence first (CONTRIBUTING.md#product-and-compatibility-contracts). | ||
| const viewportHeightIsError = false; | ||
| const compositionErrors = (qualityGatesEnforced ? containerBorderRuns.length : 0) | ||
| const compositionErrors = (crossingAnalysis.exhausted ? 1 : 0) | ||
| + (qualityGatesEnforced ? containerBorderRuns.length : 0) | ||
| + (crossingIsError ? relationshipCrossings.length : 0) | ||
| + (corridorIsError ? ambiguousCorridors.length + arrowheadCollisions.length : 0) | ||
| + (labelClearanceIsError ? labelRouteClearance.length : 0) | ||
|
|
@@ -328,6 +345,17 @@ if (svgMatches.length === 1) { | |
| otherRelationship: relationshipRecord(hit.right), | ||
| point: hit.point.map((value) => Math.round(value * 10) / 10), | ||
| })), | ||
| ...(crossingAnalysis.exhausted ? [{ | ||
| severity: 'error', | ||
| code: 'composition/crossing-analysis-budget', | ||
| attemptedPairs: crossingAnalysis.attemptedPairs, | ||
| attemptedSegmentPairs: crossingAnalysis.attemptedSegmentPairs, | ||
| pairLimit: maxCrossingPairChecks, | ||
| segmentPairLimit: maxCrossingSegmentPairChecks, | ||
| exhaustedBudget: crossingAnalysis.exhaustedBudget, | ||
| detail: crossingBudgetDetail(crossingAnalysis), | ||
| supportedFixes: crossingBudgetFixes(crossingAnalysis), | ||
| }] : []), | ||
| ...ambiguousCorridors.map((hit) => ({ | ||
| severity: corridorIsError ? 'error' : 'warning', | ||
| code: 'composition/ambiguous-corridor', | ||
|
|
@@ -399,10 +427,10 @@ if (svgMatches.length === 1) { | |
| ); | ||
| addCheck( | ||
| 'relationship_crossings', | ||
| !crossingIsError || relationshipCrossings.length === 0, | ||
| relationshipCrossings.map((hit) => ( | ||
| !crossingAnalysis.exhausted && (!crossingIsError || relationshipCrossings.length === 0), | ||
| [...relationshipCrossings.map((hit) => ( | ||
| `[composition/proper-crossing] ${qualityProfile} ${relationshipName(hit.left)} crosses ${relationshipName(hit.right)} at [${formatPoint(hit.point)}]` | ||
| )), | ||
| )), ...(crossingAnalysis.exhausted ? [`[composition/crossing-analysis-budget] ${crossingBudgetDetail(crossingAnalysis)}`] : [])], | ||
| ); | ||
| addCheck( | ||
| 'relationship_corridors', | ||
|
|
@@ -734,14 +762,21 @@ function parseRoutePoints(value) { | |
| return points.length >= 2 && points.every(isPoint) ? points : null; | ||
| } | ||
|
|
||
| function collectRelationshipCrossings(arrows, includeSharedEndpoints = false) { | ||
| const relationships = arrows.filter((arrow) => arrow.from && arrow.to && arrow.segments.length).map(arrow => ({ | ||
| function collectRelationshipCrossings( | ||
| arrows, | ||
| includeSharedEndpoints = false, | ||
| pairLimit = DEFAULT_MAX_CROSSING_PAIR_CHECKS, | ||
| segmentPairLimit = DEFAULT_MAX_CROSSING_SEGMENT_PAIR_CHECKS, | ||
| ) { | ||
| const relationships = arrows.filter((arrow) => arrow.from && arrow.to).map(arrow => ({ | ||
| ...arrow, | ||
| // Readable-v2 routePoints come from the visible path, never its metadata. | ||
| // A straight-through via is not a visual endpoint; preserve real bends. | ||
| segments: includeSharedEndpoints ? arrow.routeSubpaths.flatMap(forwardCollinearAnalysisSegments) : arrow.segments, | ||
| })); | ||
| })).filter((arrow) => arrow.segments.length); | ||
| const crossings = []; | ||
| let attemptedPairs = 0; | ||
| let attemptedSegmentPairs = 0; | ||
| for (let leftIndex = 0; leftIndex < relationships.length; leftIndex += 1) { | ||
| const left = relationships[leftIndex]; | ||
| for (let rightIndex = leftIndex + 1; rightIndex < relationships.length; rightIndex += 1) { | ||
|
|
@@ -752,9 +787,17 @@ function collectRelationshipCrossings(arrows, includeSharedEndpoints = false) { | |
| && !includeSharedEndpoints | ||
| && !(left.independentPorts && right.independentPorts) | ||
| && !(left.automaticWorkflowRoute && right.automaticWorkflowRoute)) continue; | ||
| if (attemptedPairs >= pairLimit) { | ||
| return { crossings, attemptedPairs, attemptedSegmentPairs, exhausted: true, exhaustedBudget: 'relationship-pairs' }; | ||
| } | ||
| attemptedPairs += 1; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When either eligible relationship contains many path segments (for example, two imported SVG routes with thousands of supported Useful? React with 👍 / 👎. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
In a AGENTS.md reference: AGENTS.md:L3-L3 Useful? React with 👍 / 👎. |
||
| let point = null; | ||
| for (const leftSegment of left.segments) { | ||
| for (const rightSegment of right.segments) { | ||
| if (attemptedSegmentPairs >= segmentPairLimit) { | ||
| return { crossings, attemptedPairs, attemptedSegmentPairs, exhausted: true, exhaustedBudget: 'segment-pairs' }; | ||
| } | ||
| attemptedSegmentPairs += 1; | ||
| point = properSegmentIntersection(leftSegment.start, leftSegment.end, rightSegment.start, rightSegment.end); | ||
| if (point) break; | ||
| } | ||
|
|
@@ -763,7 +806,26 @@ function collectRelationshipCrossings(arrows, includeSharedEndpoints = false) { | |
| if (point) crossings.push({ left, right, point }); | ||
| } | ||
| } | ||
| return crossings; | ||
| return { crossings, attemptedPairs, attemptedSegmentPairs, exhausted: false, exhaustedBudget: null }; | ||
| } | ||
|
|
||
| function crossingBudgetDetail(analysis) { | ||
| const attempted = analysis.exhaustedBudget === 'segment-pairs' | ||
| ? `${analysis.attemptedSegmentPairs} segment pairs (limit ${maxCrossingSegmentPairChecks}) after ${analysis.attemptedPairs} relationship pairs` | ||
| : `${analysis.attemptedPairs} relationship pairs (limit ${maxCrossingPairChecks}) after ${analysis.attemptedSegmentPairs} segment pairs`; | ||
| return `Crossing analysis stopped after ${attempted}; inspection is incomplete.`; | ||
| } | ||
|
|
||
| function crossingBudgetFixes(analysis) { | ||
| return analysis.exhaustedBudget === 'segment-pairs' | ||
| ? ['increase ARCHIFY_CHECK_MAX_CROSSING_SEGMENT_PAIR_CHECKS for this known-large artifact and rerun the check'] | ||
| : ['increase ARCHIFY_CHECK_MAX_CROSSING_PAIR_CHECKS for this known-large artifact and rerun the check']; | ||
| } | ||
|
|
||
| function positiveIntegerEnvironment(name, fallback) { | ||
| const value = process.env[name]; | ||
| if (value === undefined || value === '') return fallback; | ||
| return /^[1-9]\d*$/.test(value) && Number.isSafeInteger(Number(value)) ? Number(value) : fallback; | ||
| } | ||
|
|
||
| function relationshipName(arrow) { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,18 @@ | ||
| import fs from 'node:fs'; | ||
| import path from 'node:path'; | ||
|
|
||
| const output = process.argv[2]; | ||
| if (!output) throw new Error('Usage: node test/helpers/crossing-budget-fixture.mjs <output.html>'); | ||
|
|
||
| const arrows = Array.from({ length: 120 }, (_, index) => { | ||
| const y = 10 + index * 3; | ||
| const points = Array.from({ length: 101 }, (_, step) => [ | ||
| 20 + Math.ceil(step / 2) * 20, | ||
| y + Math.floor(step / 2) % 2, | ||
| ]); | ||
| const d = `M${points.map(([x, pointY], step) => step ? `L${x} ${pointY}` : `${x} ${pointY}`).join('')}`; | ||
| return `<path id="r${index}" data-edge-id="r${index}" data-edge-from="from-${index}" data-edge-to="to-${index}" d="${d}" class="a-default" marker-end="url(#arrowhead)"/>`; | ||
| }).join(''); | ||
|
|
||
| fs.mkdirSync(path.dirname(path.resolve(output)), { recursive: true }); | ||
| fs.writeFileSync(output, `<!doctype html><svg viewBox="0 0 2100 400" data-quality-profile="standard">${arrows}</svg>`); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When this budget is hit through the documented
archify deliver <type> <input.json> ... [--json]orfinalizeflows,checkerDiagnostics()callscompositionFixes(), butarchify/bin/archify.mjs:2675-2683,2727-2729has no mapping for this new code. WithARCHIFY_CHECK_MAX_CROSSING_PAIR_CHECKS=1onevent-stream.dataflow.json, the emitted diagnostic hadsupportedFixes: [], leaving the rerun instruction only inevidence.detail; add this code to the mapping so agent-facing failures expose the executable recovery insupportedFixes.AGENTS.md reference: AGENTS.md:L3-L3
Useful? React with 👍 / 👎.