fix: resolve critical and minor issues from C++ subsystem validation by Copilot · Pull Request #54 · dfeen87/CuraFrame

Copilot · 2026-04-10T13:54:39Z

The C++ constraint-bundle subsystem (introduced in v2.5.0–3.0.0) had no build system, a fragile self-registration pattern, several interface correctness gaps, and minor documentation/metadata inconsistencies identified in a full-scope validation pass.

Build system (Critical)

Added CMakeLists.txt targeting C++17; compiles all 11 bundle TUs and 3 scoring TUs into libcuraframe_cpp.a with -Wall -Wextra -Wpedantic (zero warnings)
Documents the required --whole-archive linker flag for the self-registration pattern to work correctly with static libraries on GCC/Clang/MSVC

Self-registration macro placement (Minor #13 / partially Major #2)

REGISTER_CONSTRAINT_BUNDLE was invoked inside each .hpp, meaning any TU that included a bundle header would silently double-register it. Moved the macro call and the ConstraintRegistry.hpp include to each bundle's .cpp:

// AntiInfectiveBundle.cpp — before
#include "AntiInfectiveBundle.hpp"  // macro fired here too

// AntiInfectiveBundle.cpp — after
#include "AntiInfectiveBundle.hpp"
#include "../../constraint_core/ConstraintRegistry.hpp"
REGISTER_CONSTRAINT_BUNDLE("AntiInfective", AntiInfectiveBundle)

Interface and type correctness

MultiBundleEvaluator::evaluate() is now const (had no member side-effects)
MultiBundleEvaluator.hpp now directly includes ScoringReport.hpp instead of relying on transitive resolution through ScoringPipeline.hpp

Brittle profile-name string comparison eliminated

Added virtual std::string narrative_annotation() const to WeightProfile (default ""), overridden in HighSafetyProfile. generate_narrative() now calls this instead of comparing weight_profile_name == "HighSafetyProfile".

Narrative completeness

generate_narrative() now appends eval_report.combined_narrative under a "Domain Evaluations:" heading, so ScoringReport::narrative_summary is fully self-contained.

Documentation and metadata

ScoringReport.hpp: documented "domain_name::signal_name" key format on penalty_breakdown/bonus_breakdown
WeightProfile.hpp: clarified global_scaling_factor() override intent
WeightProfile.cpp: replaced misleading "intentionally blank" comment with a clear rationale
docs/constraint_bundles.md: "Global Safety" → "Safety" to match the registered domain key
docs/CHANGELOG.md: added ISO 8601 release dates to both entries
docs/evaluation_pipeline.md: fixed double-escaped \\n in the C++ code example
pyproject.toml: added PEP 621 [project] section so version is no longer only in setup.py

…report CRITICAL: - Add CMakeLists.txt with full C++ build configuration, compiler warning flags, and documented whole-archive linker guidance for the self-registration pattern MINOR #13 / partially resolves MAJOR #2: - Move REGISTER_CONSTRAINT_BUNDLE macro from every bundle .hpp to its .cpp, eliminating double-registration risk if a bundle header is included elsewhere - Remove ConstraintRegistry.hpp include from all bundle .hpp files (only needed in .cpp for the registration macro) MINOR #9: Add direct #include of ScoringReport.hpp to MultiBundleEvaluator.hpp MINOR #10: Make MultiBundleEvaluator::evaluate() const MINOR #7: Add virtual narrative_annotation() to WeightProfile; override in HighSafetyProfile; use it in generate_narrative() instead of brittle string comparison on the profile name MINOR #8: Propagate EvaluationReport::combined_narrative into the ScoringReport narrative_summary via generate_narrative() MINOR #11: Expand global_scaling_factor() doc comment to explain override intent MINOR #12: Replace misleading "intentionally blank" comment in WeightProfile.cpp with a meaningful explanation of the header-only implementation rationale MINOR #17: Document "domain::signal" key format on ScoringReport penalty/bonus breakdown fields MINOR #18: Fix "Global Safety" -> "Safety" in docs/constraint_bundles.md to match the registered domain key MINOR #14: Add [project] section with version 3.0.0 to pyproject.toml (PEP 621) MINOR #15: Add ISO 8601 release dates to both CHANGELOG.md entries MINOR #16: Fix double-escaped \\n -> \n in evaluation_pipeline.md code example Also suppress -Wunused-parameter warnings on pre-existing signal_weight overrides that became visible once the build system was added. Agent-Logs-Url: https://github.com/dfeen87/CuraFrame/sessions/dec508dc-52d2-4f94-9e2a-3a0ffac17658 Co-authored-by: dfeen87 <158860247+dfeen87@users.noreply.github.com>

chatgpt-codex-connector · 2026-04-10T13:54:53Z

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

Copilot

Pull request overview

This PR hardens the C++ constraint-bundle/scoring subsystem by adding a CMake build, fixing unsafe bundle self-registration, tightening interface correctness, and improving scoring narrative generation and project metadata/docs consistency.

Changes:

Add a CMake (C++17) static-library build for the C++ constraint-bundle + scoring subsystem and document whole-archive linking requirements for self-registration.
Move REGISTER_CONSTRAINT_BUNDLE invocations from headers into per-bundle .cpp files to prevent duplicate registrations.
Improve scoring/type interfaces and narrative generation (profile-specific annotation hook; include combined domain narratives), plus small docs/metadata updates.

Reviewed changes

Copilot reviewed 34 out of 34 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
`CMakeLists.txt`	Adds a CMake build producing `curaframe_cpp` static library and documents whole-archive linking for static initializers.
`constraint_core/MultiBundleEvaluator.hpp`	Makes `evaluate()` `const` and adds a direct include for `ScoringReport.hpp`.
`constraints/anti_infective/AntiInfectiveBundle.hpp`	Removes registry include + header-level self-registration macro.
`constraints/anti_infective/AntiInfectiveBundle.cpp`	Adds registry include + TU-level `REGISTER_CONSTRAINT_BUNDLE`.
`constraints/cardiac/CardiacBundle.hpp`	Removes registry include + header-level self-registration macro.
`constraints/cardiac/CardiacBundle.cpp`	Adds registry include + TU-level `REGISTER_CONSTRAINT_BUNDLE`.
`constraints/cns/CNSBundle.hpp`	Removes registry include + header-level self-registration macro.
`constraints/cns/CNSBundle.cpp`	Adds registry include + TU-level `REGISTER_CONSTRAINT_BUNDLE`.
`constraints/formulation/FormulationBundle.hpp`	Removes registry include + header-level self-registration macro.
`constraints/formulation/FormulationBundle.cpp`	Adds registry include + TU-level `REGISTER_CONSTRAINT_BUNDLE`.
`constraints/hepatic/HepaticBundle.hpp`	Removes registry include + header-level self-registration macro.
`constraints/hepatic/HepaticBundle.cpp`	Adds registry include + TU-level `REGISTER_CONSTRAINT_BUNDLE`.
`constraints/immunologic/ImmunologicBundle.hpp`	Removes registry include + header-level self-registration macro.
`constraints/immunologic/ImmunologicBundle.cpp`	Adds registry include + TU-level `REGISTER_CONSTRAINT_BUNDLE`.
`constraints/metabolic/MetabolicBundle.hpp`	Removes registry include + header-level self-registration macro.
`constraints/metabolic/MetabolicBundle.cpp`	Adds registry include + TU-level `REGISTER_CONSTRAINT_BUNDLE`.
`constraints/oncology/OncologyBundle.hpp`	Removes registry include + header-level self-registration macro.
`constraints/oncology/OncologyBundle.cpp`	Adds registry include + TU-level `REGISTER_CONSTRAINT_BUNDLE`.
`constraints/pkpd/PKPDBundle.hpp`	Removes registry include + header-level self-registration macro.
`constraints/pkpd/PKPDBundle.cpp`	Adds registry include + TU-level `REGISTER_CONSTRAINT_BUNDLE`.
`constraints/renal/RenalBundle.hpp`	Removes registry include + header-level self-registration macro.
`constraints/renal/RenalBundle.cpp`	Adds registry include + TU-level `REGISTER_CONSTRAINT_BUNDLE`.
`constraints/safety/SafetyBundle.hpp`	Removes registry include + header-level self-registration macro.
`constraints/safety/SafetyBundle.cpp`	Adds registry include + TU-level `REGISTER_CONSTRAINT_BUNDLE`.
`constraints/systemic_exposure/SystemicExposureBundle.hpp`	Removes registry include + header-level self-registration macro.
`constraints/systemic_exposure/SystemicExposureBundle.cpp`	Adds registry include + TU-level `REGISTER_CONSTRAINT_BUNDLE`.
`docs/CHANGELOG.md`	Adds ISO-8601 dates to version headings.
`docs/constraint_bundles.md`	Renames “Global Safety” to “Safety” to match domain key naming.
`docs/evaluation_pipeline.md`	Fixes `\\n` escaping in C++ snippet output strings.
`pyproject.toml`	Adds PEP 621 `[project]` metadata.
`scoring/ScoringReport.hpp`	Documents `domain_name::signal_name` key format for breakdown maps.
`scoring/WeightProfile.hpp`	Adds `narrative_annotation()` hook; clarifies scaling-factor intent; minor parameter-unused cleanup.
`scoring/WeightProfile.cpp`	Updates rationale comment for having a `.cpp` TU for an otherwise inline profile definition.
`scoring/WeightedScoringEngine.cpp`	Switches from profile-name string comparison to `narrative_annotation()`; appends domain evaluation narrative to summary.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-10T14:00:11Z

+[project]
+name = "CuraFrame"
+version = "3.0.0"
+description = "Constraint-driven therapeutic design reasoning framework."
+requires-python = ">=3.9"


[project] metadata is now duplicated between pyproject.toml and setup.py (name/version/description/python_requires). This creates two sources of truth and can cause drift or ambiguous packaging behavior with setuptools. Consider moving all core metadata to pyproject.toml and reducing setup.py to a minimal shim (or removing the duplicated fields) so only one file owns the version and project metadata.

Copilot · 2026-04-10T14:00:12Z

+// All WeightProfile methods are implemented inline in WeightProfile.hpp because
+// the class relies entirely on virtual dispatch with small, default-valued
+// implementations that benefit from inlining.  This translation unit exists to
+// compile the header as part of the curaframe_cpp static library, ensuring that
+// the vtable and RTTI for WeightProfile, DefaultResearchProfile, and
+// HighSafetyProfile are emitted exactly once.


This comment claims compiling this TU ensures the vtable/RTTI for these polymorphic types are emitted exactly once, but with all virtual functions (including the destructor) defined inline there is no key function, so vtable emission is not guaranteed to be centralized in this TU. Either adjust the comment to reflect that this file primarily exists to force compilation of the header in the library, or introduce an out-of-line key function (e.g., a non-inline virtual destructor definition) if you truly need single-TU vtable emission.

Copilot · 2026-04-10T14:00:12Z

+# All constraint-bundle translation units
+set(BUNDLE_SOURCES
+    constraints/anti_infective/AntiInfectiveBundle.cpp
+    constraints/cardiac/CardiacBundle.cpp
+    constraints/cns/CNSBundle.cpp


PR description says the build compiles "11 bundle TUs", but BUNDLE_SOURCES here lists 12 bundles (including systemic_exposure). Please update the PR description or the source list so they match.

Copilot AI assigned Copilot and dfeen87 Apr 10, 2026

Copilot created this pull request from a session on behalf of dfeen87 April 10, 2026 13:54 View session

dfeen87 marked this pull request as ready for review April 10, 2026 13:54

Copilot AI review requested due to automatic review settings April 10, 2026 13:54

dfeen87 merged commit 963df3a into main Apr 10, 2026
2 checks passed

dfeen87 deleted the copilot/validate-cpp-subsystem-architecture branch April 10, 2026 13:54

Copilot started reviewing on behalf of dfeen87 April 10, 2026 13:55 View session

Copilot AI reviewed Apr 10, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: resolve critical and minor issues from C++ subsystem validation#54

fix: resolve critical and minor issues from C++ subsystem validation#54
dfeen87 merged 1 commit intomainfrom
copilot/validate-cpp-subsystem-architecture

Copilot AI commented Apr 10, 2026

Uh oh!

Uh oh!

chatgpt-codex-connector Bot commented Apr 10, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Apr 10, 2026

Uh oh!

Copilot AI Apr 10, 2026

Uh oh!

Copilot AI Apr 10, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

Copilot AI commented Apr 10, 2026

Build system (Critical)

Self-registration macro placement (Minor #13 / partially Major #2)

Interface and type correctness

Brittle profile-name string comparison eliminated

Narrative completeness

Documentation and metadata

Uh oh!

Uh oh!

chatgpt-codex-connector Bot commented Apr 10, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Apr 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 10, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants