Skip to content

Commit cf019f4

Browse files
Bill LeoutsakosBill Leoutsakos
authored andcommitted
fix(design): explain unchecked source diagnostics
1 parent 24a69df commit cf019f4

8 files changed

Lines changed: 123 additions & 11 deletions

scripts/check-design-conformance-appearance.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
/* biome-ignore-all lint/suspicious/noTemplateCurlyInString: Fixtures contain literal proposed JavaScript templates. */
1+
/** biome-ignore-all lint/suspicious/noTemplateCurlyInString: Fixtures contain literal proposed JavaScript templates. */
22
import { existsSync } from 'node:fs'
33
import { beforeAll, expect, test } from 'vitest'
44
import { extract } from '#design-conformance/extract'

scripts/check-design-conformance-command.test.ts

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,43 @@ test('CI tolerates genuine findings and preserves the report, but fails missing
104104
expect(run(['--repo', repo, '--base', noCentral, '--head', noCentral], ci).status).toBe(2)
105105
})
106106

107+
test.each([
108+
{ label: 'parser failure', source: 'const A=()=> <p className={', reason: 'Parser failure' },
109+
{
110+
label: 'source limit',
111+
source: ' '.repeat(2 * 1024 * 1024 + 1),
112+
reason: 'Source exceeds the 2 MiB parsing limit',
113+
},
114+
])('skipped changed files remain actionable without findings: $label', ({ source, reason }) => {
115+
const { repo, head: base } = fixture()
116+
writeFileSync(path.join(repo, ui), source)
117+
commit(repo)
118+
const args = ['--repo', repo, '--base', base]
119+
const output = path.join(repo, 'report.json')
120+
const human = run([...args, '--output', output])
121+
expect(human.status).toBe(0)
122+
expect(human.stdout).toContain('coverage incomplete')
123+
expect(human.stdout).toContain(`${ui}:1 (after`)
124+
expect(human.stdout).toContain(reason)
125+
const report: Report = JSON.parse(readFileSync(output, 'utf8'))
126+
expect(report.status).toBe('completed')
127+
expect(report.flagged).toBe(false)
128+
expect(report.findings).toEqual([])
129+
expect(report.unchecked).toEqual(
130+
expect.arrayContaining([
131+
expect.objectContaining({ file: ui, side: 'after', reason: expect.stringContaining(reason) }),
132+
])
133+
)
134+
const summary = path.join(repo, 'summary.md')
135+
const child = run(args, ci, { GITHUB_ACTIONS: 'true', GITHUB_STEP_SUMMARY: summary })
136+
expect(child.status).toBe(0)
137+
expect(child.stdout).toContain(`${ui}:1 (after`)
138+
expect(child.stderr).not.toContain('::warning')
139+
const markdown = readFileSync(summary, 'utf8')
140+
expect(markdown).toContain(`${ui}:1 (after`)
141+
expect(markdown).toContain(reason)
142+
})
143+
107144
test.each([
108145
[0, null, { status: 'completed', flagged: false, findings: [] }, 0],
109146
[1, null, { status: 'completed', flagged: true, findings: [{}] }, 0],
@@ -194,6 +231,49 @@ test('GitHub annotations escape source values and normal logs cannot inject work
194231
expect(githubSummary(report)).toContain('| Usage violations | 1 |')
195232
})
196233

234+
test('unchecked diagnostics preserve both sides and safely render source-authored text', () => {
235+
const report = new ConformanceLinter().report(null)
236+
for (const side of ['before', 'after'])
237+
report.unchecked.push({
238+
file: 'path`|</pre>\n::error::injected.tsx',
239+
line: 7,
240+
side,
241+
context: 'className',
242+
reason: 'Unknown helper: <script>& value\r\n::warning::injected',
243+
})
244+
const original = JSON.stringify(report)
245+
const text = textReport(report)
246+
const markdown = githubSummary(report)
247+
expect(text).toContain(':7 (before; className)')
248+
expect(text).toContain(':7 (after; className)')
249+
expect(text).not.toMatch(/[\r\n]::(?:error|warning)::/)
250+
expect(markdown).toContain('&lt;/pre&gt;')
251+
expect(markdown).toContain('&lt;script&gt;&amp; value')
252+
expect(markdown).not.toContain('<script>')
253+
expect(markdown).not.toMatch(/[\r\n]::(?:error|warning)::/)
254+
expect(githubAnnotations(report)).toEqual([])
255+
expect(JSON.stringify(report)).toBe(original)
256+
})
257+
258+
test('large unchecked summaries show explicit limits while logs retain every complete diagnostic', () => {
259+
const report = new ConformanceLinter().report(null)
260+
report.unchecked = Array.from({ length: 101 }, (_, index) => ({
261+
file: `component-${index}.tsx`,
262+
line: 1,
263+
side: 'after',
264+
context: '',
265+
reason: index === 0 ? `${'&'.repeat(2000)} complete-long-diagnostic` : `reason-${index}`,
266+
}))
267+
const markdown = githubSummary(report)
268+
const text = textReport(report)
269+
expect(markdown).toContain('Showing 100 of 101 diagnostics')
270+
expect(markdown).toContain('[truncated; see check log]')
271+
expect(markdown).not.toContain('complete-long-diagnostic')
272+
expect(markdown).not.toContain('component-100.tsx')
273+
expect(text).toContain('complete-long-diagnostic')
274+
expect(text).toContain('component-100.tsx:1 (after) — reason-100')
275+
})
276+
197277
test('system edits remain flagged but are reported as design review warnings', () => {
198278
const report = findingReport()
199279
report.findings[0].kind = 'system-change'

scripts/check-design-conformance-composition.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
/* biome-ignore-all lint/suspicious/noTemplateCurlyInString: Proposed source fixtures. */
1+
/** biome-ignore-all lint/suspicious/noTemplateCurlyInString: Proposed source fixtures. */
22
import { expect, test } from 'vitest'
33
import { ConformanceLinter } from '#design-conformance/conformance'
44
import { extract } from '#design-conformance/extract'

scripts/check-design-conformance-refactors.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
/* biome-ignore-all lint/suspicious/noTemplateCurlyInString: Proposed source fixtures. */
1+
/** biome-ignore-all lint/suspicious/noTemplateCurlyInString: Proposed source fixtures. */
22
import { expect, test } from 'vitest'
33
import { ConformanceLinter } from '#design-conformance/conformance'
44
import { type Change, type Entry, hash, TOKEN_FILE } from '#design-conformance/model'

scripts/check-design-conformance.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
/* biome-ignore-all lint/suspicious/noTemplateCurlyInString: Fixtures contain proposed source text. */
1+
/** biome-ignore-all lint/suspicious/noTemplateCurlyInString: Fixtures contain proposed source text. */
22
import { expect, test } from 'vitest'
33
import { ConformanceLinter } from '#design-conformance/conformance'
44
import { designSystem } from '#design-conformance/design-system'

scripts/design-conformance/README.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
# Design conformance check
22

3-
Tool **3.3.0**, policy **design-conformance/1.3.0**. This is the maintained implementation of Sim's design guardrails.
3+
Tool **3.3.1**, policy **design-conformance/1.3.0**. This is the maintained implementation of Sim's design guardrails.
44

55
From the repository root, using Bun **1.4.1** and the normal root installation:
66

@@ -16,7 +16,7 @@ Use the actual PR target for `--base`; it is required. The repository defaults t
1616

1717
- **Usage violations** identify newly introduced inputs that violate an explicit central design contract, the authoritative definition, and the permitted token/component mechanism. Fix confirmed violations through that mechanism.
1818
- **System changes** identify edits to central definitions or this registry. Review them as changes to the design system; central authoring may introduce new styling.
19-
- **Unchecked inputs** describe unresolved syntax or coverage limits. They do not create findings, and a clean result does not prove exhaustive conformance.
19+
- **Unchecked inputs** describe unresolved syntax or coverage limits. They do not create findings, and a clean result does not prove exhaustive conformance. Text output identifies incomplete coverage and lists every diagnostic with its file, line, comparison side, context and reason. The CI summary includes an expandable list of the first 100 diagnostics, with long entries shortened; complete details remain in the check log and JSON report.
2020

2121
The ordinary command exits **0** for no findings, **1** for findings and **2** for operational failure. JSON retains `flagged: true` even during the warning-only rollout; failed reports have `flagged: null`.
2222

scripts/design-conformance/model.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import { createHash } from 'node:crypto'
22
import { readdirSync, readFileSync } from 'node:fs'
33
import type { Route, SourceSummary } from '#design-conformance/source-summary'
44

5-
export const VERSION = '3.3.0'
5+
export const VERSION = '3.3.1'
66
export const CATALOGUE_VERSION = '1.0.0'
77
export type Policy = 'appearance' | 'tokens' | 'conformance'
88
export const policyVersion = (policy: Policy) =>

scripts/design-conformance/reporting.ts

Lines changed: 36 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -25,12 +25,18 @@ function description(finding: Finding): string {
2525
return parts.join('; ')
2626
}
2727

28+
function uncheckedDescription(note: Report['unchecked'][number]): string {
29+
return line(
30+
`${note.file}:${note.line} (${note.side}${note.context ? `; ${note.context}` : ''}) — ${note.reason}`
31+
)
32+
}
33+
2834
export function textReport(report: Report): string {
2935
if (report.status === 'failed')
3036
return `Design check failed: ${line(report.error ?? 'Operational failure')}\n`
3137
const counts = findingCounts(report)
3238
const lines = [
33-
`Design check: ${report.flagged ? 'findings reported' : 'no new findings'} (${report.policyVersion}).`,
39+
`Design check: ${report.flagged ? 'findings reported' : 'no new findings'}${report.unchecked.length ? '; coverage incomplete' : ''} (${report.policyVersion}).`,
3440
`Usage violations: ${counts.usage}; system changes: ${counts.system}; unchecked diagnostics: ${report.unchecked.length}.`,
3541
]
3642
for (const [kind, title] of [
@@ -47,10 +53,13 @@ export function textReport(report: Report): string {
4753
)
4854
}
4955
}
50-
if (report.unchecked.length)
56+
if (report.unchecked.length) {
57+
lines.push('', 'Unchecked inputs — coverage incomplete')
58+
for (const note of report.unchecked) lines.push(` ${uncheckedDescription(note)}`)
5159
lines.push(
5260
'Unchecked inputs remain outside the result; no findings does not prove complete coverage.'
5361
)
62+
}
5463
return `${lines.join('\n')}\n`
5564
}
5665

@@ -73,7 +82,7 @@ export function githubAnnotations(report: Report): string[] {
7382

7483
export function githubSummary(report: Report): string {
7584
const counts = findingCounts(report)
76-
return [
85+
const lines = [
7786
'### Design conformance',
7887
'',
7988
report.status === 'failed'
@@ -91,5 +100,28 @@ export function githubSummary(report: Report): string {
91100
'',
92101
'Full findings and authoritative sources are in the check log. Unchecked inputs do not establish conformance.',
93102
'',
94-
].join('\n')
103+
]
104+
if (report.unchecked.length) {
105+
/** Bound summary size; the check log retains every complete diagnostic. */
106+
const notes = report.unchecked.slice(0, 100)
107+
lines.push(
108+
'<details>',
109+
`<summary>Unchecked inputs (${report.unchecked.length}) — coverage incomplete</summary>`,
110+
'',
111+
`Showing ${notes.length} of ${report.unchecked.length} diagnostics. Long entries are truncated here. Full details are in the check log; these are not design violations.`,
112+
'',
113+
'<pre>',
114+
...notes.map((note) => {
115+
const text = uncheckedDescription(note)
116+
const excerpt =
117+
text.length > 1000 ? `${text.slice(0, 1000)}… [truncated; see check log]` : text
118+
/** Source text must not close the HTML block or introduce Markdown formatting. */
119+
return excerpt.replaceAll('&', '&amp;').replaceAll('<', '&lt;').replaceAll('>', '&gt;')
120+
}),
121+
'</pre>',
122+
'</details>',
123+
''
124+
)
125+
}
126+
return lines.join('\n')
95127
}

0 commit comments

Comments
 (0)