ENH: set a flag when DICOM ambiguous ordering fallback is used

[seanm]

This flag can be used by callers to know the fallback happened, and warn end users.

PR Checklist

• No API changes were made (or the changes have been approved)
• No major design changes were made (or the changes have been approved)
• Added test (or behavior not changed)
• Updated API documentation (or API not changed)
• Added license to new files (if any)
• Added Python wrapping to new files (if any) as described in ITK Software Guide Section 9.5
• Added ITK examples for all new major features (if any)

Refer to the ITK Software Guide for
further development details if necessary.

[greptile-apps]

Greptile Summary

This change exposes whether GDCM series ordering used the legacy ambiguous-ordering fallback. Two correctness issues need attention before merge: the status can remain set after the same object subsequently orders a geometrically sortable series, and public mutators let application code overwrite a result that should describe the ordering operation.

T-Rex validation blocked

• Missing tool: The fallback-then-geometric-ordering C++ harness could not run because cmake is unavailable and the workspace has no configured ITK build tree or generated itkConfigure.h.
• Artifact metadata failure: The focused probe for the public mutators executed, but its uploaded artifact references omitted required labels. The execution evidence therefore cannot be published as a complete proof.

Confidence Score: 3/5

Not safe to merge until the per-ordering status is reset and the diagnostic result is made read-only to callers.

There are two independent non-security correctness issues: a stale status can misreport a later reliable ordering, and public setters can falsify the reported result while changing cache invalidation behavior.

Files Needing Attention: Modules/IO/GDCM/src/itkGDCMSeriesFileNames.cxx needs the status lifecycle corrected; Modules/IO/GDCM/include/itkGDCMSeriesFileNames.h needs the result API restricted to read-only access.

T-Rex Logs

What T-Rex did

• T-Rex attempted to configure the focused C++ harness but was blocked by missing cmake and a preconfigured ITK build, so the configuration could not start.
• A focused probe against the public mutator and MTime cache ran, but the uploaded evidence lacked required labels, so the execution record cannot be published as a complete proof.
• T-Rex produced proofs for two posted P1 findings, as referenced in the review comments.
• Validation identified a public API gap for DidUseAmbiguousOrdering and recommended a fix that uses only a getter and a private assignment path when fallback occurs.

_{Ran code and verified through T-Rex}

Comments Outside Diff (2)

1. General comment

   Ambiguous-ordering status can remain true after later successful geometric ordering

   • Bug
     • On a reused itk::GDCMSeriesFileNames object configured with FailOnAmbiguousOrdering=false, an initial duplicate-IPP series takes the legacy fallback and sets DidUseAmbiguousOrdering true. After SetInputDirectory selects a later geometrically sortable series, OrderSeries succeeds through gdcm::IPPSorter::Sort but does not clear the status, so callers observe a stale warning flag despite the most recent requested series having reliable geometric ordering.
   • Cause
     • m_DidUseAmbiguousOrdering is initialized false in itkGDCMSeriesFileNames.h:268 and assigned true only in itkGDCMSeriesFileNames.cxx:234-237. The success branch at itkGDCMSeriesFileNames.cxx:221-226 returns without assigning false. Cache invalidation by SetInputDirectory (:91-109) rebuilds the second directory but does not reset this member.
   • Fix
     • Reset m_DidUseAmbiguousOrdering for each new ordering request/cache rebuild, or explicitly set it to false before returning from a successful geometric sort. Add the captured focused regression harness as a test using duplicate-IPP files followed by a valid multi-slice geometric series on the same object.

   _{Ran code and verified through T-Rex}

2. General comment

   Public mutators let callers overwrite the ambiguous-ordering result and spuriously invalidate the cache

   • Bug
     • itkSetMacro(DidUseAmbiguousOrdering, bool) and itkBooleanMacro(DidUseAmbiguousOrdering) publish SetDidUseAmbiguousOrdering, DidUseAmbiguousOrderingOn, and DidUseAmbiguousOrderingOff. A caller can clear a true fallback result or set a false result to true. The generated setter calls Modified() when the value changes; BuildSeriesMap() reuses its cache only when its build time is newer than object MTime, so this result-only mutation unnecessarily invalidates the cache.
   • Cause
     • The PR treated an implementation-owned output/result flag as a configurable boolean property by applying ITK setter and boolean macros.
   • Fix
     • Expose only itkGetConstMacro(DidUseAmbiguousOrdering, bool). Keep the member private and update it internally when ambiguous fallback occurs; do not expose a public setter or On/Off methods.

   _{Ran code and verified through T-Rex}

_{Reviews (1): Last reviewed commit: "ENH: set a flag when DICOM ambiguous ord..." | Re-trigger Greptile}

[hjmjohnson]

One open question on the contract, not a defect in the current code: OrderSeries runs once per series, and m_DidUseAmbiguousOrdering is assigned (not accumulated) on each call, so after scanning a directory with several series the flag reflects only the last series ordered.

Two readings, both defensible — the doc comment ("If ambiguous ordering is encountered, this is set to true.") doesn't say which is intended:

• Per-series — "did the ordering I just performed fall back?" The current assignment is exactly right; the doc comment could say so explicitly (e.g. "reflects the most recent ordering operation").
• Per-scan — "did any series in this directory need the fallback?" That needs an OR-accumulate across OrderSeries calls plus a reset at the start of each scan; otherwise a caller checking the flag after GetFileNames() on a multi-series directory can silently miss a fallback that happened on an earlier series.

Worth pinning down in the Doxygen either way, since the flag exists to be read by callers deciding whether to warn.