feat(MD-16): material sale value subset (DRAFT, do not merge) - #153
Draft
TheCodingDad-TisonK wants to merge 4 commits into
Draft
TheCodingDad-TisonK wants to merge 4 commits into
TheCodingDad-TisonK wants to merge 4 commits into
Conversation
The assigned MD-16 draft subset off development 9e3ae20, per Tyson's assignment of 2026-09-15. Sale components and getSaleQuoteComponents, the grade/organic reducer, the futures plan, the five-argument sellFillType wrapper, the OrganicPremium retirement, the vendored codec, the private preview event and the resolvers on stub providers. No RfPdaMenuPage.xml edit in this subset. DO NOT MERGE. This retires the pooled OrganicPremium modifier in every mode, including when MD-16 is disabled or unavailable, because a farm-average premium is the wrong answer to "what is THIS load worth". Its replacement is the captured-origin term, which needs SG-1, SG-2, SG-4, SG-3 and FP-1 Organic to exist. Merging before those land removes the live organic premium with nothing in its place. This branch is deliberately NOT deployed for the same reason. - src/md16/Md16Material.lua is the pure arithmetic: grade term, captured-origin term, rate composition and the ordered reduction. The consumer rail is applied AFTER the material factors compose, never before. Grade is a whole-call decision, so one unknown required part puts every paid litre in the grade-unavailable bucket rather than in C. Organic is independent and shares no fate with grade: every paid litre stays in the denominator while only proved eligible litres enter the numerator. - src/md16/Md16SaleComponents.lua captures SaleComponentsV1 from inside the existing MarketEngine:_recalculate, while each registered callback is evaluated exactly once. otherConsumerProduct is stored UNCLAMPED, because the rail belongs after the material terms and a pre-clamped record would make the correct composition unreachable. marketRevision is an opaque mission-local revision advanced by an actual component change, never by the clock. - The retirement is applied at the COMPOSITION point rather than at registration, so it holds whichever mod registered the modifier, whether MD-16 is enabled and whether its bridge ran. The name stays reserved; it simply cannot contribute again. - src/md16/Md16FuturesPlan.lua freezes and validates the allocation plan and extends onCropDelivered with an optional fifth argument, so every existing caller is untouched and an unadmitted plan falls back to the unchanged legacy path. Contract litres are valued at the baseline and only spot excess receives the material term; one weighted rate pays the whole call. baselineValue is the TOTAL for its litres and is converted to a per-litre rate once, in one place, because recordDelivery multiplies. - src/PriceHook.lua takes the native five-argument sale vector. The old six-slot shape did NOT drop toolType or extraAttributes, since forwarding the unchanged positional vector passed them in their original slots; what it did was misname the wrapper's own locals, so a named read got the wrong value. Both facts are asserted rather than just the convenient one. - src/md16/Md16Values.lua vendors the data-only SG_VALUES_2 codec so the native floor does not require StockGuard installed. The wire is deliberately identical and the bench asserts the format and version tokens so a silent drift fails. - src/md16/MarketDynamicsSalePreviewEvent.lua is the owner-private demand/reply channel with the exact declared wire order, direction checked on both the write and the read, budgets that refuse rather than truncate, and request records that REFUSE an extra trusted field rather than ignoring it. - src/md16/Md16Resolvers.lua holds the private read contract. Every provider it reads is unbuilt today, so the absent path is written as the normal path: protected probes, explicit unavailable with a reason, and no favorable default anywhere. The destination role is derived freshly from the real getStoreGoods and getSkipSell, and an unresolved policy is unsupported rather than assumed to store. The latest-outcome map is keyed by farm AND destination, so one farm's last sale can never be read by another at a shared public station. Bench: new tools/test/lua/MD-16-material_sale_value_spec_test.lua, 164 assertions, suite 609/0 across 6 files (445 before). The certified reference bar is ported against the BUILT modules rather than local copies of the arithmetic, so the shipped constants and the certified numbers are the same numbers. The retirement case deliberately sits inside the 0.5-3.0 rail: at a consumer product of 10 the rail clamps to 3 with or without the premium, so a test run there would pass against both builds and discriminate nothing. Syntax and lint clean, no em dashes. Built but NOT deployed.
sasha-rf
previously approved these changes
Sep 16, 2026
sasha-rf
left a comment
sasha-rf
left a comment
There was a problem hiding this comment.
Hey! I gave this branch a proper look and it is all good: every source file at the PR head reads clean (no UTF-16, which would break the game at load), and it follows the one-feature-one-PR protocol. Approved for merge.
DRAFT approval only - this does NOT clear the PR to merge. MD-16 material sale value subset at 6747d7a: sale components captured once per _recalculate pass (unclamped product, correct rail-after-material-terms order), grade/organic reducers correctly kept independent with distinct denominators, the pooled OrganicPremium retirement applied at composition point so it holds regardless of MD-16 enable state, the five-vs-six-argument sellFillType wrapper bug (toolType/extraAttributes silently misnamed/nil) caught and fixed, resolvers written for the honest all-unbuilt-provider state with no favorable default. Bench 609/0, reference bar ported against built modules not re-implemented arithmetic. Approving on the merits of what's here; the merge gate stands as stated in the PR body - this stays blocked until SG-1/SG-2/SG-4/SG-3/FP-1 Organic land, and the branch must not be deployed to any game (would strip the live organic premium with nothing replacing it). Noted the organic-premium reshape (farm-wide flat to per-load proved-origin) as a pre-draft-worthy player communication item for when this eventually ships.
UTF-16 scan: 11 file(s) read at the PR head, all clean.
@TheCodingDad-TisonK verified and approved, ready to merge.
added 3 commits
September 16, 2026 10:49
…747d7a) Still a DRAFT and still must not deploy. The retirement needs SG-1, SG-2, SG-4, SG-3 and FP-1 Organic before it can reach a player's game. MAJOR 1, and it is the one that mattered. The retirement hung on a string literal that nothing bound to the name the bridge actually registers, and the bench registered its probe under that same constant, so the two agreed with each other and neither agreed with reality. Bob set the constant to "OrganicPremiumX" and the bar stayed 609/0 green while the premium kept paying. Added C.verifyRetiredModifierBinding, called from the mission load, which compares the constant against OrganicPremiumBridge.MODIFIER_NAME and says loudly that the premium is still paying if they differ. The bench now loads the bridge and pins the two, and drifts the registry name to prove the check catches it. MAJOR 2. effectiveFactor blended the grade factors and never railed, while the money path rails each bucket inside partRate and only then blends. They agree until a bucket crosses the rail: base 100, product 2.8, 50 L grade A with 50 L grade C pays 290 and the blended factor implied 300. A single factor cannot express the railed result, because railing happens per bucket against the consumer product and the reducer is not given it, so the field is gone from M.reduce and replaced by M.effectiveFactor(reduced, base, product), which derives the answer from the same partRate and weightedRate the money uses. MAJOR 3 and MAJOR 4. C.validate's eight refusal branches and weightedRate's two had no coverage at all: a blanket pass-through and a zero-instead-of-nil both left the bar green. Covered, each through the public reader. MAJOR 5. C.reset was documented as called on mission load and teardown and nothing called it, so a second savegame in one process inherited the first one's records and revision counter. Wired into onMissionLoaded and delete, guarded so MarketDynamics still works with the MD-16 modules absent. MAJOR 6. The capture cleared the pure-client early return only when entry.current was already set, so a client asked for a price before the server's first sync composed locally and captured a client-local record that getSaleQuoteComponents then served as the owner's answer. Gated on g_server. MAJOR 7. The token count travelled as a UInt32 against a 4096 bound, leaving every value from 4097 up expressible; the reader refused, zeroed the count and read no strings, which left the sender's strings in the stream and mis-aligned every later event in the same packet. The count now travels in a field sized to exactly the budget (12 bits, MAX_TOKENS 4095), so an out-of-range count cannot be represented and the branch is gone rather than handled. The bench harness was carrying no width on its sized-integer lane, so a write and read at different widths was invisible; the prelude now carries it and counts a mismatch as a type error. MINOR 1. The bridge logged at INFO that it had registered the premium with its certified 1.2 after the composition point made it unable to contribute, which is the opposite of what ships on the surface used to diagnose a deploy. It now says registered and retired. MINOR 2. Recorded in Md16Resolvers that getStoreGoods and getSkipSell live on a ProductionPoint's unloading station and not on a plain SellingStation, so an ordinary selling station always takes the UNSUPPORTED branch. Nothing is wrong; the READY path is simply unreachable there. Bench 654/0 across 6 files, up from 609/0; the MD-16 file goes 164 to 209. Three mutations run against the new cases and all three were killed by correctly named assertions: dropping the capture's server gate, making validate a pass-through, and returning 0 rather than nil on zero litres.
…ob re-check) MAJOR. The sized-count protection was not exercised on MD-16's own wire. G22b only inspects the DECLARED WRITE width, so the read side was unguarded: reading the token count at a width it was not written at left the bar fully green. The mock stream already counts a tag or width mismatch and an underflow, and RSF-F203 and RSF-F204 both already assert them; MD-16's wire test did not. Added on both round trips it performs. Bob's R5 mutation, reading at 16 bits against the 12-bit write, now fails on G28b rather than passing silently. MINOR. streamWriteUInt32 and streamReadUInt32 in the bench prelude are used by nothing in src and nothing in any test now that the event moved to UIntN. Removed rather than left as a stub that looks like a supported lane. NOT DONE, deliberately: the reset wiring has no regression guard. The only way to assert it from the bench is to read MarketDynamics.lua as text and look for the two call sites, which tests the file rather than the behaviour and goes stale the moment the call moves or is renamed. The behaviour it protects, that reset clears the captured components and restarts the revision counter, is covered by J5, J5b and J5c. Raised rather than papered over. Bench 657/0 across 6 files, up from 654/0.
…(Bob MINOR) I argued against this case on the grounds that the only way to assert the wiring was to read MarketDynamics.lua as text, which tests the file rather than the behaviour and breaks when the call legitimately moves. Bob pointed out a third option I had missed and checked its reachability before suggesting it: call the REAL methods. MarketDynamics.lua is loadable in this bench, and two other tests already carry it. So J8 captures a component, calls the real MarketDynamics:onMissionLoaded, and asserts the component is gone; then captures another, calls the real MarketDynamics:delete, and asserts the same. That executes the wiring instead of reading it, survives the call moving inside its function, and fails if anyone removes it. Both sites are pinned INDIVIDUALLY: deleting the load-side call fails only J8b, deleting the teardown call fails only J8e. Verified by running both mutations. The stubs are confined to the do block and restored after it, and g_client is left nil so the method's own client-only guard skips the six dialog registrations rather than needing them stubbed. This was the gap MAJOR 5 actually was: the code was right and nothing would have noticed if it stopped being called. Same shape as the day's other three. MarketDynamics.lua joins the load list. Bench 663/0 across 6 files, up from 657/0; the MD-16 file goes 209 to 218.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
DO NOT MERGE
Opened as a draft under Tyson's 2026-09-15 assignment and it stays that way until SG-1, SG-2, SG-4, SG-3 and FP-1 Organic land.
This subset retires the pooled OrganicPremium modifier in every mode, including when MD-16 itself is disabled or unavailable (brief :66, :148; the modifier is registered at
src/MarketDynamics.lua:184). Its replacement is the captured-origin term, which needs the providers above to exist. Merging this before they land removes the live organic premium with nothing in its place.For the same reason this branch is built but deliberately not deployed. Deploying it would put the retirement in a player's game today.
What is in the subset
Sale components and
getSaleQuoteComponents, the grade/organic reducer, the futures plan, the five-argumentsellFillTypewrapper, the retirement, the vendored codec, the preview event, and resolvers on stub providers. NoRfPdaMenuPage.xmledit, per the assignment.The arithmetic
src/md16/Md16Material.luais pure: no mission, no market, no native object, no clock, and it writes no money. The same reducer supplies the preview and the later revalidated execution frame, which is what makes it impossible for the two to disagree because one of them only saw the first witnesses.Three rules a builder gets wrong, each asserted as a difference rather than left to reading:
partRate = baseThroughEvents * clamp(otherConsumerProduct * gradeFactor * organicFactor). Multiplying an already clamped quote by the material term breaks the rail; the bench constructs that build and shows it exceeding 300 where the correct one sits exactly on it.Sale components
Captured from inside the existing
MarketEngine:_recalculate, while each registered callback is evaluated exactly once. The alternatives are both wrong: invoking the consumers again during a sale re-runs stateful inputs, and dividing the finished quote cannot separate the rail from what it clamped. The bench asserts that division fails.otherConsumerProductis stored unclamped on purpose, because the rail belongs after the material terms; a pre-clamped record would make the correct composition unreachable.marketRevisionis an opaque mission-local revision advanced by an actual component change, never by the clock, and the bench proves a no-change recalculation does not advance it.The retirement
Applied at the composition point rather than at registration, so it holds whichever mod registered the modifier, whether MD-16 is enabled, and whether its bridge ran at all. The name stays registered and reserved against another suite mod claiming it; the modifier simply cannot contribute again.
The bench case for this deliberately sits inside the 0.5 to 3.0 rail. At a consumer product of 10 the rail clamps to 3 with or without the premium, so a retirement test run there would pass against both builds and prove nothing. At a product of 2 the difference is visible: 200 against the 240 an unretired premium would have paid. I got this wrong on the first run and the assertion failed, which is the only reason it is right now.
Futures
onCropDeliveredgains an optional fifth argument, so every existing caller is unchanged and an unadmitted plan falls back to the legacy path rather than refusing the delivery. Contract-assigned litres are valued at the baseline and only spot excess receives the material term; one weighted rate pays the whole accepted call.Units, because this is where the brief warns a builder goes wrong.
baselineValueis the TOTAL baseline money for its litres, not a per-litre rate. The existingrecordDeliverytakes a rate and multiplies (FuturesMarket.lua:93), so the conversion happens once, in one place, where it can be seen. Handing a total to a function expecting a rate would inflatevalueReceivedby the litre count and silently claw back the player's settlement.The reference bar's allocation is driven through the real
FuturesMarket: 10 assigned litres and 90 spot, the contract storing only its assigned baseline value of 20, and the averaged-rate counterexample losing 6.84 of spot bonus.The five-argument wrapper
The native sale vector is
(farmId, fillDelta, fillTypeIndex, toolType, extraAttributes). The old wrapper declared six slots withfillPositionDatain the middle.Both facts are asserted, not just the convenient one: forwarding the unchanged positional vector did still pass the native toolType and extraAttributes in their original slots, so the pass-through alone did not drop them, and claiming otherwise would be wrong. What it did was misname the wrapper's own locals, so
toolTypeactually held the attributes andextraAttributeswas always nil. Any MD-16 work reading a named local here needs the real shape.Codec and event
The codec is vendored, so the native floor does not require StockGuard installed. The wire is deliberately identical (
SG_VALUES/2) and the bench asserts the format and version tokens so a silent drift fails here rather than on someone's server.The preview event carries the exact declared wire order (isReply Bool, schema UInt8, kind UInt8, requestId Int32, bounded UInt32 token count, then the string tokens), checks direction on both the write and the read, and refuses unknown kind, schema or direction before any apply. Budgets refuse rather than truncate, because a silently clipped record is a wrong answer that looks like a right one. A request carrying an extra trusted field (a farm, a rate, an amount, a plan) refuses rather than ignoring it.
Resolvers, and the honest state of them
Every provider this subset reads is unbuilt today. SG-2's
getNativeSaleInputsV1, SG-3'sassessMaterialUseand Organic'sharvestOrganicOriginV1are paired owner work in their own handoffs, and SG-1's pagedgetManagementViewlands with the StockGuard family.So the absent path is not an edge case here, it is the only path that runs, and it is written as the normal one: protected probes, explicit unavailable with a reason, and no favorable default anywhere. A missing fact must never be worth money. The bench asserts the absent path directly rather than assuming it.
Details worth a reviewer's eye:
managementPagingVersion = 1is refused rather than probed, because feeding a first page to an old whole-farm interpreter is worse than reporting unavailable.getStoreGoodsandgetSkipSell. The four combinations are not symmetric: store plus skip is native storage, neither is a paid spot sale, store without skip is a paid-capacity path only when its basis supports an estimate, and skip without store is unsupported rather than invented storage. An unresolved policy is unsupported, which is a different answer from "stores".NO_PAID_SALEis reported only for a proved non-paying delivery.Bench
New
tools/test/lua/MD-16-material_sale_value_spec_test.lua, 164 assertions. Suite 609 passed, 0 failed across 6 files, up from 445.The certified reference bar is ported against the built modules rather than against local copies of the arithmetic, so the shipped constants and the certified numbers are the same numbers. A design bar that re-implements the rule beside the code proves only that someone can do the multiplication twice.
tools/test/lua/prelude.luagainsstreamWriteUInt32andstreamReadUInt32, which the event's token count needs.Syntax and lint clean, no em dashes. Built, not deployed.
What this subset does not reach
No
RfPdaMenuPage.xmland no Wizard craft. No SG2 facade (paired owner work). No native station execution, and no GUI, locale, multiplayer, save or balance observation: those are implementation and release observations and none is claimed here. The strict FOOD_MILL proposal stays unarmed and excluded, as the brief requires.Bob has the cold diff review and Sasha the PR review. Nobody merges this, including after both approve, until the five gating items land.