Skip to content

Refactor string use and fix memory leaks #52

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
popmonkey merged 6 commits into from
Jan 15, 2026
Merged

Refactor string use and fix memory leaks #52

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
f0df8ca
refactor to minimize string usage
popmonkey Jan 15, 2026
0b250f0
missing newlines ¯\_(ツ)_/¯
popmonkey Jan 15, 2026
8df63c3
make formatter output test part of integration
popmonkey Jan 15, 2026
8b12815
protect against bad inputs
popmonkey Jan 15, 2026
e37194d
fix memory leaks when input json is bad
popmonkey Jan 15, 2026
46cccbd
verify multi-day race doesn't break with new mapping
popmonkey Jan 15, 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
1 change: 1 addition & 0 deletions .github/workflows/linux-build.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,7 @@ jobs:
- name: Run Integration Tests
run: |
chmod +x test/integration/run_test.sh
chmod +x test/integration/test_formatter_stdout.sh
./test/integration/run_test.sh

# ---------------------------------------------------------
Expand Down
1 change: 0 additions & 1 deletion CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,6 @@ add_library(jres_solver_lib
src/jres_solver.cpp
src/jres_json_converter.cpp
src/jres_internal_types.cpp
src/jres_solver_base.cpp
src/jres_standard_solver.cpp
src/analysis/capacity_analyzer.cpp
src/analysis/solver_diagnostics.cpp
Expand Down
2 changes: 1 addition & 1 deletion GEMINI.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ ctest

There is also a script to test the formatter's stdout functionality:
```bash
./test_formatter_stdout.sh
./test/integration/test_formatter_stdout.sh
```

### Running the CLI Tools
Expand Down
2 changes: 1 addition & 1 deletion cmd/solver/cli.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -212,4 +212,4 @@ int main(int argc, char **argv)
}

return returnCode;
}
}
2 changes: 1 addition & 1 deletion include/jres_solver/jres_solver.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -237,4 +237,4 @@ JRES_SOLVER_API void free_json_string(char* json_string);
} // extern "C"
#endif

