Fix C++ review findings in pipeline: k-prefixed enum values, include ordering, populated validation error messages, conventional const notation, and endl replaced with newline

This commit is contained in:
Aaron Po
2026-08-23 15:45:35 -04:00
parent 754bd6764e
commit 9bec9b1edd
6 changed files with 34 additions and 24 deletions

View File

@@ -191,9 +191,9 @@ struct SamplingOptions {
* @brief Selects which DataGenerator backend the pipeline uses. * @brief Selects which DataGenerator backend the pipeline uses.
*/ */
enum class GeneratorMode { enum class GeneratorMode {
Mock, kMock,
Llama, kLlama,
OpenAI, kOpenAI,
}; };
/** /**
@@ -202,25 +202,25 @@ enum class GeneratorMode {
struct GeneratorOptions { struct GeneratorOptions {
/** /**
* @brief Path to the LLM model file (gguf format). Only meaningful for * @brief Path to the LLM model file (gguf format). Only meaningful for
* GeneratorMode::Llama. * GeneratorMode::kLlama.
*/ */
std::filesystem::path model_path; std::filesystem::path model_path;
/** /**
* @brief Which generator backend to use. * @brief Which generator backend to use.
*/ */
GeneratorMode mode = GeneratorMode::Mock; GeneratorMode mode = GeneratorMode::kMock;
/** /**
* @brief Specific sampling parameters for this generator. Only * @brief Specific sampling parameters for this generator. Only
* meaningful for GeneratorMode::Llama; if nullopt, LlamaGenerator uses * meaningful for GeneratorMode::kLlama; if nullopt, LlamaGenerator uses
* its own defaults. * its own defaults.
*/ */
std::optional<SamplingOptions> sampling; std::optional<SamplingOptions> sampling;
/** /**
* @brief OpenAI API key, read from the OPENAI_API_KEY environment * @brief OpenAI API key, read from the OPENAI_API_KEY environment
* variable. Only populated for GeneratorMode::OpenAI. * variable. Only populated for GeneratorMode::kOpenAI.
*/ */
std::string openai_api_key; std::string openai_api_key;

View File

@@ -182,16 +182,16 @@ std::optional<ApplicationOptions> ParseArguments(
.phase = PipelinePhase::Startup, .phase = PipelinePhase::Startup,
.message = msg}); .message = msg});
} else { } else {
std::cerr << msg << std::endl; std::cerr << msg << '\n';
} }
return std::nullopt; return std::nullopt;
} }
options.generator.openai_api_key = api_key_env; options.generator.openai_api_key = api_key_env;
} }
options.generator.mode = use_mocked ? GeneratorMode::Mock options.generator.mode = use_mocked ? GeneratorMode::kMock
: use_openai ? GeneratorMode::OpenAI : use_openai ? GeneratorMode::kOpenAI
: GeneratorMode::Llama; : GeneratorMode::kLlama;
options.generator.model_path = model_path; options.generator.model_path = model_path;
options.generator.openai_model = var_map["openai-model"].as<std::string>(); options.generator.openai_model = var_map["openai-model"].as<std::string>();
@@ -208,7 +208,7 @@ std::optional<ApplicationOptions> ParseArguments(
if (user_provided_sampling) { if (user_provided_sampling) {
// Warn but do not fail — the run is still valid, the flags are just // Warn but do not fail — the run is still valid, the flags are just
// silently irrelevant when no local model is loaded. // silently irrelevant when no local model is loaded.
if (options.generator.mode != GeneratorMode::Llama) { if (options.generator.mode != GeneratorMode::kLlama) {
const std::string msg = const std::string msg =
"Sampling parameters are ignored unless using --model " "Sampling parameters are ignored unless using --model "
"(Llama)"; "(Llama)";

View File

@@ -31,8 +31,8 @@ double RadiansToDegrees(double radians) {
} }
} // namespace } // namespace
static Coords GetRandomCoordsWithinRange(Coords const& centre, static Coords GetRandomCoordsWithinRange(const Coords& centre,
int const distance_km, const int distance_km,
std::mt19937& rng) { std::mt19937& rng) {
std::uniform_real_distribution<double> angle_dist(0.0, std::uniform_real_distribution<double> angle_dist(0.0,
2.0 * std::numbers::pi); 2.0 * std::numbers::pi);

View File

@@ -8,12 +8,14 @@
#include <algorithm> #include <algorithm>
#include <array> #include <array>
#include <boost/json.hpp>
#include <cctype> #include <cctype>
#include <format>
#include <span> #include <span>
#include <string> #include <string>
#include <string_view> #include <string_view>
#include <boost/json.hpp>
namespace { namespace {
/** /**
* String trimming: removes leading and trailing whitespace * String trimming: removes leading and trailing whitespace
@@ -66,11 +68,18 @@ bool ReadRequiredTrimmedStringField(const boost::json::object& obj,
std::string* error_out) { std::string* error_out) {
const boost::json::value* field = obj.if_contains(key); const boost::json::value* field = obj.if_contains(key);
if (field == nullptr || !field->is_string()) { if (field == nullptr || !field->is_string()) {
if (error_out != nullptr) {
*error_out =
std::format("Missing or invalid string field: {}", key);
}
return false; return false;
} }
const auto& string_value = field->as_string(); const auto& string_value = field->as_string();
out = Trim(std::string_view(string_value.data(), string_value.size())); out = Trim(std::string_view(string_value.data(), string_value.size()));
if (out.empty() && error_out != nullptr) {
*error_out = std::format("Field must not be empty: {}", key);
}
return !out.empty(); return !out.empty();
} }

View File

@@ -9,7 +9,6 @@
* Structured Outputs: https://platform.openai.com/docs/guides/structured-outputs * Structured Outputs: https://platform.openai.com/docs/guides/structured-outputs
*/ */
#include <boost/json.hpp>
#include <format> #include <format>
#include <stdexcept> #include <stdexcept>
#include <string> #include <string>
@@ -17,6 +16,8 @@
#include <utility> #include <utility>
#include <vector> #include <vector>
#include <boost/json.hpp>
#include "data_generation/openai_generator.h" #include "data_generation/openai_generator.h"
namespace { namespace {

View File

@@ -67,7 +67,7 @@ int main(const int argc, char** argv) {
std::unique_ptr<IPromptDirectory> prompt_directory; std::unique_ptr<IPromptDirectory> prompt_directory;
if (options.generator.mode != GeneratorMode::Mock) { if (options.generator.mode != GeneratorMode::kMock) {
try { try {
prompt_directory = std::make_unique<PromptDirectory>( prompt_directory = std::make_unique<PromptDirectory>(
options.pipeline.prompt_dir, log_producer); options.pipeline.prompt_dir, log_producer);
@@ -90,7 +90,7 @@ int main(const int argc, char** argv) {
di::bind<IPostalCodeService>().to( di::bind<IPostalCodeService>().to(
[options, [options,
&log_producer]() -> std::unique_ptr<IPostalCodeService> { &log_producer]() -> std::unique_ptr<IPostalCodeService> {
if (options.generator.mode == GeneratorMode::Mock) { if (options.generator.mode == GeneratorMode::kMock) {
log_producer->Log( log_producer->Log(
{.level = LogLevel::Info, {.level = LogLevel::Info,
.phase = PipelinePhase::Startup, .phase = PipelinePhase::Startup,
@@ -110,7 +110,7 @@ int main(const int argc, char** argv) {
di::bind<ICuratedDataService>().to( di::bind<ICuratedDataService>().to(
[options, [options,
&log_producer]() -> std::unique_ptr<ICuratedDataService> { &log_producer]() -> std::unique_ptr<ICuratedDataService> {
// if (options.generator.mode == GeneratorMode::Mock) { // if (options.generator.mode == GeneratorMode::kMock) {
// log_producer->Log({.level = LogLevel::Info, // log_producer->Log({.level = LogLevel::Info,
// .phase = PipelinePhase::Startup, // .phase = PipelinePhase::Startup,
// .message = "Curated data: mock"}); // .message = "Curated data: mock"});
@@ -131,7 +131,7 @@ int main(const int argc, char** argv) {
}); });
}), }),
di::bind<IPromptFormatter>().to([options, log_producer] { di::bind<IPromptFormatter>().to([options, log_producer] {
if (options.generator.mode != GeneratorMode::Llama) { if (options.generator.mode != GeneratorMode::kLlama) {
log_producer->Log( log_producer->Log(
{.level = LogLevel::Info, {.level = LogLevel::Info,
.phase = PipelinePhase::Startup, .phase = PipelinePhase::Startup,
@@ -150,7 +150,7 @@ int main(const int argc, char** argv) {
std::make_unique<Gemma4JinjaPromptFormatter>()); std::make_unique<Gemma4JinjaPromptFormatter>());
}), }),
di::bind<WebClient>().to([options, log_producer] { di::bind<WebClient>().to([options, log_producer] {
if (options.generator.mode == GeneratorMode::Mock) { if (options.generator.mode == GeneratorMode::kMock) {
{ {
log_producer->Log( log_producer->Log(
{.level = LogLevel::Info, {.level = LogLevel::Info,
@@ -170,7 +170,7 @@ int main(const int argc, char** argv) {
di::bind<IEnrichmentService>().to( di::bind<IEnrichmentService>().to(
[options, &log_producer]( [options, &log_producer](
const auto& inj) -> std::unique_ptr<IEnrichmentService> { const auto& inj) -> std::unique_ptr<IEnrichmentService> {
if (options.generator.mode == GeneratorMode::Mock) { if (options.generator.mode == GeneratorMode::kMock) {
log_producer->Log({.level = LogLevel::Info, log_producer->Log({.level = LogLevel::Info,
.phase = PipelinePhase::Startup, .phase = PipelinePhase::Startup,
.message = "Enrichment: mock"}); .message = "Enrichment: mock"});
@@ -190,7 +190,7 @@ int main(const int argc, char** argv) {
[&options, &model_path, &sampling, &prompt_directory, [&options, &model_path, &sampling, &prompt_directory,
&log_producer]( &log_producer](
const auto& inj) -> std::unique_ptr<DataGenerator> { const auto& inj) -> std::unique_ptr<DataGenerator> {
if (options.generator.mode == GeneratorMode::Mock) { if (options.generator.mode == GeneratorMode::kMock) {
log_producer->Log({.level = LogLevel::Info, log_producer->Log({.level = LogLevel::Info,
.phase = PipelinePhase::Startup, .phase = PipelinePhase::Startup,
.message = "Generator: mock"}); .message = "Generator: mock"});
@@ -198,7 +198,7 @@ int main(const int argc, char** argv) {
return std::make_unique<MockGenerator>(); return std::make_unique<MockGenerator>();
} }
if (options.generator.mode == GeneratorMode::OpenAI) { if (options.generator.mode == GeneratorMode::kOpenAI) {
log_producer->Log( log_producer->Log(
{.level = LogLevel::Info, {.level = LogLevel::Info,
.phase = PipelinePhase::Startup, .phase = PipelinePhase::Startup,