Repository navigation
Conversation
Signed-off-by: Juan Cruz Viotti <jv@jviotti.com>
| REPLACE $CWD WITH '[CWD]' IN result.txt | ||
|
|
||
| WRITE expected.txt UNTIL EOF | ||
| 2> error: The dialect that this schema declares does not define the keyword that declares it |
Signed-off-by: Juan Cruz Viotti <jv@jviotti.com>
|
This pull request is too large for Augment to review. The PR exceeds the maximum size limit of 75000 tokens (approximately 300000 characters) for automated code review. Please consider breaking this PR into smaller, more focused changes. |
Signed-off-by: Juan Cruz Viotti <jv@jviotti.com>
There was a problem hiding this comment.
4 issues found and verified against the latest diff
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="docs/lint.markdown">
<violation number="1" location="docs/lint.markdown:170">
P2: OpenAPI 2.0 documents use `swagger`, not `openapi`, so `lint` treats them as schemas rather than rejecting them. Limit this claim to unsupported `openapi` revisions, or update detection to reject Swagger 2.0 documents.</violation>
</file>
<file name="test/format/pass_openapi_3_0.clitest">
<violation number="1" location="test/format/pass_openapi_3_0.clitest:14">
P3: This only checks the quiet path, so the OpenAPI 3.0 fixture never verifies the formatter’s success diagnostics; add `--verbose` and assert the `Formatting:` line on stderr.
(Based on your team's feedback about verbose output assertions.)</violation>
</file>
<file name="src/utils.h">
<violation number="1" location="src/utils.h:369">
P2: This location can belong to a schema reached through the resolver, while `positions` tracks only the entry file; a matching pointer can therefore report the wrong source line. Apply positions only when the error is known to come from the entry document, and otherwise report file-level context.</violation>
</file>
<file name="src/input.h">
<violation number="1" location="src/input.h:283">
P1: This treats any trailing non-whitespace as another document without parsing it. For unknown-extension YAML inputs, `rdf` ignores `ParsedJSON.multidocument`, so malformed trailing YAML is silently discarded; validate the remainder before returning.</violation>
</file>
Note: This PR contains a large number of files. cubic selects up to 200 of the highest-priority eligible files for this review, so some files may not have been reviewed.
View guided diff | Re-trigger cubic
| throw sourcemeta::core::YAMLFileParseError{path, error}; | ||
| } | ||
|
|
||
| return {.multidocument = !at_end_of_stream(input, stream)}; |
There was a problem hiding this comment.
P1: This treats any trailing non-whitespace as another document without parsing it. For unknown-extension YAML inputs, rdf ignores ParsedJSON.multidocument, so malformed trailing YAML is silently discarded; validate the remainder before returning.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At src/input.h, line 283:
<comment>This treats any trailing non-whitespace as another document without parsing it. For unknown-extension YAML inputs, `rdf` ignores `ParsedJSON.multidocument`, so malformed trailing YAML is silently discarded; validate the remainder before returning.</comment>
<file context>
@@ -265,16 +265,24 @@ read_yaml_file(const std::filesystem::path &path,
+ throw sourcemeta::core::YAMLFileParseError{path, error};
+ }
+
+ return {.multidocument = !at_end_of_stream(input, stream)};
}
</file context>
| } | ||
| EOF | ||
|
|
||
| RUN fmt openapi.json STDIN /dev/null IN . INTO result.txt EXPECTING 0 |
There was a problem hiding this comment.
P3: This only checks the quiet path, so the OpenAPI 3.0 fixture never verifies the formatter’s success diagnostics; add --verbose and assert the Formatting: line on stderr.
(Based on your team's feedback about verbose output assertions.)
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At test/format/pass_openapi_3_0.clitest, line 14:
<comment>This only checks the quiet path, so the OpenAPI 3.0 fixture never verifies the formatter’s success diagnostics; add `--verbose` and assert the `Formatting:` line on stderr.
(Based on your team's feedback about verbose output assertions.) </comment>
<file context>
@@ -0,0 +1,41 @@
+}
+EOF
+
+RUN fmt openapi.json STDIN /dev/null IN . INTO result.txt EXPECTING 0
+
+WRITE expected.txt UNTIL EOF
</file context>
There was a problem hiding this comment.
2 issues found across 443 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/command_bundle.cc">
<violation number="1" location="src/command_bundle.cc:192">
P2: This can report an external Schema Object's dialect error at an unrelated line in the entry document when its local pointer matches one there. Preserve the error's document provenance before mapping it to source positions, or avoid adding an entry-document position for errors from resolved schemas.</violation>
</file>
<file name="src/utils.h">
<violation number="1" location="src/utils.h:369">
P2: These handlers map the error's pointer through the entry document's tracker even when bundling can raise it while traversing a fetched reference; an identical pointer can send users to an unrelated line. Preserve the originating document before producing a position, or omit line mapping when the error does not identify its source.</violation>
</file>
Note: This PR contains a large number of files. cubic selects up to 200 of the highest-priority eligible files for this review, so some files may not have been reviewed.
View guided diff | Re-trigger cubic
| sourcemeta::core::SchemaAnchorCollisionError>(schema_display_path, | ||
| error); | ||
| } catch (const sourcemeta::core::SchemaDialectImpossibleError &error) { | ||
| const auto position{parsed_schema.positions.get(error.location())}; |
There was a problem hiding this comment.
P2: This can report an external Schema Object's dialect error at an unrelated line in the entry document when its local pointer matches one there. Preserve the error's document provenance before mapping it to source positions, or avoid adding an entry-document position for errors from resolved schemas.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At src/command_bundle.cc, line 192:
<comment>This can report an external Schema Object's dialect error at an unrelated line in the entry document when its local pointer matches one there. Preserve the error's document provenance before mapping it to source positions, or avoid adding an entry-document position for errors from resolved schemas.</comment>
<file context>
@@ -188,6 +188,18 @@ auto sourcemeta::jsonschema::bundle(const sourcemeta::core::Options &options)
sourcemeta::core::SchemaAnchorCollisionError>(schema_display_path,
error);
+ } catch (const sourcemeta::core::SchemaDialectImpossibleError &error) {
+ const auto position{parsed_schema.positions.get(error.location())};
+ if (position.has_value()) {
+ throw PositionError<sourcemeta::core::FileError<
</file context>
| throw sourcemeta::core::FileError< | ||
| sourcemeta::core::SchemaAnchorCollisionError>(display_path, error); | ||
| } catch (const sourcemeta::core::SchemaDialectImpossibleError &error) { | ||
| const auto position{positions.get(error.location())}; |
There was a problem hiding this comment.
P2: These handlers map the error's pointer through the entry document's tracker even when bundling can raise it while traversing a fetched reference; an identical pointer can send users to an unrelated line. Preserve the originating document before producing a position, or omit line mapping when the error does not identify its source.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At src/utils.h, line 369:
<comment>These handlers map the error's pointer through the entry document's tracker even when bundling can raise it while traversing a fetched reference; an identical pointer can send users to an unrelated line. Preserve the originating document before producing a position, or omit line mapping when the error does not identify its source.</comment>
<file context>
@@ -365,6 +365,17 @@ auto with_openapi_diagnostics(
throw sourcemeta::core::FileError<
sourcemeta::core::SchemaAnchorCollisionError>(display_path, error);
+ } catch (const sourcemeta::core::SchemaDialectImpossibleError &error) {
+ const auto position{positions.get(error.location())};
+ if (position.has_value()) {
+ throw PositionError<sourcemeta::core::FileError<
</file context>
There was a problem hiding this comment.
All reported issues were addressed across 21 files (changes from recent commits).
Reply with feedback, questions, or to request a fix.
View guided diff | Re-trigger cubic
Signed-off-by: Juan Cruz Viotti jv@jviotti.com