Repository navigation
Conversation
Grouping moves clashes from Results into Groups, but ExportClashBcf on documents and assemblies, the matrix's SaveBcf, SaveJson and NwcClashMatrixCell.Clashes read only Results, so grouped clashes were left out without notice, also through the CLI's --clash-group with --clash-bcf. The BCF paths now go through new NwcClashBcfExporter.Save/Write overloads for several tests, which write a topic per group and per ungrouped clash into one archive, like the existing single-test overload that now uses them. The matrix lists AllClashes. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
mahach666
left a comment
There was a problem hiding this comment.
Changes requested: the intended fix is useful, but the inline comment below describes a regression reproduced against the reviewed head and the unchanged base. Please address it and add the indicated regression test before merging.
Finding: Keep existing calls with an empty collection source-compatible.
| /// <summary> | ||
| /// Saves clash tests, including both standalone results and grouped clashes, to one BCF 2.1 archive file. | ||
| /// </summary> | ||
| public static void Save(string path, IEnumerable<NwcClashTest> tests, string projectName = "Navisworks Clash Coordination") |
There was a problem hiding this comment.
[P2] Keep existing calls with an empty collection source-compatible
Adding a second public collection overload makes this existing C# call ambiguous:
NwcClashBcfExporter.Save("empty.bcfzip", []);A separate consumer project compiles against the current main/base but fails against this PR with CS0121: IEnumerable<NwcClashResult> versus IEnumerable<NwcClashTest>. The new Write overload similarly breaks Write(stream, []).
Please use a distinct method name for exporting a collection of tests, or an internal helper for the document/assembly/matrix callers, so existing public calls continue to compile. Add a consumer compilation check covering the empty collection.
Fixes #17
Grouping moves clashes from
ResultsintoGroups, but several export paths read onlyResults, so grouped clashes were silently left out. Following the issue's suggestion:NwcClashBcfExportergetsSave(string, IEnumerable<NwcClashTest>, string)andWrite(Stream, IEnumerable<NwcClashTest>, string). They write every test into one archive: a topic per group and a topic per ungrouped clash. The existingWrite(Stream, NwcClashTest, string)now calls the newWritewith its one test, so its output is unchanged.NwcDocument.ExportClashBcf(both overloads),NwcAssembly.ExportClashBcfandNwcClashMatrix.SaveBcfpass their tests to these overloads. The CLI's--clash-bcfgoes throughExportClashBcf, so--clash-grouptogether with--clash-bcfnow exports every clash.NwcClashMatrixCell.ClashesreturnsExecutedTest.AllClashes, andSaveJsonlistsAllClashes, so theclashesarray agrees withclashesCount.Ungrouped tests export the same topics as before. I left
NwcGridSystemalone: it reads onlyResultswhen annotating grid locations, which is a separate behaviour.Tests (
ClashGroupingTests): each groups five clashes into two groups and one ungrouped clash.Document_ExportClashBcf_IncludesGroupedClasheschecks thatNwcDocument.ExportClashBcf(Stream)writes 3 topics.Assembly_And_Matrix_ExportClashBcf_IncludeGroupedClasheschecks the same throughNwcAssembly.ExportClashBcfandNwcClashMatrix.SaveBcf.Matrix_ClashesAndJson_IncludeGroupedClasheschecks thatClasheshas 5 items, and that theSaveJsoncell hasclashesCount5 and 5 entries inclashes.All three fail without the change (1 topic, 1 clash).
dotnet build NwcIO.slnxsucceeds, andNwcIO.Testspasses locally: 242 passed, and 2 skipped because they need private Navisworks files (.NET 10.0.400 on Windows, withglobal.jsonpinned down to it temporarily; not committed).🤖 Generated with Claude Code