#endif // JRES_SOLVER_HPP
#endif // JRES_SOLVER_HPP
41 changes: 20 additions & 21 deletions src/analysis/capacity_analyzer.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,29 +14,26 @@ CapacityAnalysis CapacityAnalyzer::calculate_max_potential_capacity(
const std::vector<TeamMember>& participants,
const SolverInput& input)
{
// Parse stint times once
std::vector<std::chrono::system_clock::time_point> startTimes;
std::vector<std::chrono::system_clock::time_point> endTimes;
std::vector<std::time_t> startTimes;
std::vector<std::time_t> endTimes;
startTimes.reserve(input.stints.size());
endTimes.reserve(input.stints.size());

std::chrono::system_clock::time_point raceStart;
std::chrono::system_clock::time_point raceEnd;
std::time_t raceStart;
std::time_t raceEnd;
bool raceTimesInit = false;

for (const auto& stint : input.stints) {
auto s = TimeHelpers::stringToTimePoint(stint.startTime);
auto e = TimeHelpers::stringToTimePoint(stint.endTime);
startTimes.push_back(s);
endTimes.push_back(e);
startTimes.push_back(stint.startTime);
endTimes.push_back(stint.endTime);

if(!raceTimesInit) {
raceStart = s;
raceEnd = e;
raceStart = stint.startTime;
raceEnd = stint.endTime;
raceTimesInit = true;
} else {
if(s < raceStart) raceStart = s;
if(e > raceEnd) raceEnd = e;
if(stint.startTime < raceStart) raceStart = stint.startTime;
if(stint.endTime > raceEnd) raceEnd = stint.endTime;
}
}

Expand All @@ -47,10 +44,11 @@ CapacityAnalysis CapacityAnalyzer::calculate_max_potential_capacity(
for (const auto& p : participants) {
// Build Availability
std::vector<bool> is_available(input.stints.size(), true);
auto member_availability_it = input.availability.find(p.name);
auto member_availability_it = input.availability.find(p.nameId);
if (member_availability_it != input.availability.end()) {
for (size_t s = 0; s < input.stints.size(); ++s) {
std::string key = TimeHelpers::timePointToKey(startTimes[s]);
// Check if this stint time is unavailable
std::time_t key = TimeHelpers::roundToHour(startTimes[s]);
auto time_it = member_availability_it->second.find(key);
if (time_it != member_availability_it->second.end() &&
time_it->second == Availability::Unavailable) {
Expand All @@ -68,24 +66,24 @@ CapacityAnalysis CapacityAnalyzer::calculate_max_potential_capacity(
planned_drive[s] = true;
base_capacity++;

auto duration_ms = std::chrono::duration_cast<std::chrono::milliseconds>(endTimes[s] - startTimes[s]).count();
driver_total_hours += static_cast<double>(duration_ms) / 3600000.0;
double h = std::difftime(endTimes[s], startTimes[s]) / 3600.0;
driver_total_hours += h;
}
}

// Adjust for Global Minimum Rest (One Instance)
int final_capacity = base_capacity;
if (input.minimumRestHours > 0) {
auto minRestDuration = std::chrono::hours(input.minimumRestHours);
long long minRestSeconds = (long long)input.minimumRestHours * 3600;
int min_loss = base_capacity;
bool found_valid_window = false;

std::vector<std::chrono::system_clock::time_point> candidateStarts;
std::vector<std::time_t> candidateStarts;
candidateStarts.push_back(raceStart);
for(const auto& t : endTimes) candidateStarts.push_back(t);

for(const auto& tStart : candidateStarts) {
auto tEnd = tStart + minRestDuration;
auto tEnd = tStart + minRestSeconds;
if (tEnd > raceEnd) continue;
found_valid_window = true;

Expand All @@ -109,7 +107,8 @@ CapacityAnalysis CapacityAnalyzer::calculate_max_potential_capacity(

analysis.totalCapacity += final_capacity;

ss << "\n- " << p.name << ": " << final_capacity
std::string name = input.strings.get_string(p.nameId);
ss << "\n- " << name << ": " << final_capacity
<< " stints (approx " << std::fixed << std::setprecision(1) << driver_total_hours
<< "h, MinRest=" << input.minimumRestHours << "h)";
}
Expand Down
86 changes: 45 additions & 41 deletions src/analysis/solver_diagnostics.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,59 +18,61 @@ namespace jres::analysis {

std::string explain_assignment_failure(
int stintIndex,
const std::string& violationDriver,
jres::internal::ID violationDriverId,
const jres::internal::SolverInput& input,
const std::vector<jres::internal::TeamMember>& driverPool,
const std::map<std::pair<std::string, int>, int>& driverWorkVars,
const std::map<std::pair<jres::internal::ID, int>, int>& driverWorkVars,
const std::vector<double>& colValues)
{
using namespace jres::internal;
std::ostringstream ss;

// Time points for the target stint
auto tStart = TimeHelpers::stringToTimePoint(input.stints[stintIndex].startTime);
auto tEnd = TimeHelpers::stringToTimePoint(input.stints[stintIndex].endTime);
auto tStart = input.stints[stintIndex].startTime;
auto tEnd = input.stints[stintIndex].endTime;

// Identify which drivers are assigned to which stints in the CURRENT solution
std::map<std::string, std::set<int>> driverAssignments;
std::map<ID, std::set<int>> driverAssignments;
for (size_t s = 0; s < input.stints.size(); ++s) {
for (const auto& p : driverPool) {
if (driverWorkVars.count({p.name, (int)s})) {
int idx = driverWorkVars.at({p.name, (int)s});
if (driverWorkVars.count({p.nameId, (int)s})) {
int idx = driverWorkVars.at({p.nameId, (int)s});
if (idx < (int)colValues.size() && colValues[idx] > 0.5) {
driverAssignments[p.name].insert((int)s);
driverAssignments[p.nameId].insert((int)s);
}
}
}
}

// Helper: Get stint duration in hours
auto getDuration = [&](int sIdx) {
auto s = TimeHelpers::stringToTimePoint(input.stints[sIdx].startTime);
auto e = TimeHelpers::stringToTimePoint(input.stints[sIdx].endTime);
return std::chrono::duration<double, std::ratio<3600>>(e - s).count();
return std::difftime(input.stints[sIdx].endTime, input.stints[sIdx].startTime) / 3600.0;
};

ss << " Alternatives Analysis:";

for (const auto& candidate : driverPool) {
if (candidate.name == violationDriver) continue;
if (candidate.nameId == violationDriverId) continue;

ss << "\n - " << candidate.name << ": ";
std::string candidateName = input.strings.get_string(candidate.nameId);

ss << "\n - " << candidateName << ": ";
std::vector<std::string> reasons;

// Check Availability
bool isUnavailable = false;
auto it = input.availability.find(candidate.name);
auto it = input.availability.find(candidate.nameId);
if (it != input.availability.end()) {
auto cursor = tStart;
// Align cursor to hour if not already (assuming availability is hourly)
cursor = TimeHelpers::roundToHour(cursor);

while (cursor < tEnd) {
std::string key = TimeHelpers::timePointToKey(cursor);
if (it->second.count(key) && it->second.at(key) == Availability::Unavailable) {
if (it->second.count(cursor) && it->second.at(cursor) == Availability::Unavailable) {
isUnavailable = true;
break;
}
cursor += std::chrono::hours(1);
cursor += 3600;
}
}
if (isUnavailable) {
Expand All @@ -80,11 +82,11 @@ std::string explain_assignment_failure(
// Check Consecutive Stints
int consecutiveCount = 1;
for (int k = stintIndex - 1; k >= 0; --k) {
if (driverAssignments[candidate.name].count(k)) consecutiveCount++;
if (driverAssignments[candidate.nameId].count(k)) consecutiveCount++;
else break;
}
for (size_t k = stintIndex + 1; k < input.stints.size(); ++k) {
if (driverAssignments[candidate.name].count((int)k)) consecutiveCount++;
if (driverAssignments[candidate.nameId].count((int)k)) consecutiveCount++;
else break;
}

Expand All @@ -95,15 +97,15 @@ std::string explain_assignment_failure(
// Check Minimum Rest
if (input.minimumRestHours > 0) {
double minRestSec = input.minimumRestHours * 3600.0;
for (int assignedS : driverAssignments[candidate.name]) {
auto s1_start = TimeHelpers::stringToTimePoint(input.stints[stintIndex].startTime);
auto s1_end = TimeHelpers::stringToTimePoint(input.stints[stintIndex].endTime);
auto s2_start = TimeHelpers::stringToTimePoint(input.stints[assignedS].startTime);
auto s2_end = TimeHelpers::stringToTimePoint(input.stints[assignedS].endTime);
for (int assignedS : driverAssignments[candidate.nameId]) {
auto s1_start = input.stints[stintIndex].startTime;
auto s1_end = input.stints[stintIndex].endTime;
auto s2_start = input.stints[assignedS].startTime;
auto s2_end = input.stints[assignedS].endTime;

double gap = 0.0;
if (s1_end <= s2_start) gap = std::chrono::duration<double>(s2_start - s1_end).count();
else if (s2_end <= s1_start) gap = std::chrono::duration<double>(s1_start - s2_end).count();
if (s1_end <= s2_start) gap = std::difftime(s2_start, s1_end);
else if (s2_end <= s1_start) gap = std::difftime(s1_start, s2_end);
else gap = -1.0;

if (gap < minRestSec - 1.0) {
Expand All @@ -120,11 +122,11 @@ std::string explain_assignment_failure(
if (input.maximumBusyHours > 0) {
double busyDuration = getDuration(stintIndex);
for (int k = stintIndex - 1; k >= 0; --k) {
if (driverAssignments[candidate.name].count(k)) busyDuration += getDuration(k);
if (driverAssignments[candidate.nameId].count(k)) busyDuration += getDuration(k);
else break;
}
for (size_t k = stintIndex + 1; k < input.stints.size(); ++k) {
if (driverAssignments[candidate.name].count((int)k)) busyDuration += getDuration(k);
if (driverAssignments[candidate.nameId].count((int)k)) busyDuration += getDuration(k);
else break;
}
if (busyDuration > input.maximumBusyHours) {
Expand Down Expand Up @@ -171,8 +173,8 @@ std::string formatStintList(const std::vector<int>& indices, const jres::interna
std::vector<std::string> formatHumanDiagnostic(
const std::map<int, jres::internal::SlackInfo>& slackInfo,
const std::set<int>& unavailableVars,
const std::map<std::pair<std::string, int>, int>& driverWorkVars,
const std::map<std::pair<std::string, int>, int>& spotterWorkVars,
const std::map<std::pair<jres::internal::ID, int>, int>& driverWorkVars,
const std::map<std::pair<jres::internal::ID, int>, int>& spotterWorkVars,
const std::vector<double>& colValues,
const jres::internal::SolverInput& input,
const std::vector<jres::internal::TeamMember>& driverPool,
Expand Down Expand Up @@ -206,14 +208,14 @@ std::vector<std::string> formatHumanDiagnostic(
std::map<ViolationKey, std::vector<int>> groupedViolations;
std::vector<std::string> globalViolations;

// Build reverse map for variable index -> (Member, Stint, Role)
// Build reverse map for variable index -> (MemberName, Stint, Role)
std::map<int, std::tuple<std::string, int, std::string>> varToInfo;

for(const auto& [key, varIdx] : driverWorkVars) {
varToInfo[varIdx] = std::make_tuple(key.first, key.second, "Driver");
varToInfo[varIdx] = std::make_tuple(input.strings.get_string(key.first), key.second, "Driver");
}
for(const auto& [key, varIdx] : spotterWorkVars) {
varToInfo[varIdx] = std::make_tuple(key.first, key.second, "Spotter");
varToInfo[varIdx] = std::make_tuple(input.strings.get_string(key.first), key.second, "Spotter");
}

// Check Unavailable (Priority 0)
Expand All @@ -235,23 +237,25 @@ std::vector<std::string> formatHumanDiagnostic(
if (info.type.find("Busy") != std::string::npos) priority = 1;
else if (info.type.find("Rest") != std::string::npos) priority = 2;
else if (info.type.find("Fair Share") != std::string::npos) priority = 2;

std::string infoMemberName = input.strings.get_string(info.memberNameId);

if (info.stintIndex >= 0) {
std::string reason;
if (priority == 1) reason = "Max Busy Time exceeded (" + info.memberName + ")";
if (priority == 1) reason = "Max Busy Time exceeded (" + infoMemberName + ")";
else if (priority == 2) {
if (info.type.find("Fair Share") != std::string::npos) reason = "Fair Share Rules violated (" + info.memberName + ")";
else reason = "Rest Rules violated (" + info.memberName + ")";
if (info.type.find("Fair Share") != std::string::npos) reason = "Fair Share Rules violated (" + infoMemberName + ")";
else reason = "Rest Rules violated (" + infoMemberName + ")";
}
else reason = info.type + " (" + info.memberName + ")";
else reason = info.type + " (" + infoMemberName + ")";

groupedViolations[{priority, reason, info.memberName}].push_back(info.stintIndex);
groupedViolations[{priority, reason, infoMemberName}].push_back(info.stintIndex);
} else {
// Try to attribute global violation to stints
bool assignedAny = false;
for(const auto& [vIdx, tupleInfo] : varToInfo) {
auto [memName, stintIdx, role] = tupleInfo;
if (memName == info.memberName && colValues[vIdx] > 0.5) {
if (memName == infoMemberName && colValues[vIdx] > 0.5) {
std::string reason;
if (priority == 1) reason = "Max Busy Time exceeded (" + memName + ")";
else if (priority == 2) {
Expand All @@ -273,7 +277,7 @@ std::vector<std::string> formatHumanDiagnostic(
else
reason = "Rest Rules violated";
}
globalViolations.push_back(reason + " (" + info.memberName + ")");
globalViolations.push_back(reason + " (" + infoMemberName + ")");
}
}
}
Expand Down Expand Up @@ -314,4 +318,4 @@ std::vector<std::string> formatHumanDiagnostic(
return report;
}

} // namespace jres::analysis
} // namespace jres::analysis
8 changes: 4 additions & 4 deletions src/analysis/solver_diagnostics.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,10 @@ namespace jres::analysis {
*/
std::string explain_assignment_failure(
int stintIndex,
const std::string& violationDriver,
jres::internal::ID violationDriverId,
const jres::internal::SolverInput& input,
const std::vector<jres::internal::TeamMember>& driverPool,
const std::map<std::pair<std::string, int>, int>& driverWorkVars,
const std::map<std::pair<jres::internal::ID, int>, int>& driverWorkVars,
const std::vector<double>& colValues
);

Expand All @@ -40,8 +40,8 @@ std::string explain_assignment_failure(
std::vector<std::string> formatHumanDiagnostic(
const std::map<int, jres::internal::SlackInfo>& slackInfo,
const std::set<int>& unavailableVars,
const std::map<std::pair<std::string, int>, int>& driverWorkVars,
const std::map<std::pair<std::string, int>, int>& spotterWorkVars,
const std::map<std::pair<jres::internal::ID, int>, int>& driverWorkVars,
const std::map<std::pair<jres::internal::ID, int>, int>& spotterWorkVars,
const std::vector<double>& colValues,
const jres::internal::SolverInput& input,
const std::vector<jres::internal::TeamMember>& driverPool,
Expand Down
Loading