feat: add exact-cover allocation strategy#663
Conversation
…dation-engine-contract # Conflicts: # .github/workflows/security.yml
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9c8e77d88c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| let limits = request | ||
| .limits | ||
| .unwrap_or_default() | ||
| .bounded(DEFAULT_MAX_OPTIONS, DEFAULT_MAX_SEARCH_NODES); |
There was a problem hiding this comment.
The handler currently derives limits only from request.limits plus hardcoded defaults, so the new module settings (allocation_strategy, exact_cover_max_options, exact_cover_max_search_nodes) loaded in RecommendationEngineConfig::load are never enforced here. In practice, an admin can lower exact_cover_max_options in module config and still receive allocations computed with up to 256 options unless every caller also sends per-request limits, which makes the new config surface effectively a no-op.
Useful? React with 👍 / 👎.
| if let Err(err) = app_state.db.save_audit_log(&entry).await { | ||
| tracing::warn!( | ||
| %trace_id, | ||
| error = ?err, | ||
| "failed to persist exact-cover allocation audit trace" | ||
| ); | ||
| } |
There was a problem hiding this comment.
save_audit_log failures are only logged, so the endpoint can return success with an allocation_trace_id even when no trace was persisted. Under DB write failures (e.g., disk/full or transient storage issues), this silently violates the traceability guarantee for served exact-cover allocations and leaves operators with a response ID that has no immutable audit record.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds a new admin-only exact_cover_v1 allocation lane for batch/recurring recommendation constraints, while keeping weighted_v1 as the default for the standard recommendation workflow.
Changes:
- Introduces
RecommendationAllocationConfigand module settings for allocation strategy + solver limits. - Adds a new
/api/v1/recommendations/allocation/exact-coverendpoint with auditing + deterministic Algorithm X solver. - Adds shared exact-cover fixtures + contract gate checks and updates contract docs.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| scripts/check-recommendation-contract.sh | Extends the contract gate to validate exact-cover fixtures and key contract strings. |
| parkhub-server/src/api/recommendations.rs | Adds allocation config to engine config/stats and loads new module settings. |
| parkhub-server/src/api/recommendation_allocation.rs | New exact-cover solver + admin endpoint + audit trace generation + fixtures-based tests. |
| parkhub-server/src/api/modules/schemas.rs | Extends recommendations module schema with allocation strategy and solver limits. |
| parkhub-server/src/api/mod.rs | Wires the new allocation module and POST route. |
| docs/recommendation-engine-fixtures/exact_cover_v1.*.json | Adds parity fixtures to pin exact-cover semantics across implementations. |
| docs/recommendation-engine-contract.md | Documents exact-cover strategy semantics, config boundary, and audit trace requirements. |
| let state_guard = state.read().await; | ||
| if let Err((status, msg)) = check_admin(&state_guard, &auth_user).await { | ||
| return (status, Json(ApiResponse::error("FORBIDDEN", msg))); | ||
| } | ||
| drop(state_guard); |
| { | ||
| let state_guard = state.read().await; | ||
| audit_exact_cover_allocation( | ||
| &state_guard, | ||
| allocation_trace_id, | ||
| &auth_user, | ||
| &request, | ||
| limits, | ||
| &result, | ||
| ) | ||
| .await; | ||
| } | ||
|
|
| pub struct RecommendationAllocationConfig { | ||
| pub strategy: String, | ||
| pub exact_cover_max_options: usize, | ||
| pub exact_cover_max_search_nodes: usize, | ||
| } |
| cfg.allocation.strategy = | ||
| read_module_string(db, "allocation_strategy", "weighted_v1").await; | ||
| if !matches!( | ||
| cfg.allocation.strategy.as_str(), | ||
| "weighted_v1" | "exact_cover_v1" | ||
| ) { | ||
| tracing::warn!( | ||
| allocation_strategy = %cfg.allocation.strategy, | ||
| "unknown allocation strategy requested; falling back to weighted_v1" | ||
| ); | ||
| cfg.allocation.strategy = "weighted_v1".to_string(); | ||
| } |
| fn status_name(status: ExactCoverStatus) -> &'static str { | ||
| match status { | ||
| ExactCoverStatus::Solved => "solved", | ||
| ExactCoverStatus::FallbackNoSolution => "fallback_no_solution", | ||
| ExactCoverStatus::FallbackInputLimited => "fallback_input_limited", | ||
| ExactCoverStatus::FallbackSearchLimited => "fallback_search_limited", | ||
| } | ||
| } |
| "human_review_required": true, | ||
| "note": "Exact-cover allocation is operational scheduling support, not legal advice or a compliance certification." |
| @@ -358,7 +393,6 @@ pub struct RecommendationQuery { | |||
|
|
|||
| #[derive(Debug, Clone, Serialize, utoipa::ToSchema)] | |||
| pub struct SlotRecommendation { | |||
Adds the exact-cover allocation strategy lane for recommendation constraints while keeping weighted_v1 as the normal default.
Local Nido evidence: