Accept the object form of lint rules in the configuration schema - #1288
Conversation
Signed-off-by: Juan Cruz Viotti <jv@jviotti.com>
🤖 Augment PR SummarySummary: This PR adds object-form custom lint-rule configuration for Sourcemeta One collections. Changes:
Technical Notes: Existing string-form rules remain supported; the new form relies on the existing runtime scope mapping to 🤖 Was this summary useful? React with 👍 or 👎 |
There was a problem hiding this comment.
2 issues found across 14 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="test/unit/configuration/stub/collections/lint/jsonschema.json">
<violation number="1" location="test/unit/configuration/stub/collections/lint/jsonschema.json:5">
P2: This stub references `./rules/root_rule.json` and `./schemas`, but neither exists: `test/unit/configuration/stub/collections/lint/` contains only `jsonschema.json`. The PR's own unit test (`lint_rules_accept_the_object_form_from_an_included_manifest`) asserts the lint rule resolves to `stub/collections/lint/rules/root_rule.json` and the collection absolute path to `stub/collections/lint/schemas`; the comparison uses `weakly_canonical`, which silently tolerates missing paths, so the unit test can pass while the fixture stays broken. As soon as this collection is actually indexed, `load_custom_lint_rules` (src/index/generators.h) opens the rule file and fails on the missing `rules/root_rule.json`. Add the missing fixture files under `collections/lint/` (a valid lint rule at `rules/root_rule.json` and the `schemas/` directory with at least one schema), mirroring the other stub collections.</violation>
</file>
<file name="enterprise/e2e/html/one.json">
<violation number="1" location="enterprise/e2e/html/one.json:9">
P2: Adding the jsonld-type rule introduces a new lint error on `test/jsonld/product` (its top-level `x-jsonld-type` is `Product`, not `Thing`). The aggregate health of the `test` directory is asserted as exactly 21 in `list.all.hurl` and the MCP list endpoint in `mcp-2025-03-26.all.hurl:786-794`, and neither file is updated in this PR. Verify the directory aggregate still computes to 21 after this rule is active; if it shifts, update those assertions.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| "title": "With a root only lint rule", | ||
| "path": "./schemas", | ||
| "lint": { | ||
| "rules": [ { "path": "./rules/root_rule.json", "topLevel": true } ] |
There was a problem hiding this comment.
P2: This stub references ./rules/root_rule.json and ./schemas, but neither exists: test/unit/configuration/stub/collections/lint/ contains only jsonschema.json. The PR's own unit test (lint_rules_accept_the_object_form_from_an_included_manifest) asserts the lint rule resolves to stub/collections/lint/rules/root_rule.json and the collection absolute path to stub/collections/lint/schemas; the comparison uses weakly_canonical, which silently tolerates missing paths, so the unit test can pass while the fixture stays broken. As soon as this collection is actually indexed, load_custom_lint_rules (src/index/generators.h) opens the rule file and fails on the missing rules/root_rule.json. Add the missing fixture files under collections/lint/ (a valid lint rule at rules/root_rule.json and the schemas/ directory with at least one schema), mirroring the other stub collections.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At test/unit/configuration/stub/collections/lint/jsonschema.json, line 5:
<comment>This stub references `./rules/root_rule.json` and `./schemas`, but neither exists: `test/unit/configuration/stub/collections/lint/` contains only `jsonschema.json`. The PR's own unit test (`lint_rules_accept_the_object_form_from_an_included_manifest`) asserts the lint rule resolves to `stub/collections/lint/rules/root_rule.json` and the collection absolute path to `stub/collections/lint/schemas`; the comparison uses `weakly_canonical`, which silently tolerates missing paths, so the unit test can pass while the fixture stays broken. As soon as this collection is actually indexed, `load_custom_lint_rules` (src/index/generators.h) opens the rule file and fails on the missing `rules/root_rule.json`. Add the missing fixture files under `collections/lint/` (a valid lint rule at `rules/root_rule.json` and the `schemas/` directory with at least one schema), mirroring the other stub collections.</comment>
<file context>
@@ -0,0 +1,7 @@
+ "title": "With a root only lint rule",
+ "path": "./schemas",
+ "lint": {
+ "rules": [ { "path": "./rules/root_rule.json", "topLevel": true } ]
+ }
+}
</file context>
| "rules": [ "./rules/camelcase.json" ] | ||
| "rules": [ | ||
| "./rules/camelcase.json", | ||
| { "path": "./rules/jsonld-type.json", "topLevel": true } |
There was a problem hiding this comment.
P2: Adding the jsonld-type rule introduces a new lint error on test/jsonld/product (its top-level x-jsonld-type is Product, not Thing). The aggregate health of the test directory is asserted as exactly 21 in list.all.hurl and the MCP list endpoint in mcp-2025-03-26.all.hurl:786-794, and neither file is updated in this PR. Verify the directory aggregate still computes to 21 after this rule is active; if it shifts, update those assertions.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At enterprise/e2e/html/one.json, line 9:
<comment>Adding the jsonld-type rule introduces a new lint error on `test/jsonld/product` (its top-level `x-jsonld-type` is `Product`, not `Thing`). The aggregate health of the `test` directory is asserted as exactly 21 in `list.all.hurl` and the MCP list endpoint in `mcp-2025-03-26.all.hurl:786-794`, and neither file is updated in this PR. Verify the directory aggregate still computes to 21 after this rule is active; if it shifts, update those assertions.</comment>
<file context>
@@ -4,7 +4,10 @@
- "rules": [ "./rules/camelcase.json" ]
+ "rules": [
+ "./rules/camelcase.json",
+ { "path": "./rules/jsonld-type.json", "topLevel": true }
+ ]
}
</file context>
There was a problem hiding this comment.
Benchmark (community)
Details
| Benchmark suite | Current: fac91e6 | Previous: 3191145 | Ratio |
|---|---|---|---|
Add one schema (0 existing) |
232 ms |
242 ms |
0.96 |
Add one schema (100 existing) |
33 ms |
39 ms |
0.85 |
Add one schema (1000 existing) |
101 ms |
94 ms |
1.07 |
Add one schema (10000 existing) |
758 ms |
716 ms |
1.06 |
Update one schema (1 existing) |
26 ms |
30 ms |
0.87 |
Update one schema (101 existing) |
34 ms |
40 ms |
0.85 |
Update one schema (1001 existing) |
97 ms |
96 ms |
1.01 |
Update one schema (10001 existing) |
736 ms |
743 ms |
0.99 |
Cached rebuild (1 existing) |
7 ms |
11 ms |
0.64 |
Cached rebuild (101 existing) |
8 ms |
13 ms |
0.62 |
Cached rebuild (1001 existing) |
24 ms |
38 ms |
0.63 |
Cached rebuild (10001 existing) |
195 ms |
300 ms |
0.65 |
Index 100 schemas |
406 ms |
522 ms |
0.78 |
Index 1000 schemas |
1090 ms |
1469 ms |
0.74 |
Index 10000 schemas |
10630 ms |
12756 ms |
0.83 |
Index 10000 schemas (custom meta-schema) |
12629 ms |
15510 ms |
0.81 |
Index 10000 schemas ($ref fan-out) |
12789 ms |
15603 ms |
0.82 |
test/e2e/html: Schema Fetch (p50) |
280 us |
396 us |
0.71 |
test/e2e/html: Schema Fetch (p99) |
369 us |
481 us |
0.77 |
This comment was automatically generated by workflow using github-action-benchmark.
There was a problem hiding this comment.
Benchmark (enterprise)
Details
| Benchmark suite | Current: fac91e6 | Previous: 3191145 | Ratio |
|---|---|---|---|
Add one schema (0 existing) |
342 ms |
344 ms |
0.99 |
Add one schema (100 existing) |
118 ms |
116 ms |
1.02 |
Add one schema (1000 existing) |
168 ms |
177 ms |
0.95 |
Add one schema (10000 existing) |
774 ms |
804 ms |
0.96 |
Update one schema (1 existing) |
107 ms |
111 ms |
0.96 |
Update one schema (101 existing) |
119 ms |
126 ms |
0.94 |
Update one schema (1001 existing) |
172 ms |
180 ms |
0.96 |
Update one schema (10001 existing) |
796 ms |
826 ms |
0.96 |
Cached rebuild (1 existing) |
14 ms |
14 ms |
1 |
Cached rebuild (101 existing) |
15 ms |
17 ms |
0.88 |
Cached rebuild (1001 existing) |
42 ms |
52 ms |
0.81 |
Cached rebuild (10001 existing) |
307 ms |
317 ms |
0.97 |
Index 100 schemas |
582 ms |
621 ms |
0.94 |
Index 1000 schemas |
1618 ms |
1733 ms |
0.93 |
Index 10000 schemas |
13784 ms |
13956 ms |
0.99 |
Index 10000 schemas (custom meta-schema) |
15530 ms |
15328 ms |
1.01 |
Index 10000 schemas ($ref fan-out) |
17387 ms |
17592 ms |
0.99 |
enterprise/e2e/auth: Schema Anonymous (p50) |
396 us |
399 us |
0.99 |
enterprise/e2e/auth: Schema Anonymous (p99) |
504 us |
508 us |
0.99 |
enterprise/e2e/auth: Schema API Key Identity (p50) |
400 us |
404 us |
0.99 |
enterprise/e2e/auth: Schema API Key Identity (p99) |
504 us |
506 us |
1.00 |
enterprise/e2e/auth: Schema API Key SHA256 (p50) |
407 us |
411 us |
0.99 |
enterprise/e2e/auth: Schema API Key SHA256 (p99) |
522 us |
518 us |
1.01 |
enterprise/e2e/auth: Schema JWT (p50) |
531 us |
537 us |
0.99 |
enterprise/e2e/auth: Schema JWT (p99) |
674 us |
697 us |
0.97 |
test/e2e/html: Schema Fetch (p50) |
402 us |
409 us |
0.98 |
test/e2e/html: Schema Fetch (p99) |
484 us |
502 us |
0.96 |
This comment was automatically generated by workflow using github-action-benchmark.
Signed-off-by: Juan Cruz Viotti jv@jviotti.com