Conversation
Signed-off-by: Juan Cruz Viotti <jv@jviotti.com>
There was a problem hiding this comment.
1 issue found across 12 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="src/actions/action_serve_static_v1.h">
<violation number="1" location="src/actions/action_serve_static_v1.h:39">
P2: `error_schema_` can be empty when the route doesn’t provide `errorSchema`, but it is still used to build the Link header in `json_error`. This produces an invalid `Link: <>` header and drops the error schema reference. Consider providing a default or guarding against empty values before using it.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| request, response, sourcemeta::one::STATUS_INTERNAL_SERVER_ERROR, | ||
| "missing-base-path", "The base path is not configured for this route", | ||
| std::string{this->base_path()} + "/self/v1/schemas/api/error"); | ||
| this->error_schema_); |
There was a problem hiding this comment.
P2: error_schema_ can be empty when the route doesn’t provide errorSchema, but it is still used to build the Link header in json_error. This produces an invalid Link: <> header and drops the error schema reference. Consider providing a default or guarding against empty values before using it.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/actions/action_serve_static_v1.h, line 39:
<comment>`error_schema_` can be empty when the route doesn’t provide `errorSchema`, but it is still used to build the Link header in `json_error`. This produces an invalid `Link: <>` header and drops the error schema reference. Consider providing a default or guarding against empty values before using it.</comment>
<file context>
@@ -34,17 +36,18 @@ class ActionServeStatic_v1 : public sourcemeta::one::Action {
request, response, sourcemeta::one::STATUS_INTERNAL_SERVER_ERROR,
"missing-base-path", "The base path is not configured for this route",
- std::string{this->base_path()} + "/self/v1/schemas/api/error");
+ this->error_schema_);
return;
}
</file context>
🤖 Augment PR SummarySummary: This PR moves most schema URLs (response and error Problem Details schemas) from being derived at runtime from Changes:
Technical Notes: This centralizes schema routing configuration in 🤖 Was this summary useful? React with 👍 or 👎 |
| request, response, sourcemeta::one::STATUS_METHOD_NOT_ALLOWED, | ||
| "method-not-allowed", "This HTTP method is invalid for this URL", | ||
| std::string{this->base_path()} + "/self/v1/schemas/api/error"); | ||
| this->error_schema_); |
There was a problem hiding this comment.
src/actions/action_health_check_v1.h:35: error_schema_ is never defaulted, so if errorSchema isn't present in the route arguments (for example, an older routes.bin), json_error will emit an invalid Link: <>; rel="describedby" header. Consider ensuring a non-empty default (like the prior base_path()+"/self/v1/schemas/api/error") or validating before calling json_error.
Severity: medium
Other Locations
src/actions/action_not_found_v1.h:33src/actions/action_schema_search_v1.h:45src/actions/action_serve_static_v1.h:39src/actions/action_jsonschema_serve_v1.h:85src/actions/action_serve_schema_artifact_v1.h:41
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
| response, std::string{this->base_path()} + | ||
| "/self/v1/schemas/api/schemas/evaluate/response"); | ||
| } | ||
| sourcemeta::one::write_link_header(response, this->response_schema_); |
There was a problem hiding this comment.
src/actions/action_jsonschema_evaluate_v1.h:334: write_link_header is now driven by response_schema_ from route arguments; if that argument is missing/empty, the response will include an invalid Link header and lose the previous mode-based schema selection. Consider ensuring responseSchema is always populated for both evaluate/trace routes (or falling back based on mode_).
Severity: medium
Other Locations
src/actions/action_schema_search_v1.h:143src/actions/action_serve_explorer_artifact_v1.h:44src/actions/action_serve_schema_artifact_v1.h:48
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
There was a problem hiding this comment.
Benchmark Index (community)
Details
| Benchmark suite | Current: 5d7d412 | Previous: 8434752 | Ratio |
|---|---|---|---|
Add one schema (0 existing) |
18 ms |
18 ms |
1 |
Add one schema (100 existing) |
23 ms |
25 ms |
0.92 |
Add one schema (1000 existing) |
73 ms |
80 ms |
0.91 |
Add one schema (10000 existing) |
884 ms |
726 ms |
1.22 |
Update one schema (1 existing) |
16 ms |
17 ms |
0.94 |
Update one schema (101 existing) |
33 ms |
25 ms |
1.32 |
Update one schema (1001 existing) |
74 ms |
81 ms |
0.91 |
Update one schema (10001 existing) |
885 ms |
715 ms |
1.24 |
Cached rebuild (1 existing) |
9 ms |
9 ms |
1 |
Cached rebuild (101 existing) |
14 ms |
12 ms |
1.17 |
Cached rebuild (1001 existing) |
30 ms |
34 ms |
0.88 |
Cached rebuild (10001 existing) |
258 ms |
283 ms |
0.91 |
Index 100 schemas |
111 ms |
126 ms |
0.88 |
Index 1000 schemas |
974 ms |
1057 ms |
0.92 |
Index 10000 schemas |
13102 ms |
13435 ms |
0.98 |
This comment was automatically generated by workflow using github-action-benchmark.
There was a problem hiding this comment.
Benchmark Index (enterprise)
Details
| Benchmark suite | Current: 5d7d412 | Previous: 9658355 | Ratio |
|---|---|---|---|
Add one schema (0 existing) |
19 ms |
20 ms |
0.95 |
Add one schema (100 existing) |
24 ms |
26 ms |
0.92 |
Add one schema (1000 existing) |
72 ms |
78 ms |
0.92 |
Add one schema (10000 existing) |
673 ms |
703 ms |
0.96 |
Update one schema (1 existing) |
18 ms |
18 ms |
1 |
Update one schema (101 existing) |
24 ms |
26 ms |
0.92 |
Update one schema (1001 existing) |
74 ms |
83 ms |
0.89 |
Update one schema (10001 existing) |
627 ms |
656 ms |
0.96 |
Cached rebuild (1 existing) |
10 ms |
10 ms |
1 |
Cached rebuild (101 existing) |
12 ms |
13 ms |
0.92 |
Cached rebuild (1001 existing) |
32 ms |
36 ms |
0.89 |
Cached rebuild (10001 existing) |
262 ms |
291 ms |
0.90 |
Index 100 schemas |
116 ms |
134 ms |
0.87 |
Index 1000 schemas |
974 ms |
1312 ms |
0.74 |
Index 10000 schemas |
14233 ms |
13178 ms |
1.08 |
This comment was automatically generated by workflow using github-action-benchmark.
Signed-off-by: Juan Cruz Viotti jv@jviotti.com