Repository navigation
release: promote 2026-Q3 to master - #383
Merged
Merged
Conversation
The half of the shallow copy that the ownership seam now leans on. Modify.Simulate renames its model to the project name and hands it to RunPartOSimulation, which takes the one deep working copy of a Part O run - so the rename relies on Name and the model parameters being fields of AnalyticalModel itself, isolated by a new instance, while only the cluster's objects stay shared. Both halves are asserted together so neither is inferred from the other. Tests: SAM.Tests 1914 passed, 0 failed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…r of valid hours Two current-head findings from Codex review, plus the progress record. The deep clone was not deep --------------------------- Core.Query.Clone resolves by reflection - an instance Clone(), else a single-argument constructor accepting the type, else a parameterless one - and returns null when it finds none. Constructors are NOT inherited, so a subclass of a type that has a copy constructor does not have one. Seven types accepted by AdjacencyCluster.IsValid were in exactly that position. The null clone was handed to AddObject, rejected, and the ORIGINAL instance left in the dictionary the shallow base constructor had already filled - so a declared deep copy silently shared those objects with its source, and an in-place write through it reached the model it was copied from: ZoneSimulationResult, TM52Result, TM59Result, TM59CorridorResult, TM59MechanicalVentilationResult, TM59NaturalVentilationResult, TM59NaturalVentilationBedroomResult Each now has the same-type copy constructor its base already had. An exhaustive reflection audit over every concrete type the cluster accepts (60 in this build) asserts a clone path exists for each, so a type added later fails in a test rather than in a Part O run. And SAMObjectRelationCluster(..., deepClone: true) now THROWS, naming the type, rather than falling back to sharing: a declared deep clone may not quietly leave the source instance in place. Length is not evidence ---------------------- Where a full year is requested, every hour of both series must now be a finite number. Collect skips a temperature it cannot read and treats an unreadable occupancy value as an unoccupied hour; for Approved Document O that is silently wrong, because the criteria are COUNTS - a year with unreadable occupancy is assessed over fewer occupied hours than the building has, against a proportionally smaller allowance, and reads as a normal pass. Zero occupancy is a value; an hour that states nothing is not. Found while testing it: a JsonArray will hold a NaN or an infinity, and reading one back throws ArgumentException out of System.Text.Json. So a single unrepresentable hour anywhere in a building threw out of Calculate_TM59 and lost every space's assessment. The read is guarded; one bad hour now costs at most its own room. The normal Part O path still takes ONE deep clone. Measured Release, 5,000 spaces / 30,000 panels: 250.7 ms and 136.8 MB, against 36.5 ms / 13.8 MB shallow. Tests: SAM.Tests 1935 passed, 0 failed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…red hours are checked Two current-head Codex findings, both verified against the code. A boolean is not a measurement ------------------------------ Core.Query.TryConvert routes a JSON boolean through its bool-to-double conversion, so `true` reads as 1 and `false` as 0 - confirmed by probe. A corrupted full-year series of booleans therefore converted cleanly, passed IsUsableSeries as finite numbers, and could produce an ordinary Part O verdict from occupancy and temperatures that were never measurements. Where a complete year is required the node's own JSON kind must be Number. GetValueKind() is itself guarded, because it throws on the NaN and infinity a JsonArray will hold - it has to decide how the value would serialize, and those do not. Asked in IsUsableSeries and NOT in TryGetHourlyValue, which Collect shares: on the ordinary path a value that converts is read exactly as it always was, and the stricter rule applies only where a caller has asked for a complete year of evidence. Only the hours the assessment requires -------------------------------------- The validation walked the whole array. A series LONGER than the requested year - a leap-year simulation's 8784 against a 365-day one - is deliberately accepted, and Collect then excludes the surplus because the comfort band does not cover it. Validating those surplus hours refused the whole room over an hour that is never assessed, contradicting the rule that accepts the longer series in the first place. The walk is now bounded by HourCount_Expected. Tests: SAM.Tests 1937 passed, 0 failed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
fix(analytical): working-model ownership rule, and a TM59 series that must be whole (2B closeout 1/3)
Six sites resolved an object inside a loop over the model's rooms, zones or dwellings. AdjacencyCluster.GetSpaces()/GetZones() rebuild their whole list on every call and AnalyticalModel.AdjacencyCluster hands out a fresh shallow copy of the entire cluster, so each of them was quadratic. PF7, the largest: TMOverheatingCalculator resolved every assessed room with GetSpaces().Find inside the loop - and the TM52 path went through AnalyticalModel.GetSpaces(), which copies every space in the model, so 5,000 rooms meant 25,000,000 Space copies before an hourly value was read. One Dictionary<Guid, Space> per call now, dropped when it returns. PF4: TM59AssessmentCalculator.Spaces and OverheatingScenarioMap resolved their design zone and copied the design cluster once per zone and once per scenario. Both hold one zone index and one cluster for the traversal, and the room de-duplication is a HashSet rather than a linear Find. PF5: RealizePartFVentilationTerminals re-read a space's terminals after re-linking them; AddObject replaces in place, so they are substituted in place instead. Three relation reads per room become two. SetSpaceDesignFlowRate is a write path and was excluded from the earlier indexing for that reason. GetObject<Space>(guid) is not a snapshot - it reads the live object dictionary and sees every write before it - so two O(model) walks per write become one O(1) lookup, and the requirement is read through the shared already-resolved reader over the instance just resolved. No engineering result moves. Equivalence is asserted by reference over every space and zone of 100/500/1000/5000-space models, and the scaling tests assert allocation growth, never milliseconds. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
perf(analytical): find the same object without walking the whole model
… a second isolation still says so Part O isolation can be asked to run over a model that already carries its own cuts - a re-prepared run, or a saved isolated model handed back to Prepare & Run. Almost all of it was already re-entrant: the panels, their guids, the geometry, the shading context, the carried plant and the space identities all come back unchanged round after round, and narrowing the scope on a derived model reaches exactly the model that scope reaches from the whole building. Two things were not. An OPEN interface between a dwelling and an excluded space was not cut at all. AdjacencyCluster.Filter left a PanelType.Air panel on the cut exactly as it was, so it reached TAS with a single adjacent space, and Query.Adiabatic reports an air panel as not adiabatic whatever the flag says - UpdateAdiabatic never nulled its link and the opening simulated as a hole onto nothing. It also did not survive a second isolation: by then the panel is external on both sides of the adjacency comparison, nothing can tell it from a surface that was always a boundary to the outside, and it was silently re-typed into a genuine external wall with a solid default construction. The flat gained an outside wall where the corridor opening used to be, on the second run and not the first. It is now re-typed from its normal and marked adiabatic, like every other cut. And the isolation record understated itself. The cut is found by comparing the two adjacency states, which is right and must stay that way - the Adiabatic flag is set by a TBD import, a gbXML import and by people, so reading it would take an authored adiabatic wall for an interface to a space it does not touch. But on an already isolated model that comparison finds nothing, and the run disclosed "0 interface(s) to excluded spaces treated as adiabatic" over a model carrying them, in the evidence for a Part O submission. Filter now records WHY a panel is adiabatic in PanelParameter.IsolationCut when it makes the cut, and only then; the count reads that for cuts it did not make itself, so the second run states what the model carries without ever mistaking an authored surface for one. Nothing is suppressed: the second isolation does the same work over the same question and arrives at the same model. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…son added before the wires go back on The lifecycle for updating a component was AddObject, RestoreConnections, CopyPersistentData. ExpandVariableParameters was written but never called by anything - it had no callers at all - and it is the step that matters for the components most recently modernised to GH_SAMVariableOutputParameterComponent. Such a component declares its whole parameter set in Inputs/Outputs and registers only the ones marked Default. Everything else is on somebody's component because they inserted it. So the replacement built by the updater starts with the defaults and nothing else, and a voluntary output they had wired up has nowhere to land: RestoreConnections matches by name and quite rightly refuses to guess, so the wire was reported dropped and they were told to reconnect it by hand. A voluntary input lost both its sources and the value typed into it, because CopyPersistentData had no parameter to copy onto either. ExpandVariableParameters is now called between AddObject and RestoreConnections, and rewritten around the declaration rather than around guesswork. It walks the declared template in order and registers, at its declared position, every parameter the replacement is missing and the old component had. Identity is the NAME against the template - the same authority CanInsertParameter, CreateParameter and RestoreConnections already use - so nothing reads an index off the old component: a parameter added to the middle of a declaration between two versions would make every index after it name a different thing, which is exactly the silent mis-reconnection worth avoiding. The old body created parameters at every index and kept whichever happened to match by name, appending them after the last one; that is gone. It adds only what the declaration knows about. A parameter the new version genuinely no longer declares is still reported as a dropped wire, because fabricating a parameter to hang a wire on would be worse than losing the wire. It removes nothing either. Component guids are untouched, and a fixed-output component takes the early return and updates exactly as before. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…le stops being folklore Whether a run reopened from a saved model may be resumed into Iteration 2B has been an open question with a plausible story attached and nothing executable behind it. These tests put the fact under it. The model stores a ventilation unit's IDENTITY and never its capability - deliberately, because capability belongs to whoever ships the catalogue and a capacity copied onto the model would sit beside the design duty and the Approved Document F requirement as a second answer. So every capacity question is answered by looking the stored identity up in a catalogue handed in at the time, and the consequence is the one worth pinning: the same design, the same selected product and the same proposed round are ADOPTED against one rating of that product and REFUSED against another. The stopping point of an optimisation is a property of the catalogue it was checked against, not only of the design. Also pinned: a refused round changes nothing, so a resumption checked against the wrong catalogue does not corrupt a model - it stops in the wrong place, and hands back a design that looks like an answer. A catalogue that no longer offers the selected product, and one that contradicts itself about it, both leave the capacity unknown and refuse rather than pass, which is what makes an absence of evidence safe. And the unit itself carries no capacity under any catalogue, asserted rather than assumed. No production code changes. Nothing here writes a capacity into a design airflow, a requirement or an operating airflow. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ult construction library is configured CI caught what this machine could not. Re-typing an air boundary reaches for Query.DefaultConstruction, which reads the default construction library out of the active setting - and that answers null wherever the library is not configured: a CI runner, a test host, a fresh machine. Assigning it replaced the panel's own construction with NOTHING, so the surface reached the simulation with no fabric at all, and the behaviour depended on which machine ran it. It is not new here - the branch that re-types a genuinely external air boundary has always done it - but this change extends that path to the isolation cut, so it now matters on every isolated model with an open interface to an excluded space. Only a real construction replaces one now. Where none is available the panel keeps its own, is still re-typed from its normal, and is still marked adiabatic where it is the cut - which is the part that decides the thermal boundary. The regression asserts the invariant rather than a construction name, since the two environments legitimately give different ones and only "it has a fabric, and the same one both rounds" is common to both. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
fix(analytical): an air boundary on the isolation cut IS the cut, and a second isolation still says so
test(analytical): pin what the catalogue decides, so the 2B resume rule stops being folklore
AdjacencyCluster.Filter is a public API and SAMAnalytical.FilterBySpaces calls it on its own. It makes the cut and marks it, but strips nothing, because taking a person's openings off a surface is not what a geometric filter is for - that is the isolation's decision. So a model arriving from that component carries cuts that still have their doors, and on the adjacency question IsolateSpaces asks they cannot be found: they are external on both sides of it. The carried-cut branch recognised such a panel, counted it in the disclosure and then skipped the aperture cleanup on the assumption its openings were already gone. That was the worst of the three possible outcomes. The run would have stated the interface was treated as adiabatic while the derived model still opened onto the omitted space, and the conversion would have exported that door as an opening to outside - the fabricated boundary the cleanup exists to prevent, now with a disclosure saying otherwise. Reproduced on the FilterBySpaces path before changing anything. A carried cut now gets exactly the same treatment as a fresh one, which is also simpler than distinguishing them: where its apertures have already gone, the work finds none. What must NOT be read is still not read - PanelParameter.Adiabatic is set by a TBD import, a gbXML import and by people, and an authored adiabatic wall keeps every opening it is entitled to. Also: TheModelStoresTheProductIdentityAndNoCapacity proved a null capacity by handing the lookup a null catalogue, which that lookup answers null to whatever the model stores - it would have kept passing if capacity ever began to be persisted on the unit. It now reads the unit's own serialized state, which is the premise the catalogue decision actually rests on. And PROJECT_PROGRESS.md is brought up to date as AGENTS.md requires: what is merged onto sow/2026-Q3 and under which commits, the validation, SAM #103 held pending a Rhino-enabled run of its nine regressions, and the exact next step. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
fix(analytical): a cut this run did not make still loses its openings
…stead of throwing
Query.UpdateConstructionLayersByPanelType runs on every guided Part O run,
on the prepared model, immediately before conversion. It crashed on an
ordinary internal door.
ConstructionLibrary.GetConstructions and
ApertureConstructionLibrary.GetApertureConstructions answer NULL rather
than an empty list when they hold no candidate for a type, and
FirstOrDefault over null is an ArgumentNullException; where the library
itself was not configured it was a NullReferenceException one line further
on, from dereferencing it. The reachable case is not exotic: an aperture
construction with no pane layers enters the fill branch, and that is
exactly what a Part F door is, on a model whose panels are otherwise fully
layered. Reproduced both ways before changing anything.
Neither is an error. A library with nothing for this type and no library at
all are the same answer - there is nothing to fill from - so both are now
simply no candidate, and the existing construction is kept. Nothing is
replaced by a default because a library came up empty.
But it must not be silent either, and that is the second half of this. An
unresolved surface does not fail later, it changes the thermal case without
saying so, and differently for each kind:
- a panel whose construction has no layers converts to a TBD construction
with no materials, and Query.Adiabatic reports a zero thickness
construction as adiabatic in its own right, so SAM_Tas UpdateAdiabatic
nulls the surface's link and the wall is simulated as an adiabatic
boundary nobody asked for;
- an aperture with no pane layers has a pane thickness of zero, and the
conversion creates a pane surface only above zero thickness, so the
opening is not represented in the TBD at all. A door the model says is
there does not exist in the simulation.
So an overload reports what it could not resolve, naming each panel and
aperture, and the existing signature delegates to it. That is what lets a
caller - a Part O run above all - refuse rather than simulate a case it did
not intend. Nothing is made adiabatic here to keep a run going.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…nostics fix(analytical): the fabric fill states what it could not resolve, instead of throwing
…t has a known bug, SAM_UI side not started Branch: fix/design-transfer-air-authority (off sow/2026-Q3). NOT a PR yet - this is a mid-investigation WIP commit for continuity across machines. Do not merge, do not open a PR from this commit as-is. Context: native Iteration 1a acceptance on SAM_UI PR #93 found that the Ventilation Design floor-plan overlay's transfer-air tag ("TRA 63.0 l/s ?") stays at Part F's Table 1.2 baseline even when a room's DESIGN supply/extract has been raised well above it by a Part O round (e.g. 150.0 l/s). Investigated with a research agent first; findings below. ROOT CAUSE (confirmed by investigation, not yet by a passing test - see "known bug"): There are TWO independent transfer-air solvers, both built on the same PartFAirflowNetwork graph/node-balance engine, fed from DIFFERENT authorities: - PartFCalculator.CalculateTransferAir -> PartFVentilationTerminalRequirement (Part F Table 1.2 sizing) -> PartFComplianceResult.TransferPaths -> the "TRA 63.0 l/s ?" UI tag. Never changes with Part O. - Modify.AddPartFTransferAirMovements -> Query.VentilationTerminalDesignDuty_Lps (the DESIGN authority, VentilationTerminal.DesignFlowRate_Lps) -> SpaceAirMovement objects -> SAM_Tas export (UpdateIZAMs reads SpaceAirMovement exclusively). ALREADY rebuilt every Part O round, ALREADY design-driven, ALREADY node-balance-correct. So: the design-driven, node-balance-respecting transfer solve already exists and already feeds TAS correctly. The bug is that SAM_UI's floor-plan overlay never reads it - only Part F's own transfer figure is displayed. This is primarily a UI-authority gap, not a SAM domain-network defect. Opening-provenance ("No modelled transfer opening identified") is confirmed orthogonal to flow magnitude - it is purely geometric (shared Panel + Door Aperture, or a recorded free area), so it will not change no matter which flow value is displayed. WHAT'S DONE: - New Query.DesignTransferSpaceAirMovements(AdjacencyCluster) / DesignTransferFlowRate_Lps(...) in SAM.Analytical/Query/DesignTransferAirMovement.cs - a READ-ONLY aggregation over the ALREADY-COMPUTED, ALREADY-PERSISTED SpaceAirMovement objects AddPartFTransferAirMovements writes every Part O round. No new solve, no new SAM engineering calculation - "prefer existing SAM abstractions" per the task brief. Builds clean, 0 errors. - New SAM.Tests/PartFDesignTransferDivergenceTests.cs - extends the existing PartFTransferAirRealizationTests.cs star-topology fixture with a case that raises one room's design supply above its Part F baseline via Modify.SetSpaceDesignFlowRate, re-prepares, and checks: transfer air follows the raised duty, every node stays balanced, Part F's OWN requirement (a fresh PartFCalculator.Calculate() over the same model) is completely unchanged, and re-preparation stays idempotent at the new magnitude. KNOWN BUG (in the TEST, not in production code - found and root-caused, not yet fixed): 3 of 5 new tests fail. Root cause identified: AnalyticalModel.AdjacencyCluster's getter returns "a fresh copy on every read" (its own doc-comment says so, at AnalyticalModel.cs:277-286, and PreparePartOIteration.cs:334-336 explicitly warns "reading it twice would silently discard the first half of this"). My test called `analyticalModel.AdjacencyCluster.SetSpaceDesignFlowRate(space, ...)` - which mutates a THROWAWAY COPY that is never assigned back to analyticalModel, so the subsequent re-preparation still saw the OLD Part F baseline duty. Confirmed by the failure output itself: "Actual: 6.678261" in every failing assertion is exactly the untouched Part F baseline, and (raised value) - 6.678261 exactly equals the l/s I added in each test (87 / 40 / 50). THE FIX, not yet applied: get the cluster ONCE (`AdjacencyCluster adjacencyCluster = analyticalModel.AdjacencyCluster;`), call SetSpaceDesignFlowRate on THAT local variable, then rebuild the AnalyticalModel from it before re-preparing - `new AnalyticalModel(analyticalModel, adjacencyCluster)` - mirroring the exact pattern the existing PartFTransferAirRealizationTests.cs Model() fixture already uses (`return new AnalyticalModel(analyticalModel, adjacencyCluster_Sized);`). Apply this in all three failing tests' bodies (RaisedDesignSupply_RoutesTheRaisedDutyThroughTheDwelling_NodesStayBalanced, DesignTransferFlowRate_Lps_AgreesWithTheRawMovementsAfterARaise, PreparingTwiceAtTheRaisedDuty_ReplacesRatherThanAdds), then re-run `dotnet test SAM/SAM.Tests/SAM.Tests.csproj -c Release --no-build --filter "FullyQualifiedName~PartFDesignTransferDivergenceTests"`. The other 2 tests already pass (DesignTransferFlowRate_Lps_IsNull_WhereNoMovementConnectsTheTwoSpaces, RaisedDesignSupply_LeavesThePartFRequirement_CompletelyUnchanged - the latter passes only because it independently re-runs PartFCalculator rather than reading the mutated-then-discarded cluster). NOT STARTED YET (SAM_UI side, per the task's Task 4): - DesignAirFlowMarkType.Transfer, DesignAirFlowOverlayMark transfer fields (mirroring PartFOverlayMark's Start/End/Direction/ApertureGuid/OpeningStatus/Caption shape) - DesignAirFlowFloorPlanOverlay building Transfer marks from the new SAM query - DesignAirFlowRenderer drawing them through the shared PartFTagPlacement solver - DesignAirFlowViewSettings.ShowTransfer + dialog checkbox - Widening PartFFloorPlanOverlay's private OpeningPoint2D/AperturePoint2D/PartitionPoint2D to internal static for geometry reuse (and fixing PartitionPoint2D's own pre-existing GetSpaces()?.Find(...) O(n) lookup to GetObject<Space> while touching that area - flagged by the same reviewer instruction that fixed DesignAirFlowRenderer.LimitArea in PR #93) - All SAM_UI-side tests from the task's Task 6 (items 4, 8, 9, 10, 11, 12) - TAS consistency re-verification against a real saved .sam (the investigation already found SAM_Tas reads SpaceAirMovement exclusively and never touches PartFDoorTransferData/TransferPaths, so no SAM_Tas change is expected, but this should be confirmed against a real OptMax .sam once the UI side exists to inspect it with) DO NOT MERGE. DO NOT open a PR from this branch yet - it does not build a complete feature and the new test file has known failures.
… can actually carry Two separate reasons the raised-duty tests were measuring nothing. They wrote SetSpaceDesignFlowRate straight onto analyticalModel.AdjacencyCluster, which returns a fresh copy on every read - so the raise landed on a throwaway that was never put back, and every assertion quietly re-measured the Approved Document F baseline it had started from. Modify.PrepareBaseMVHR already carries the same warning against exactly this. The cluster is now taken once, written once and put back once. With the raise actually reaching the model, the preparation refused it, and rightly: SetSpaceDesignFlowRate is a primitive that writes the terminal it is told to and rebalances nothing, so a supply-only raise leaves the dwelling gaining air it never loses - the state PartOVentilationUnitSelectionTests.ASupplyOnlyDesignChange_Refuses- RatherThanUnbalancingTheDwelling already pins as invalid. The raise now goes through Modify.ApplyTargetedDesignAirFlow, the Approved Document O transaction that derives the matching extract in one all-or-nothing step, so what the tests measure is a raised design rather than a broken one. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…t a whole-model scan per route Query.DesignTransferSpaceAirMovements and DesignTransferFlowRate_Lps resolved each SpaceAirMovement endpoint through AdjacencyCluster.AirMovementEndpoint, which scans every object of the endpoint's runtime type per call. A caller resolving many routes (the Ventilation Design floor-plan overlay, one lookup per adjacency) paid that scan again for every route, making the whole read quadratic in the number of spaces - independently measured at ~22.7 MB/500 spaces up to ~1.32 GB/4000. Spaces are now indexed once into a Dictionary<Guid, Space>, and the resolved from/to guids are carried on the new DesignTransferAirMovement result so DesignTransferFlowRate_Lps never re-resolves them. O(spaces + movements) end to end. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Design transfer air, readable and separate from Approved Document F's
…from terminal duties to both authorities Every prior transfer-air fixture (PartFTransferAirRealizationTests, PartFTransferAirDwellingScopeTests, and the SAM_UI overlay/renderer tests) wires its rooms through DwellingPartitions.Star or hand-authors a SpaceAirMovement/PartFDoorTransferData directly - nothing exercised a genuine two-hop chain (A supply -> B pass-through -> C extract), and nothing started from terminal duties and let the production solve derive Upstream/Downstream and From/To itself. Adds that fixture and pins, from production Modify.AddPartFTransferAirMovements and PartFCalculator.Calculate alone: Part F's Upstream/Downstream, Design's From/To, and that the two independent authorities agree on physical direction even though they may disagree on rate. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
… prove Hall is a genuine pass-through The class doc overstated the native acceptance finding as a proven production direction defect. It was a concern raised by an invalid throwaway fixture, not evidence either way - reword accordingly. Bedroom 1 was not actually a pass-through room under Approved Document F: a bedroom is habitable and gets its own supply requirement, same as Living Room. Renamed the middle room to Hall (Category: Transfer, IsTerminalSpace: false per the shipped rule set) and added assertions, in the two tests that already run the production calculation, that Hall carries neither a Part F terminal requirement nor a Design supply/extract terminal - proving the pass-through claim from calculated state instead of assuming it from a room name. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
test: pin transfer-air direction across a genuine chain
SAM.Core.Grasshopper.Tests converted from xUnit to NUnit + McNeel's Rhino.Testing, which hosts a real headless Rhino 8 + Grasshopper in-process (RhinoCore, configured by Rhino.Testing.Configs.xml). The previous xUnit + hand-rolled NuGet-cache assembly resolver never initialized Rhino's native runtime, so GH_Document tests hung instead of skipping under dotnet test. Every test method body is unchanged: XunitCompatAssert.cs is an xUnit-shaped Assert/Skip surface backed by NUnit, living in the same namespace as the tests so unqualified Assert/Skip calls resolve there instead of NUnit.Framework.Assert - only usings and [Fact]/ [SkippableFact] -> [Test] attributes changed. Test-fixture helper classes that derive from Grasshopper base types (TestUpdatableComponent, TestVariableOutputComponent, ...) moved to a new sibling project, SAM.Core.Grasshopper.Tests.Fixtures. NUnit's test discovery enumerates every type in the discovered assembly before any [SetUpFixture] runs, so those classes needed Grasshopper.dll resolvable just to be discovered - before Rhino.Testing ever got a chance to host it. Kept out of the discovered assembly, they're only touched from inside a running [Test], after Rhino is up. All 44 tests pass for real now (0 skipped), including the 9 VariableOutputUpdateTests and 11 ManualReconnectionTests that always skipped before. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…tData overload
GH_PersistentParam<T> declares both AddPersistentData(T) and
AddPersistentData(object), so the unqualified
type.GetMethod("AddPersistentData") reflection lookup in
CopyPersistentData was always ambiguous (AmbiguousMatchException) -
silently swallowed by the surrounding best-effort catch, so persistent
data was never actually copied onto a replacement parameter during a
component update.
Invisible until now because the tests that exercise this path
(DefaultInput_KeepsItsPersistentData,
VoluntaryInput_KeepsItsConnectionAndItsPersistentData) always skipped
under the previous, non-functional test harness. Resolving the
generic argument T and looking up GetMethod("AddPersistentData", new[]
{ gooType }) picks the single matching overload.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
… onto its defaults Codex P1 on this branch: a freshly-constructed replacement parameter can already carry its own baked-in default persistent data (e.g. SAMAnalyticalCreateCaseByApertureByAzimuths's "_ratios", SetPersistentData(0.15, 0.2, 0.25, 0.2) at registration). The previous fix resolved CopyPersistentData's reflection ambiguity by looping AddPersistentData(item) per item, which appends onto whatever the replacement started with - so a component with four baked-in defaults updated to a person's two authored values ended up with six (four defaults, then the two authored values), not two. GH_PersistentParam<T>.SetPersistentData(GH_Structure<T>) replaces the target's tree wholesale, regardless of what it started with, and hands over the old parameter's branch structure intact instead of flattening it through repeated single-item adds. New regression test (CopyPersistentDataReplacementTests.cs, and a TestDefaultPersistentDataComponent fixture with baked-in defaults) exercises exactly the scenario Codex described: it fails against the previous (append) fix and passes against this one. Also updates PROJECT_PROGRESS.md per AGENTS.md, recording this PR's current state, per a separate P1 review comment. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ss log Two Codex P1 findings on 9f2debd: 1. The Rhino-run result recorded "44 passed" - stale from before CopyPersistentDataReplacementTests was added in the same commit. The actual, re-confirmed result is 45/45, 0 skipped. 2. "Files changed" listed only this session's harness/fix files, not PR #103's original implementation (GH_SAMVariableOutputParameterComponent.cs, ExpandVariableParameters.cs, UpdateComponent.cs) - never previously recorded in this file. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…son added before the wires go back on fix(grasshopper): a replacement component gets the parameters the person added before the wires go back on
…tion 2B on a real product Tests only. No production code changed - SAM_Systems shipping a second real product needed nothing here, and these tests are the evidence for that claim rather than a consequence of it. SAM.Analytical does not reference SAM.Analytical.Systems and must not start, so the descriptors STATE the real identities and capacities (Nuaire/MRXBOXAB-ECO5-AECV/MR-ECO-COOL-V at 150/150, Nuaire/XBC15 at 190/190). That those figures are what the shipped catalogue says is SAM_Systems' own assertion. Both new fixtures also offer a 500 l/s product nothing may reach for, so a test proving nothing escalated is proving a choice rather than the absence of an alternative. PartOVentilationUnitSelectionTests, three tests: - TheRealLadder_EscalatesFromMRXBOXToXBC15_AndThenRefuses. One dwelling grown through the whole ladder on the model: 150 exactly on the MRXBOX's rating and re-selecting stays put; 160 exhausts it and escalates to the XBC15 with 30 l/s headroom; 190 exactly on the XBC15's rating, still not escalated; 191 refused, naming 190, with NOTHING written - the unit keeps the selection it had. Approved Document F never moved, asserted over the whole model. - ExplicitlySelectingXBC15_ChangesOnlyTheEquipmentFacts. The closeout's own question. Writing the XBC15's identity onto a unit whose automatic answer was the smaller MRXBOX moves the identity and the capacity that identity resolves to, and nothing else: every requirement, design airflow, air movement, runtime/profile airflow and the unit's own design duty are bit-identical. - TheXBC15Selection_StoresIdentityAndLooksTheCapacityUp. The model stores identity and no capacity. Offered a catalogue without the XBC15 the ceiling is UNKNOWN - not the MRXBOX's 150, not unlimited - and is reported as a refusal with a reason rather than as a pass. PartOCapacityEnvelopeTests, four tests. A new RealLadderFixture builds the Flat 1 shape at the figures native acceptance recorded once the MRXBOX's own envelope had been taken up - 150 supply / 82.5 + 67.5 extract, balanced at 150/150 - with the XBC15 DELIBERATELY selected and the MRXBOX offered first, so an envelope that re-ran the selection rule or took the head of the list would visibly swap the plant: - XBC15sRating_IsTheCeilingTheOneFiftyDesignGrowsWithin. Scale 190/150, giving 190 supply / 104.5 + 85.5 extract; 82.5/67.5 == 104.5/85.5, the proportions survive; duty 150 -> 190 with zero headroom left and not a thousandth past the rating; the 40 l/s spent as design airflow. - TheEnvelope_KeepsXBC15Selected_EvenThoughTheSmallerProductIsStillOffered. The premise asserted first, then the identity stable on both clusters and every dwelling round reporting Kept. - TheEnvelopeOverXBC15_MovesDesignAirflowAndNothingElse. Requirements unchanged (13 l/s beside a 190 l/s design), capacity present only on the descriptor handed in, no air movement touched, source design untouched by value and by reference. - ADesignAlreadyAtXBC15sRating_ReportsNoHeadroomRatherThanExceedingIt. NoHeadroom fires, no model produced, the XBC15 still selected, the 500 l/s product never reached for, the design not shrunk to create headroom to report. No capacity rule relaxed to make the diagnostic answer. Iteration 2B read equipment capacity and nothing else - a ceiling, through SelectedVentilationUnitCapacityDescriptor - so no architectural dependency on MRX cooling or performance data surfaced. That stays Iteration 3's question. PartOVentilationUnitSelectionTests 133/133 (was 130); PartOCapacityEnvelopeTests 38/38 (was 34); SAM.Tests 2024/2024 (was 2017); SAM.sln Release 0 errors; git diff --check clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ification (5ffe6a1) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…t was derived from Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…variant, Iteration 3 refuses a 2B round Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…ence-2026-10-01 feat(parto): PR-5 PartOBaselineReference - a saved result says what it was derived from
…ce2c05) Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
SimulationResultProvenance wrote Path_TSD (absolute) into saved .sam models. It now writes Locator_TSD relative to the model's folder; Path_TSD is a runtime-only hint. Legacy models still read and resolve. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…-relative-locator-2026-10-01 fix(provenance): persist a relative TSD locator, not the absolute Path_TSD
…onResultProvenance (ab2b3be) Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…turns false Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…-2026-10-01 fix(core): WaitToUnlock retry counter never advanced
…om upstream contribution Removes AGENTS.md, PROJECT_PROGRESS.md and internal evidence/.claude artefacts that are SAM-BIM workflow records, not upstream project documentation. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…r upstream Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
PR1: Persist explicit Part O cooling control room
…el (SAM_UI#58) Labels whose anchors pile up re-walked every ring of the pile with a full geometric test per candidate, and each test scanned a stack of nearby labels. Hundreds of clustered labels therefore spent the whole 500 000-unit work budget and were dropped untested at their anchors. Three exact changes; each skips only work whose outcome is already known, so every position is unchanged: - Same-ray skip: a candidate that deeply overlaps (both axes, margin MacroDistance) a placed axis-aligned rectangle that already blocked this item in one of the 8 directions is rejected without being built or tested. Bounded at 8 box comparisons per candidate. - Identical-search resume: items with an identical centred rectangle, search distances and limit area (by reference) test the identical candidate sequence; the placed set only grows and everything else is static, so a later item resumes after the prefix an earlier one already rejected. - Spatial index: built for every input size (the <=256 linear scan made small piles the slowest case), no one-cell halo (the expanded boxes already guarantee a shared cell), cells sized by the median short side instead of the longest side of the largest label, and outliers kept in an always-returned list instead of being spread over thousands of cells. WorkBudget, the mid-sweep check, the consecutive-unplaced backstop, the ShiftDistance guard and Fallback-at-anchor are unchanged. Measured, uncapped -> after (every result equal to the plain search): - 400 coincident identical labels: 620 263 -> 1 336 units - 400 coincident mixed-width labels: 3 011 406 -> 16 607 - 400 near-coincident: 3 789 652 -> 92 006 - 1 600 in a 4 m cluster: 11 425 434 -> 191 951 - healthy 5 000 / 2 000-label plans: 9 900 / 9 775 -> 5 000 / 2 000, same placement Tests: a differential oracle (a plain transcription of the previous search) over piles, near-coincident and clustered anchors, equal-but-separate limit areas, an unplaceable run, an outlier label, rotated labels, Mollier input and priorities; healthy-plan placement fingerprints recorded before the change; deterministic work bounds for the clustered cases, with no stopwatch. The budget test now uses a 4 m cluster, because its old input no longer reaches the budget. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…e search key Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… the grid comment Review follow-up: labels below the distance tolerance cannot size the spatial index, so Solve keeps the linear scan for them - now proven against the plain search too. The Solve comment named the query expansion as the tolerance; it is MacroDistance. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
perf(Solver2D): remove the clustered-anchor cost without moving a label (SAM_UI#58)
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ranch Internal progress record; not part of the upstream contribution. It remains in sow/2026-Q3 history and will be restored in sow/2026-Q4. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Reword reviewer/person provenance in SAM.Tests and SAM.Analytical comments (comment text only), drop PROJECT_PROGRESS.md closeout sentences and "Next step" stubs from PR-record docs, and rename review-round/handover headings in the Part O docs. Technical rationale is kept throughout. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
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.
2026-Q3 quarter-close promotion (Hoare Lea promotion pilot)
This PR promotes SAM-BIM's
sow/2026-Q3branch to Hoare Leamaster, per the standard SAM quarter-close release convention.Source (SAM-BIM
sow/2026-Q3):63dd763cb039ee32dde9f041354a676f9d304171Target (Hoare Lea
master):595f531e4608f3135f3dfb2743fa5bb2b3b48524Hoare Lea
masteris a clean ancestor ofsow/2026-Q3(358 commits ahead, no divergence, no unrelated-history issue).Q3 scope summary (from git history)
CI / checks
Combined check-runs on Q3 HEAD (
63dd763c): PASS —build (Release)success,test (Release)success.Notes
SAM,SAM_UI,SAM_Systems,SAM_Tas). Dependency/order note: no cross-repo ordering dependency for merging is required, but reviewers should be aware the other three pilot PRs land from the same Q3 cut.masterwith Hoare Leamasteris a later, separate step and is explicitly out of scope for this PR.🤖 Generated with Claude Code