Skip to content

Commit 5db7bc2

Browse files
aswamyaswamy
committed
Run liquid check on files LLM touched
1 parent 3e50116 commit 5db7bc2

File tree

5 files changed

+159
-80
lines changed

5 files changed

+159
-80
lines changed

‎src/tools/index.ts

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -634,26 +634,33 @@ function liquidMcpTools(server: McpServer) {
634634
absoluteThemePath: z
635635
.string()
636636
.describe("The absolute path to the theme directory"),
637+
filesCreatedOrUpdated: z
638+
.array(z.string())
639+
.describe(
640+
"An array of relative file paths that was generated or updated by the LLM. The file paths should be relative to the theme directory.",
641+
),
637642
}),
638643

639644
async (params) => {
640-
const validationResponse = await validateTheme(params.absoluteThemePath);
641-
642-
recordUsage("validate_theme", params, validationResponse).catch(() => {});
645+
const validationResponses = await validateTheme(
646+
params.absoluteThemePath,
647+
params.filesCreatedOrUpdated,
648+
);
643649

644-
const responseText = formatValidationResult(
645-
[validationResponse],
646-
"Theme",
650+
recordUsage("validate_theme", params, validationResponses).catch(
651+
() => {},
647652
);
648653

654+
const responseText = formatValidationResult(validationResponses, "Theme");
655+
649656
return {
650657
content: [
651658
{
652659
type: "text" as const,
653660
text: responseText,
654661
},
655662
],
656-
isError: validationResponse.result === ValidationResult.FAILED,
663+
isError: hasFailedValidation(validationResponses),
657664
};
658665
},
659666
);

‎src/validations/theme.test.ts

