[minor] Validate the ImGuiTableColumn layout before reading it, and add a test project

[matt-edmondson]

Part of #258 — the half that lives in this repository. Fixes #259. Does not file the upstream Hexa.NET.ImGui issue; #258 stays open to track that.

Scope note: this PR originally covered only the #258 guard. SonarCloud then failed the quality gate on 0.0% Coverage on New Code (required ≥ 80%) — a gate no change to this repository could pass, because there was no test project at all. That is #259, so fixing it here is the resolution rather than a widening. Details in the last section.

Part 1 — the layout guard (#258)

SaveColumnWidth reads column widths out of ImGui's native structs with manual pointer arithmetic, using sizeof(ImGuiTableColumn) plus a hardcoded 8 bytes to compensate for a Hexa.NET.ImGui binding bug. Two Debug.Assert calls guarded the assumptions behind that offset.

Debug.Assert is compiled out of a Release build — the only configuration users run. If the binding were fixed, or Dear ImGui reordered the struct, Release would have gone on adding 8 bytes to a stride that no longer needed it and read a float from the wrong address. Silently. The asserts only ever protected developers.

ProbeNativeImGuiTableColumnSize now runs once from a static initializer and returns a nullable stride. Null disables column-width persistence — which GetColumnWidth already handles by falling back to the saved or default width, so the app keeps working and merely stops tracking new widths. It fails closed instead of reading a wrong address.

It measures the bug rather than inferring it

A size threshold — which the old code used (sizeof < 112) — can only ever infer the bug; it cannot distinguish a fixed binding from one whose struct grew for an unrelated reason. Instead the probe sums the bytes the eight narrowly-bound index fields actually occupy:

Result	Meaning	Stride
8 bytes	binding still narrow	sizeof + 8
16 bytes	binding fixed, matches native	sizeof (and logs that the workaround can go)
anything else	unrecognised	null — disabled

A missing field, a moved WidthGiven, or a Marshal failure also disable the read.

This settled an inconsistency in the original numbers

The old code asserted sizeof < 112 on the grounds that native is "about 112 bytes", while adding 8 to a sizeof of 108 — implying 116. Those disagree, and nothing reconciled them. Measured against Hexa.NET.ImGui 2.2.9:

sizeof(ImGuiTableColumn) = 108
DisplayOrder             SByte   offset 86 (size 1)
IndexWithinEnabledSet    SByte   offset 87 (size 1)
PrevEnabledColumn        SByte   offset 88 (size 1)
NextEnabledColumn        SByte   offset 89 (size 1)
SortOrder                SByte   offset 90 (size 1)
DrawChannelCurrent       Byte    offset 91 (size 1)
DrawChannelFrozen        Byte    offset 92 (size 1)
DrawChannelUnfrozen      Byte    offset 93 (size 1)
OffsetOf(WidthGiven)     = 4

All eight are one byte, contiguous, exactly as the original comment described — so the bug is real and +8 is right; the "~112" figure was the loose part. The probe returns 116, the same stride the previous code computed, so behaviour is unchanged today. Only the failure modes change.

A new failure path I had to guard

The Marshal calls previously sat inside Debug.Assert, so they never executed in Release at all. Running them unconditionally introduces a path that did not exist before — from a static initializer, where an escaping exception becomes a TypeInitializationException that kills the class. They are wrapped in catch (ArgumentException) and degrade to null.

Part 2 — the test project (#259)

32 tests, net10.0, MSTest.Sdk. Most of this application cannot be unit tested (the UI needs a live ImGui context, the providers need credentials and network), so the tests target the provider-independent logic that decides what users see and how hard the APIs get hit.

DurationEstimatorTests — the estimator drives the Estimate and ETA columns and the adaptive polling interval in RunSync, so a mistake shows up both as wrong numbers on screen and as the wrong request rate against a rate-limited API. Covers the three-sample floor, the IQR outlier filter, exponential weighting toward recent runs, that failed/canceled/pending/ongoing runs are never sampled, branch-specific estimation and its fallback, determinism across repeated calls, and that the estimate stays inside its own sample range.

ColumnStrideTests — the Part 1 decision. To make it testable, the decision is extracted out of the probe into ResolveNativeColumnStride(int, int, int). The probe needs unsafe, reflection and a real Hexa.NET.ImGui type, none of which a test can vary; the decision is the part worth testing, and separating it is what makes the corrected-binding and unrecognised-layout branches reachable at all — neither can be produced with the binding currently referenced.

Verification

Builds clean in both Debug and Release (Release specifically, since that is where the old asserts vanished). 32/32 tests pass.

Mutation-checked, by substitution rather than deletion:

Mutation	Tests failed
stride resolver subtracts the adjustment instead of adding it	5
MinSamplesForEstimate lowered from 3 to 1	3
estimator samples every completed run, not only successes	2

Two mutations initially ran without effect, which is worth recording since it is exactly the trap the ktsu mutation guidance warns about:

• Replacing the adjustment with a bare sizeof left ImGuiTableColumnSizeDifference unused and failed the build before any test ran. The substitution has to keep the constant referenced.
• The success filter appears twice in DurationEstimator, so a whole-file replace asserting a single occurrence silently did nothing and reported a green 32/32. Only the EstimateDuration copy is the one under test.

The tree was restored from backups and re-verified after each run.

About the SonarCloud failure

The gate failed on two conditions. I could not read the detailed findings — sonarcloud.io is blocked by this sandbox's egress proxy — so:

• 0.0% Coverage on New Code (≥ 80%) — root-caused and fixed. No test project existed, so any code change failed this. Recent PRs are all dependabot bumps, which touch no C#, so the gate had simply never been exercised. Part 2 is the fix.
• C Security Rating on New Code (≥ A) — not resolved, and I want to flag it honestly rather than guess. Nothing in this PR introduces unsafe; the pointer arithmetic was already there and is required by the feature. What changed is that rewriting the surrounding lines re-attributes them as "new code", so a pre-existing hotspot is now measured against the new-code gate. Extracting ResolveNativeColumnStride shrinks the amount of new code inside the unsafe region, which may help. If it still fails, the finding needs a look on the SonarCloud dashboard — and if it is the unsafe block, it is a pre-existing property of this feature, not something this PR introduced.

Caveats

• The fixed-binding and unrecognised-layout branches of the probe are reasoned from the measured layout; only ResolveNativeColumnStride exercises them directly. The runtime read path needs a live ImGui context, which this sandbox has no display for, so a manual check that column widths still persist is worthwhile before merging.
• I could not run the repo's own analyzers locally: this sandbox's SDK is 10.0.111 (Roslyn 5.0.0.0) and ktsu.Sdk.Analyzers 2.28.0 requires 5.9.0.0, so CSC refuses it with CS9057. CI is the first place KTSU0001–0007 see this, and the new test project is the likeliest place for a KTSU0001 complaint.
• Removes the dead .../Hexa.NET.ImGui/issues/XXX placeholder, which read as though an upstream issue existed. None has been filed — Report the Hexa.NET.ImGui ImGuiTableColumn struct-layout bug upstream and track removing the workaround #258 tracks that, and I deliberately have not opened it, since that is outside this organization.

[sonarqubecloud]

Quality Gate failed

Failed conditions
62.1% Coverage on New Code (required ≥ 80%)
C Security Rating on New Code (required ≥ A)

See analysis details on SonarQube Cloud

Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE

[matt-edmondson]

SonarCloud is red and I've stopped pushing at it. Here's exactly what's blocking, one condition at a time.

Everything else is green — build, CodeQL, GitHub Advanced Security, and (newly) tests actually running on both Windows and Ubuntu. Before this PR the Test on ${{ matrix.os }} job was skipped, because there was no test project for the matrix to discover. That is the visible confirmation that #259 is fixed.

Condition 1 — coverage: real progress, but short

62.1% Coverage on New Code (required ≥ 80%) — up from 0.0%.

The test project (32 tests) covers DurationEstimator and ResolveNativeColumnStride. What remains uncovered is, as far as I can tell, all genuinely unreachable:

• The four Log.Warning / Log.Info branches inside ProbeNativeImGuiTableColumnSize. These fire only when the Hexa.NET.ImGui binding has a different struct layout — a corrected binding, a renamed field, a moved WidthGiven, or a Marshal failure. None can be produced against the referenced 2.2.9 package; the struct layout is a property of the assembly, not something a test can vary. Splitting the decision out into ResolveNativeColumnStride is what made those cases assertable at all, and they are — the tests cover every branch of the decision. What is not covered is the logging that follows it.
• SaveColumnWidth's body, which needs a live ImGui context with a real table.

I could squeeze the number up by moving the message selection into the pure function too, but that would be chasing the metric rather than improving anything, and it would not clear condition 2 regardless.

Condition 2 — security rating: I cannot diagnose this from here

C Security Rating on New Code (required ≥ A)

I have not been able to identify the finding, and I'd rather say so than guess. sonarcloud.io is blocked by this sandbox's network egress proxy, so I cannot open the dashboard or query the issues API, and SonarCloud left no inline annotations on the diff to read.

What I can say:

• Nothing in this PR introduces unsafe. The pointer arithmetic in SaveColumnWidth predates it and is required by the feature — reading a float out of ImGui's native table structs is the whole point. What changed is that rewriting the surrounding lines re-attributes them as new code, so a pre-existing property is now measured against the new-code gate.
• Extracting ResolveNativeColumnStride deliberately shrank the amount of new code sitting inside the unsafe region. The rating did not move, which weakens (but does not disprove) the theory that the unsafe block is what is being flagged.
• The rest of the new code is reflection over a struct, integer arithmetic, and logging. None of it touches a process, a path, a query, a network call, or a credential.

What I need: someone with dashboard access to open the analysis and say which rule fired and on which line. If it is the unsafe block, it is pre-existing and the sensible resolutions are to mark it reviewed as a Security Hotspot, or to accept it on this PR. If it is something I actually introduced, tell me the rule and I will fix it.

I am keeping this PR watched until it is green, merged or closed.

---

Generated by Claude Code