M7.2: add normalized solution metrics foundation - #14
Conversation
![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: b811de1616
ℹ️ 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".
|
|
||
| function pricingStatusFromCost(productionCost) { | ||
| if (!productionCost) return PRICING_STATUS.INCOMPLETE; | ||
| if (Number.isFinite(Number(productionCost.estimatedTotalCost))) return PRICING_STATUS.READY; |
There was a problem hiding this comment.
When an incomplete cost object has estimatedTotalCost: null, Number(null) evaluates to 0, so the solution is marked pricing ready, normalized with a zero total, and accepted by createDecisionSolution(). Such a candidate can incorrectly win cost-first ranking despite having no total price; check for an actual non-null numeric value before declaring pricing ready.
Useful? React with 👍 / 👎.
| if (metrics.pricingStatus !== PRICING_STATUS.READY || metrics.estimatedTotalCost === null) { | ||
| throw new RangeError("pricing must be ready before estimatedTotalCost can enter decision ranking"); | ||
| } |
There was a problem hiding this comment.
When fileUnderproduction or pairUnderproduction is positive, createSolutionMetrics() correctly sets zeroUnderproduction to false, but this adapter still creates a rankable solution and drops all underproduction information. The ranker therefore cannot enforce its immutable zero-underproduction constraint, allowing an insufficient-production candidate to be recommended; reject these metrics before conversion.
AGENTS.md reference: AGENTS.md:L50-L52
Useful? React with 👍 / 👎.
| pressPasses: metrics.pressPasses, | ||
| splitOrders: metrics.splitOrders, | ||
| impositionCount: metrics.impositionCount, | ||
| layoutCompactness: metrics.layoutCompactness, |
There was a problem hiding this comment.
When a normalized solution explicitly uses the supported layoutCompactness: null state, this adapter passes the null through. The existing ranker converts objective values with Number(...), so null silently becomes 0; a compactness-first profile then treats missing data as a real worst score instead of rejecting an incomplete metric, potentially changing the recommendation.
Useful? React with 👍 / 👎.
| orderedFinishedQuantity = null, | ||
| productionCost = null, | ||
| } = {}) { | ||
| const normalizedCost = normalizeProductionCost(productionCost); |
There was a problem hiding this comment.
When a caller accidentally supplies a productionCost calculated for another candidate, the normalizer accepts its total while independently retaining this solution's physicalSheets, colorPlates, and layoutForms. Because the cost object already records all three basis counts, compare them here before constructing the normalized model; otherwise ranking can use a cost that contradicts the candidate's production metrics.
Useful? React with 👍 / 👎.
Close Codex review findings from PR #14 by hardening normalized solution metrics before decision ranking: null cost stays incomplete, underproduction and unknown compactness are rejected, and imported production cost must match the candidate metrics.
1 participant
Summary
Starts M7.2 (
0.7.0-alpha.2) with a small, non-invasive foundation layer for normalized solution metrics.Changes
src/solution-metrics.jswith one normalizedSolutionMetricsmodel.pricing ready/pricing incompletestates.null.createDecisionSolution()adapter so pricing-ready normalized metrics can feed the existing M7.1 lexicographic ranking.tests/solution-metrics.test.jscovering incomplete pricing, ready BYN costing, refusal to rank missing prices, and cost-first ranking.src/solution-metrics.jstonpm run check:source.Safety
M7.2 scope alignment
This is the first M7.2 patch toward a unified metrics model for manual, paper-minimum, and future alternatives, plus explicit pricing readiness before pricing-based decisions.