Lines changed: 91 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -31,40 +31,47 @@ describe("validateTheme", () => {
3131

3232
it("should successfully validate a theme", async () => {
3333
// Create the test.liquid file with the specified content
34-
const liquidFile = join(snippetsDirectory, "test.liquid");
35-
await writeFile(liquidFile, "{{ 'hello' }}");
34+
const relativeFilePath = join("snippets", "test.liquid");
35+
const filePath = join(snippetsDirectory, "test.liquid");
36+
await writeFile(filePath, "{{ 'hello' }}");
3637

3738
// Run validateTheme on the temporary directory
38-
const result = await validateTheme(tempThemeDirectory);
39+
const responses = await validateTheme(tempThemeDirectory, [
40+
relativeFilePath,
41+
]);
3942

40-
// Assert the response was a success
41-
expect(result.result).toBe(ValidationResult.SUCCESS);
42-
expect(result.resultDetail).toBe(
43-
`Theme at ${tempThemeDirectory} passed all checks from Shopify's Theme Check.`,
44-
);
43+
expect(responses).toContainEqual({
44+
result: ValidationResult.SUCCESS,
45+
resultDetail: `Theme file ${relativeFilePath} passed all checks from Shopify's Theme Check.`,
46+
});
4547
});
4648

4749
it("should fail to validate a theme with an unknown filter", async () => {
4850
// Create the test.liquid file with the specified content
49-
const liquidFile = join(snippetsDirectory, "test.liquid");
50-
await writeFile(liquidFile, "{{ 'hello' | non-existent-filter }}");
51+
const relativeFilePath = join("snippets", "test.liquid");
52+
const filePath = join(snippetsDirectory, "test.liquid");
53+
await writeFile(filePath, "{{ 'hello' | non-existent-filter }}");
5154

5255
// Run validateTheme on the temporary directory
53-
const result = await validateTheme(tempThemeDirectory);
56+
const responses = await validateTheme(tempThemeDirectory, [
57+
relativeFilePath,
58+
]);
5459

55-
// Assert the response was a success
56-
expect(result.result).toBe(ValidationResult.FAILED);
57-
expect(result.resultDetail).toContain(
58-
"Unknown filter 'non-existent-filter' used.",
59-
);
60+
expect(responses).toContainEqual({
61+
result: ValidationResult.FAILED,
62+
resultDetail: `Theme file ${relativeFilePath} failed to validate:
63+
64+
ERROR: Unknown filter 'non-existent-filter' used.`,
65+
});
6066
});
6167

6268
it("should fail to validate a theme with an invalid schema", async () => {
6369
// Create the test.liquid file with the specified content
64-
const liquidFile = join(blocksDirectory, "test.liquid");
70+
const relativeFilePath = join("blocks", "test.liquid");
71+
const filePath = join(blocksDirectory, "test.liquid");
6572
const schemaName = "Long long long long long long name";
6673
await writeFile(
67-
liquidFile,
74+
filePath,
6875
`
6976
{% schema %}
7077
{
@@ -74,30 +81,83 @@ describe("validateTheme", () => {
7481
);
7582

7683
// Run validateTheme on the temporary directory
77-
const result = await validateTheme(tempThemeDirectory);
84+
const responses = await validateTheme(tempThemeDirectory, [
85+
relativeFilePath,
86+
]);
87+
88+
expect(responses).toContainEqual({
89+
result: ValidationResult.FAILED,
90+
resultDetail: `Theme file ${relativeFilePath} failed to validate:
7891
79-
// Assert the response was a success
80-
expect(result.result).toBe(ValidationResult.FAILED);
81-
expect(result.resultDetail).toContain(
82-
`Schema name '${schemaName}' is too long (max 25 characters)`,
92+
ERROR: Schema name '${schemaName}' is too long (max 25 characters)`,
93+
});
94+
});
95+
96+
it("should fail to validate a theme with LiquidDoc errors", async () => {
97+
const relativeSnippetFilePath = join("snippets", "example-snippet.liquid");
98+
const snippetFilePath = join(snippetsDirectory, "example-snippet.liquid");
99+
await writeFile(
100+
snippetFilePath,
101+
"{% doc %} @param {string} param {% enddoc %} {{ param }}",
83102
);
103+
104+
const relativeBlockFilePath = join("blocks", "test.liquid");
105+
const blockFilePath = join(blocksDirectory, "test.liquid");
106+
await writeFile(blockFilePath, `{% render 'example-snippet' %}`);
107+
108+
// Run validateTheme on the temporary directory
109+
const responses = await validateTheme(tempThemeDirectory, [
110+
relativeSnippetFilePath,
111+
relativeBlockFilePath,
112+
]);
113+
114+
expect(responses).toContainEqual({
115+
result: ValidationResult.FAILED,
116+
resultDetail: `Theme file ${relativeBlockFilePath} failed to validate:
117+
118+
ERROR: Missing required argument 'param' in render tag for snippet 'example-snippet'.; SUGGESTED FIXES: Add required argument 'param'.`,
119+
});
84120
});
85121

86122
it("should successfully validate a theme with an unknown filter if its check is exempted", async () => {
87123
// Create the test.liquid file with the specified content
88-
const liquidFile = join(snippetsDirectory, "test.liquid");
89-
await writeFile(liquidFile, "{{ 'hello' | non-existent-filter }}");
124+
const relativeSnippetFilePath = join("snippets", "test.liquid");
125+
const snippetFilePath = join(snippetsDirectory, "test.liquid");
126+
await writeFile(snippetFilePath, "{{ 'hello' | non-existent-filter }}");
90127

91128
const themeCheckYml = join(tempThemeDirectory, ".theme-check.yml");
92129
await writeFile(themeCheckYml, "ignore:\n- snippets/test.liquid");
93130

94131
// Run validateTheme on the temporary directory
95-
const result = await validateTheme(tempThemeDirectory);
132+
const responses = await validateTheme(tempThemeDirectory, [
133+
relativeSnippetFilePath,
134+
]);
96135

97-
// Assert the response was a success
98-
expect(result.result).toBe(ValidationResult.SUCCESS);
99-
expect(result.resultDetail).toBe(
100-
`Theme at ${tempThemeDirectory} passed all checks from Shopify's Theme Check.`,
101-
);
136+
expect(responses).toContainEqual({
137+
result: ValidationResult.SUCCESS,
138+
resultDetail: `Theme file ${relativeSnippetFilePath} passed all checks from Shopify's Theme Check.`,
139+
});
140+
});
141+
142+
it("should fail to validate only files that were touched by the LLM", async () => {
143+
for (let i = 0; i < 5; i++) {
144+
const snippetFilePath = join(snippetsDirectory, `test-${i}.liquid`);
145+
await writeFile(snippetFilePath, "{{ 'hello' | non-existent-filter }}");
146+
}
147+
148+
const relativeSnippetFilePath = join("snippets", "test-0.liquid");
149+
150+
// Run validateTheme on the temporary directory
151+
const responses = await validateTheme(tempThemeDirectory, [
152+
relativeSnippetFilePath,
153+
]);
154+
155+
expect(responses).toHaveLength(1);
156+
expect(responses).toContainEqual({
157+
result: ValidationResult.FAILED,
158+
resultDetail: `Theme file ${relativeSnippetFilePath} failed to validate:
159+
160+
ERROR: Unknown filter 'non-existent-filter' used.`,
161+
});
102162
});
103163
});

‎src/validations/theme.ts

Lines changed: 50 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,18 @@
1-
import { themeCheckRun } from "@shopify/theme-check-node";
1+
import { Offense, themeCheckRun } from "@shopify/theme-check-node";
22
import { access } from "fs/promises";
3-
import { join } from "path";
3+
import { join, normalize } from "path";
44
import { ValidationResponse, ValidationResult } from "../types.js";
55

66
/**
77
* Validates Shopify Theme
88
* @param absoluteThemePath - The path to the theme directory
9+
* @param filesCreatedOrUpdated - An array of relative file paths that was generated or updated by the LLM. The file paths should be relative to the theme directory.
910
* @returns ValidationResponse containing the success of running theme-check for the whole theme
1011
*/
1112
export default async function validateTheme(
1213
absoluteThemePath: string,
13-
): Promise<ValidationResponse> {
14+
filesCreatedOrUpdated: string[],
15+
): Promise<ValidationResponse[]> {
1416
try {
1517
let configPath: string | undefined = join(
1618
absoluteThemePath,
@@ -29,24 +31,54 @@ export default async function validateTheme(
2931
(message) => console.error(message),
3032
);
3133

32-
if (results.offenses.length > 0) {
33-
const formattedOffenses = results.offenses
34-
.map((offense) => `${offense.uri}: ${offense.message}`)
35-
.join("\n");
36-
return {
37-
result: ValidationResult.FAILED,
38-
resultDetail: `Theme at ${absoluteThemePath} failed to validate:\n\n${formattedOffenses}`,
39-
};
34+
const groupedOffensesByFileUri = groupOffensesByFileUri(results.offenses);
35+
36+
const responses: ValidationResponse[] = [];
37+
38+
for (const relativeFilePath of filesCreatedOrUpdated) {
39+
const uri = Object.keys(groupedOffensesByFileUri).find((uri) =>
40+
normalize(uri).endsWith(normalize(relativeFilePath)),
41+
);
42+
if (uri) {
43+
responses.push({
44+
result: ValidationResult.FAILED,
45+
resultDetail: `Theme file ${relativeFilePath} failed to validate:\n\n${groupedOffensesByFileUri[uri].join("\n")}`,
46+
});
47+
} else {
48+
responses.push({
49+
result: ValidationResult.SUCCESS,
50+
resultDetail: `Theme file ${relativeFilePath} passed all checks from Shopify's Theme Check.`,
51+
});
52+
}
4053
}
4154

42-
return {
43-
result: ValidationResult.SUCCESS,
44-
resultDetail: `Theme at ${absoluteThemePath} passed all checks from Shopify's Theme Check.`,
45-
};
55+
return responses;
4656
} catch (error) {
47-
return {
57+
return filesCreatedOrUpdated.map((filePath) => ({
4858
result: ValidationResult.FAILED,
49-
resultDetail: `Validation error: Could not validate ${absoluteThemePath}. Details: ${error instanceof Error ? error.message : String(error)}`,
50-
};
59+
resultDetail: `Validation error: Could not validate ${filePath}. Details: ${error instanceof Error ? error.message : String(error)}`,
60+
}));
5161
}
5262
}
63+
64+
export function groupOffensesByFileUri(offenses: Offense[]) {
65+
return offenses.reduce(
66+
(acc, o) => {
67+
let formattedMessage = `ERROR: ${o.message}`;
68+
69+
if (o.suggest && o.suggest.length > 0) {
70+
formattedMessage += `; SUGGESTED FIXES: ${o.suggest.map((s) => s.message).join("OR ")}.`;
71+
}
72+
73+
const uri = o.uri;
74+
75+
if (acc[uri]) {
76+
acc[uri].push(formattedMessage);
77+
} else {
78+
acc[uri] = [formattedMessage];
79+
}
80+
return acc;
81+
},
82+
{} as Record<string, string[]>,
83+
);
84+
}

‎src/validations/themeCodeBlock.test.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -147,7 +147,7 @@ describe("validateThemeCodeblocks", () => {
147147
expect(result).toContainEqual({
148148
result: ValidationResult.FAILED,
149149
resultDetail:
150-
"Theme codeblock test.liquid has the following offenses from using Shopify's Theme Check:\n\nERROR: The variable 'some_var' is assigned but not used; SUGGESTED FIXES: Remove the unused variable 'some_var'",
150+
"Theme codeblock test.liquid has the following offenses from using Shopify's Theme Check:\n\nERROR: The variable 'some_var' is assigned but not used; SUGGESTED FIXES: Remove the unused variable 'some_var'.",
151151
});
152152
});
153153

@@ -219,7 +219,7 @@ ERROR: Schema name '${schemaName}' is too long (max 25 characters)`,
219219
result: ValidationResult.FAILED,
220220
resultDetail: `Theme codeblock test.liquid has the following offenses from using Shopify's Theme Check:
221221
222-
ERROR: Missing required argument 'param' in render tag for snippet 'example-snippet'.; SUGGESTED FIXES: Add required argument 'param'`,
222+
ERROR: Missing required argument 'param' in render tag for snippet 'example-snippet'.; SUGGESTED FIXES: Add required argument 'param'.`,
223223
});
224224
});
225225
});

‎src/validations/themeCodeBlock.ts

Lines changed: 2 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ import {
77
FileTuple,
88
FileType,
99
LiquidHtmlNode,
10-
Offense,
1110
path,
1211
recommended,
1312
SectionSchema,
@@ -19,6 +18,7 @@ import {
1918
import { ThemeLiquidDocsManager } from "@shopify/theme-check-docs-updater";
2019
import { normalize } from "path";
2120
import { ValidationResponse, ValidationResult } from "../types.js";
21+
import { groupOffensesByFileUri } from "./theme.js";
2222

2323
type ThemeCodeblock = {
2424
fileName: string;
@@ -69,7 +69,7 @@ async function validatePartialTheme(
6969
if (fileUriToOffenses[uri]) {
7070
validationResults.push({
7171
result: ValidationResult.FAILED,
72-
resultDetail: `Theme codeblock ${name} has the following offenses from using Shopify's Theme Check:\n\n${fileUriToOffenses[uri].join("\n\n")}`,
72+
resultDetail: `Theme codeblock ${name} has the following offenses from using Shopify's Theme Check:\n\n${fileUriToOffenses[uri].join("\n")}`,
7373
});
7474
} else {
7575
validationResults.push({
@@ -157,26 +157,6 @@ function createTheme(codeblocks: ThemeCodeblock[]): Theme {
157157
}, {} as Theme);
158158
}
159159

160-
function groupOffensesByFileUri(offenses: Offense[]) {
161-
return offenses.reduce(
162-
(acc, o) => {
163-
let formattedMessage = `ERROR: ${o.message}`;
164-
165-
if (o.suggest && o.suggest.length > 0) {
166-
formattedMessage += `; SUGGESTED FIXES: ${o.suggest.map((s) => s.message).join("OR ")}`;
167-
}
168-
169-
if (acc[o.uri]) {
170-
acc[o.uri].push(formattedMessage);
171-
} else {
172-
acc[o.uri] = [formattedMessage];
173-
}
174-
return acc;
175-
},
176-
{} as Record<string, string[]>,
177-
);
178-
}
179-
180160
// We mimic a theme on a file system to be able to run theme checks
181161
class MockFileSystem implements AbstractFileSystem {
182162
constructor(private partialTheme: Theme) {}

0 commit comments

Comments
 (0)