Skip to content

Turn SimpleOutput annotations into a vector #916

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
jviotti merged 2 commits into from
Jul 8, 2026
Merged

Turn SimpleOutput annotations into a vector #916

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2d57bb8
Turn `SimpleOutput` annotations into a vector
jviotti Jul 8, 2026
3c17eb9
Fix
jviotti Jul 8, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 17 additions & 24 deletions src/output/include/sourcemeta/blaze/output_simple.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
#include <map> // std::map
#include <ostream> // std::ostream
#include <string> // std::string
#include <tuple> // std::tie
#include <utility> // std::pair
#include <vector> // std::vector

Expand Down Expand Up @@ -83,6 +82,14 @@ class SOURCEMETA_BLAZE_OUTPUT_EXPORT SimpleOutput {
std::reference_wrapper<const std::string> schema_location;
};

/// A single collected annotation, in evaluation order
struct AnnotationEntry {
sourcemeta::core::WeakPointer instance_location;
sourcemeta::core::WeakPointer evaluate_path;
std::reference_wrapper<const std::string> schema_location;
sourcemeta::core::JSON value;
};

auto operator()(const EvaluationType type, const bool result,
const Instruction &step,
const InstructionExtra &step_metadata,
Expand All @@ -97,12 +104,6 @@ class SOURCEMETA_BLAZE_OUTPUT_EXPORT SimpleOutput {
[[nodiscard]] auto cbegin() const -> const_iterator;
[[nodiscard]] auto cend() const -> const_iterator;

/// Access annotations that were collected during evaluation, indexed by
/// instance location and evaluation path
[[nodiscard]] auto annotations() const -> const auto & {
return this->annotations_;
}

/// Move out the collected error entries, leaving this output empty. Useful to
/// take ownership of the trace without copying when the output is no longer
/// needed
Expand All @@ -114,22 +115,14 @@ class SOURCEMETA_BLAZE_OUTPUT_EXPORT SimpleOutput {
return result;
}

// NOLINTNEXTLINE(bugprone-exception-escape)
struct Location {
auto operator<(const Location &other) const noexcept -> bool {
// Perform a lexicographical comparison
return std::tie(this->instance_location, this->evaluate_path,
this->schema_location.get()) <
std::tie(other.instance_location, other.evaluate_path,
other.schema_location.get());
}

// NOLINTBEGIN(cppcoreguidelines-avoid-const-or-ref-data-members)
const sourcemeta::core::WeakPointer instance_location;
const sourcemeta::core::WeakPointer evaluate_path;
const std::reference_wrapper<const std::string> schema_location;
// NOLINTEND(cppcoreguidelines-avoid-const-or-ref-data-members)
};
/// Access the annotations collected during evaluation, as a flat log in
/// evaluation order. The log records every emission as-is, so a location may
/// repeat and hold the same value more than once. Consumers that need them
/// grouped by location, or collapsed to distinct values, index this log
/// themselves
[[nodiscard]] auto annotations() const -> const auto & {
return this->annotations_;
}

private:
// Exporting symbols that depends on the standard C++ library is considered
Expand All @@ -148,7 +141,7 @@ class SOURCEMETA_BLAZE_OUTPUT_EXPORT SimpleOutput {
std::pair<sourcemeta::core::WeakPointer, sourcemeta::core::WeakPointer>,
std::vector<Entry>>
masked_traces;
std::map<Location, std::vector<sourcemeta::core::JSON>> annotations_;
std::vector<AnnotationEntry> annotations_;
#if defined(_MSC_VER)
#pragma warning(default : 4251)
#endif
Expand Down
34 changes: 32 additions & 2 deletions src/output/output_jsonld.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,8 @@

#include <algorithm> // std::ranges::sort, std::ranges::any_of
#include <deque> // std::deque
#include <functional> // std::ref
#include <functional> // std::ref, std::reference_wrapper
#include <map> // std::map
#include <optional> // std::optional
#include <sstream> // std::ostringstream
#include <string> // std::string
Expand Down Expand Up @@ -400,6 +401,35 @@ auto array_of_nodes(const Accumulator &accumulator,
return true;
}

// NOLINTNEXTLINE(bugprone-exception-escape)
struct AnnotationLocation {
auto operator<(const AnnotationLocation &other) const noexcept -> bool {
return std::tie(this->instance_location, this->evaluate_path,
this->schema_location.get()) <
std::tie(other.instance_location, other.evaluate_path,
other.schema_location.get());
}

sourcemeta::core::WeakPointer instance_location;
sourcemeta::core::WeakPointer evaluate_path;
std::reference_wrapper<const std::string> schema_location;
};

auto group_annotations(const sourcemeta::blaze::SimpleOutput &output)
-> std::map<AnnotationLocation, std::vector<sourcemeta::core::JSON>> {
std::map<AnnotationLocation, std::vector<sourcemeta::core::JSON>> result;
for (const auto &entry : output.annotations()) {
auto &values{result[{.instance_location = entry.instance_location,
.evaluate_path = entry.evaluate_path,
.schema_location = entry.schema_location}]};
if (values.empty() || values.back() != entry.value) {
values.push_back(entry.value);
}
}

return result;
}

// Turn the JSON-LD annotations into a resolved map, or the first error found.
// The first pass groups the facts by location, the second derives each kind
// from the value shape and validates the facts against it
Expand All @@ -409,7 +439,7 @@ auto resolve(const sourcemeta::core::JSON &instance,
sourcemeta::blaze::JSONLDResolutionError> {
Accumulator accumulator;

for (const auto &[location, values] : output.annotations()) {
for (const auto &[location, values] : group_annotations(output)) {
if (location.evaluate_path.empty()) {
continue;
}
Expand Down
31 changes: 10 additions & 21 deletions src/output/output_simple.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,18 +66,11 @@ auto SimpleOutput::operator()(

if (is_annotation(step.type)) {
if (type == EvaluationType::Post) {
Location location{.instance_location = instance_location,
.evaluate_path = std::move(effective_evaluate_path),
.schema_location = step_metadata.keyword_location};
const auto match{this->annotations_.find(location)};
if (match == this->annotations_.cend()) {
this->annotations_[std::move(location)].push_back(annotation);

// To avoid emitting the exact same annotation more than once
// This is right now mostly because of `unevaluatedItems`
} else if (match->second.back() != annotation) {
match->second.push_back(annotation);
}
this->annotations_.push_back(
Comment thread
jviotti marked this conversation as resolved.
{.instance_location = instance_location,
.evaluate_path = std::move(effective_evaluate_path),
.schema_location = step_metadata.keyword_location,
.value = annotation});
}

return;
Expand Down Expand Up @@ -122,15 +115,11 @@ auto SimpleOutput::operator()(
}

if (type == EvaluationType::Post && !this->annotations_.empty()) {
for (auto iterator = this->annotations_.begin();
iterator != this->annotations_.end();) {
if (iterator->first.evaluate_path.starts_with_initial(evaluate_path) &&
iterator->first.instance_location == instance_location) {
iterator = this->annotations_.erase(iterator);
} else {
++iterator;
}
}
std::erase_if(
this->annotations_, [&](const AnnotationEntry &entry) -> bool {
return entry.evaluate_path.starts_with_initial(evaluate_path) &&
entry.instance_location == instance_location;
});
}

if (keyword == "if") {
Expand Down
38 changes: 36 additions & 2 deletions src/output/output_standard.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,15 +1,49 @@
#include <sourcemeta/blaze/output_simple.h>
#include <sourcemeta/blaze/output_standard.h>

#include <sourcemeta/core/json.h>
#include <sourcemeta/core/jsonpointer.h>

#include <cassert> // assert
#include <functional> // std::ref
#include <functional> // std::ref, std::reference_wrapper
#include <map> // std::map
#include <string> // std::string
#include <tuple> // std::tie
#include <vector> // std::vector

namespace sourcemeta::blaze {

namespace {

// NOLINTNEXTLINE(bugprone-exception-escape)
struct AnnotationLocation {

@cubic-dev-ai cubic-dev-ai Bot Jul 8, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3: Annotation grouping now has two copied implementations in output_standard.cc and output_jsonld.cc, so future grouping changes can drift between Standard and JSON-LD outputs. Consider sharing one internal helper instead of maintaining duplicate struct/map logic.

Prompt for AI agents 
Check if this issue is valid — if so, understand the root cause and fix it. At src/output/output_standard.cc, line 19:

<comment>Annotation grouping now has two copied implementations in `output_standard.cc` and `output_jsonld.cc`, so future grouping changes can drift between Standard and JSON-LD outputs. Consider sharing one internal helper instead of maintaining duplicate struct/map logic.</comment>

<file context>
@@ -1,15 +1,49 @@
 namespace {
 
+// NOLINTNEXTLINE(bugprone-exception-escape)
+struct AnnotationLocation {
+  auto operator<(const AnnotationLocation &other) const noexcept -> bool {
+    return std::tie(this->instance_location, this->evaluate_path,
</file context>

auto operator<(const AnnotationLocation &other) const noexcept -> bool {
return std::tie(this->instance_location, this->evaluate_path,
this->schema_location.get()) <
std::tie(other.instance_location, other.evaluate_path,
other.schema_location.get());
}

sourcemeta::core::WeakPointer instance_location;
sourcemeta::core::WeakPointer evaluate_path;
std::reference_wrapper<const std::string> schema_location;
};

auto group_annotations(const SimpleOutput &output)
-> std::map<AnnotationLocation, std::vector<sourcemeta::core::JSON>> {
std::map<AnnotationLocation, std::vector<sourcemeta::core::JSON>> result;
for (const auto &entry : output.annotations()) {
auto &values{result[{.instance_location = entry.instance_location,
.evaluate_path = entry.evaluate_path,
.schema_location = entry.schema_location}]};
if (values.empty() || values.back() != entry.value) {
values.push_back(entry.value);
}
Comment on lines +39 to +41

@cubic-dev-ai cubic-dev-ai Bot Jul 8, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: Basic output can lose repeated annotations from distinct successful evaluations when they have the same location and value. Since annotation is now serialized from a vector, preserving every SimpleOutput::annotations() entry would avoid silently changing annotation multiplicity.

Prompt for AI agents 
Check if this issue is valid — if so, understand the root cause and fix it. At src/output/output_standard.cc, line 39:

<comment>Basic output can lose repeated annotations from distinct successful evaluations when they have the same location and value. Since `annotation` is now serialized from a vector, preserving every `SimpleOutput::annotations()` entry would avoid silently changing annotation multiplicity.</comment>

<file context>
@@ -1,15 +1,49 @@
+    auto &values{result[{.instance_location = entry.instance_location,
+                         .evaluate_path = entry.evaluate_path,
+                         .schema_location = entry.schema_location}]};
+    if (values.empty() || values.back() != entry.value) {
+      values.push_back(entry.value);
+    }
</file context>
Suggested change
if (values.empty() || values.back() != entry.value) {
values.push_back(entry.value);
}
values.push_back(entry.value);

}

return result;
}

auto handle_standard(Evaluator &evaluator, const Template &schema,
const sourcemeta::core::JSON &instance,
const StandardOutput format,
Expand All @@ -30,7 +64,7 @@ auto handle_standard(Evaluator &evaluator, const Template &schema,
auto result{sourcemeta::core::JSON::make_object()};
result.assign_assume_new("valid", sourcemeta::core::JSON{valid});
auto annotations{sourcemeta::core::JSON::make_array()};
for (const auto &annotation : output.annotations()) {
for (const auto &annotation : group_annotations(output)) {
auto unit{sourcemeta::core::JSON::make_object()};
unit.assign_assume_new(
"keywordLocation",
Expand Down
28 changes: 13 additions & 15 deletions test/evaluator/annotationsuite.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
#include <sourcemeta/core/json.h>
#include <sourcemeta/core/jsonpointer.h>

#include <algorithm>
#include <cassert>
#include <filesystem>
#include <functional>
Expand Down Expand Up @@ -61,16 +60,15 @@ has_annotation(const sourcemeta::blaze::SimpleOutput &output,

for (const auto &annotation : output.annotations()) {
std::cerr << "Checking against annotations for instance location \""
<< sourcemeta::core::to_string(annotation.first.instance_location)
<< "\" and schema location \""
<< annotation.first.schema_location.get() << "\"" << "\n";
<< sourcemeta::core::to_string(annotation.instance_location)
<< "\" and schema location \"" << annotation.schema_location.get()
<< "\"" << "\n";

if (annotation.first.instance_location != instance_location ||
!schema_location_matches(annotation.first.schema_location.get(),
if (annotation.instance_location != instance_location ||
!schema_location_matches(annotation.schema_location.get(),
schema_location, frame)) {
continue;
} else if (std::find(annotation.second.cbegin(), annotation.second.cend(),
value) != annotation.second.cend()) {
} else if (annotation.value == value) {
return true;
}
}
Expand All @@ -88,15 +86,15 @@ has_matching_annotations(const sourcemeta::blaze::SimpleOutput &output,

for (const auto &annotation : output.annotations()) {
std::cerr << "Checking against annotations for instance location \""
<< sourcemeta::core::to_string(annotation.first.instance_location)
<< sourcemeta::core::to_string(annotation.instance_location)
<< "\" and evaluate path \""
<< sourcemeta::core::to_string(annotation.first.evaluate_path)
<< "\"" << "\n";
<< sourcemeta::core::to_string(annotation.evaluate_path) << "\""
<< "\n";

if (annotation.first.instance_location == instance_location &&
!annotation.first.evaluate_path.empty() &&
annotation.first.evaluate_path.back().is_property() &&
annotation.first.evaluate_path.back().to_property() == keyword) {
if (annotation.instance_location == instance_location &&
!annotation.evaluate_path.empty() &&
annotation.evaluate_path.back().is_property() &&
annotation.evaluate_path.back().to_property() == keyword) {
return true;
}
}
Expand Down
Loading
Loading