Phase 157 — swept TaQL-lite's LIKE/glob pattern matching and _riseset's circumpolar formula against source; no bug found
Read casa/Utilities/Regex.cc's fromPattern (glob) and fromSQLPattern (SQL LIKE) directly — the two functions _glob_regex/_sqlpattern_regex (Phase 24) had been implemented from general convention, never checked line-by-line against casacore's own source. Confirmed exact matches: fromSQLPattern's own comment ("AFAIK there are no special escape characters") matches this package's documented "no SQL escape char" choice; fromPattern's *→.*, ?→., [!...]/[^...] negation, and raw pass-through of bracket contents (no re-escaping inside [...]) all match _glob_regex exactly. One deliberate, correct divergence confirmed non-bug: this package's _TQL_RE_SPECIAL escape set additionally escapes (/)/\, which casacore's own (smaller) escape list doesn't — necessary and correct since this package targets Julia's PCRE-based Regex (where parens are metacharacters) rather than reproducing casacore's own regex engine's escaping bug-for-bug; the semantic behaviour (which strings match) is unaffected.
Also re-verified _riseset's (Phase 104/131) standard hour-angle formula — cos(H₀) = (sin(elev₀) − sin(lat)·sin(dec)) / (cos(lat)·cos(dec)) — against the textbook astronomical formula (e.g. Meeus, Astronomical Algorithms): exact match, and the circumpolar (c < -1, source never sets) / never-rises (c > 1) edge cases are handled correctly since cos(lat)/cos(dec) are always non-negative for any valid latitude/declination (no sign-flip edge case to miss). No bug found in either area; no production code changed.
Phase 158 — closed a real test-coverage gap: container_mmap's non-contiguous-block fallback had never actually been exercised
Following up on Phase 156's finding (a bug hiding in an optional-key lookup that no test happened to exercise), searched for other Dict/Record-style lookups fed by on-disk data across src/ — _engine_spec_from_source, the _MEAS_FRAMES/_ref_from_code family, and every other get(kw, ...) site in src/datamanagers/ virtual.jl already use safe get-with-default or raise a deliberate, correct ArgumentError for genuinely invalid data (not analogous to Phase 156's "casacore silently tolerates this, we didn't" case) — no new bug found there. Also independently re-derived mscal.stokes()'s per-code rescale factor() values (0.5 for codes RR..YY, √2/4 for RX..YL) directly against StokesConverter.cc's own Vector<Float> factor setup — exact match, confirming the _stokes_factor/cmat[o,j] = base[...] * factor(in)/factor(out) composition Phase 78 already had right.
The concrete finding: container_mmap's own header comment (src/datamanagers/container.jl) already documented, as a known risk since Phase 20, that its non-contiguous-block fallback path (a MultiFileContainer virtual file whose physical blocks aren't laid out sequentially — never produced by this package's own writer, which always allocates contiguously) had no test exercising it — searched test/container_tests.jl and confirmed: zero references to container_mmap at all, so BOTH branches of that function (the zero-copy mmap fast path AND the materializing fallback) were completely untested, not just the fallback. Added a direct unit test that fabricates a MultiFileContainer with one virtual file laid out contiguously (exercises the mmap/SubArray fast path) and one laid out deliberately out of order (exercises the fallback, container_read) — both produce the correct bytes, and the fallback result matches an independent container_read call exactly. Inspected the fallback code itself (container_read) during this work and confirmed it's correct by construction (walks blocknrs in given order with no contiguity assumption) — this was a coverage gap, not a live bug, but a real one worth closing given it's the exact kind of "never-executed branch" risk this session's own discipline exists to catch. No production code changed. Standalone container suite green (290/290, all passing including the new test).
Phase 159 — found LGROUP/CMB velocity frames DO have a real casatools oracle after all (Phase 88 was wrong that none existed); added the cross-check
Phase 88's own writeup said the VEL_LGROUP/VEL_CMB constants were "copied verbatim from MeasTable.cc (no casatools oracle for these two)" and left them self-round-trip-tested only. That assumption was never actually checked — me.listcodes(me.frequency()) (real casatools) shows LGROUP and CMB ARE valid me.measure(...) target codes, exactly like every other frequency/radial-velocity frame this package already cross-checks. Live-verified directly: converting a 100 GHz TOPO frequency and a 20 km/s LSRK radial velocity to LGROUP and CMB via real casatools matches this package's measconvert output to the SAME precision as the already-verified BARY/LSRD/GALACTO frames (~7.8e-10 relative for frequency, sub-mm/s for radial velocity) — expected, since the LGROUP/CMB step in this package's implementation is a pure constant-vector addition on top of the BARY hub, introducing no new ephemeris error beyond what BARY already carries. Added both to the permanent CASA-oracle fixture (test/measures_fixture.py) and cross-check test (test/measures_tests.jl), using the identical tolerance buckets as the frames they're structurally identical to. No production code changed — the implementation was already correct; this closes a real "we never actually checked" gap in test coverage, not a live bug. Standalone measures suite green (490/490, was 486 before the 4 new assertions).
Phase 160 — found and fixed a real gap: AZELSW/AZELSWGEO direction frames were entirely unsupported (silently fell back to OtherRef)
Continuing Phase 159's methodology (question an unverified assumption by actually calling me.listcodes()), ran it across every measure kind: me.listcodes(me.direction()), .position(), .epoch(), .doppler(), .radialvelocity(), .earthmagnetic(), .baseline(), .uvw(). All match this package's existing frame coverage exactly — except direction/baseline/uvw's code list includes AZELSW and AZELSWGEO alongside the already-supported AZEL/AZELGEO/AZELNE/ AZELNEGEO.
Read MDirection.h's own enum directly: AZELNE=AZEL and AZELNEGEO=AZELGEO are literal C++ enum ALIASES (same integer value — this package's existing "AZELNE" => AZEL mapping was already exactly right), but AZELSW/AZELSWGEO are separate, DISTINCT enum slots — a genuinely different "azimuth measured south-through-west" convention (vs. AZEL's north-through-east), used by some older telescope control systems. This package's _DIRECTION_FRAMES string→type map (src/measures/measinfo.jl) had no entry for either name at all, so a column or measconvert call naming AZELSW/AZELSWGEO would silently resolve to OtherRef{:AZELSW} (an unconvertible frame) instead of actually converting — note _DIRECTION_ENUM (the 0-based numeric-code fallback table, used for a bare VarRefCol integer code) already had both names in the right enum positions from the start, so only the named-frame lookup path was affected.
Read MCDirection.cc's AZEL_AZELSW/AZELSW_AZEL routes: both go through MeasMath::applyAZELtoAZELSW, which simply negates the direction's Cartesian x/y (z, i.e. elevation, unchanged) — equivalent to azimuth += 180° — and is its own inverse. Implemented as a thin wrapper around the existing AZEL/AZELGEO conversion machinery (ext/SOFAExt.jl): AZELSW/AZELSWGEO flip the azimuth by π on the way in and out, reusing every other conversion path unchanged. Added the two new RefFrame singleton types, wired them into _DIRECTION_FRAMES, _FRAME_STRING (write path), and _OBS_FRAMES (the solar-system-body topocentric-parallax dispatch list, since AZELSW/AZELSWGEO are observer frames exactly like AZEL/AZELGEO).
Live-verified directly against real casatools: for a fixed J2000 direction and frame, me.measure(d, 'azelsw')/'azelswgeo' match this package's measconvert output to the same ~7×10⁻⁷ rad residual already established for plain AZEL/AZELGEO (the standard SOFA-vs-casacore ephemeris/EOP difference) — confirming both the azimuth-flip relation and the underlying AZEL/AZELGEO machinery it reuses. Added a self-consistency unit test (the exact azimuth = AZEL's azimuth + π relationship, plus round-trip) and extended the permanent CASA-oracle fixture + cross-check test with both frames. docs/src/api-measures.md gains the two new exported names (keeps the checkdocs = :exported docs build clean). Standalone measures suite green (508/508, was 490 before the 18 new assertions); standalone query suite unaffected (1008/1008, unchanged).
Phase 161 — swept Record-keyword indexing for a Phase-156-style unguarded-access bug; confirmed the ISM bucket-relative-row-0 invariant is real (no bug found)
Continued sweeping for the class of bug Phase 156 found (BitFlagsEngine's mask-key lookup indexing a Record directly instead of guarding with haskey). Grepped every .keywords[...] / kw["..."] access across src/ and ext/ — every remaining unguarded index (_BaseMappedArrayEngine_Name in _engine_spec_from_source, mi["type"] in measinfo, kw["QuantumUnits"] in UnitfulExt) is a keyword every real engine/MEASINFO writer emits unconditionally as part of constructing that record in the first place — categorically different from BitFlagsEngine's ReadMaskKeys/ WriteMaskKeys, which are user-supplied lists that may legitimately name a key absent from a particular table's FLAGSETS. No missing-key crash risk found.
Redirected to a related but distinct question raised while reading src/datamanagers/incremental.jl's reader: _le_index (the bucket-relative-row lookup used by getcell/getcolumn) returns index 1 whenever the target row is before the first entry in that bucket's row index — silently falling back to that bucket's first stored value rather than correctly inheriting the previous bucket's last value. This would be a real bug if a valid on-disk ISM bucket could ever lack an entry at bucket-relative row 0. Read ~/Development/CASACORE/casacore/ tables/DataMan/ISMBucket.cc's getInterval directly: when the binary search finds no exact match and the target precedes every index entry (inx == 0), it unconditionally does inx-- on an unsigned uInt index with no underflow guard — which would wrap to a huge value and crash/corrupt on any bucket whose row index doesn't start at 0. This confirms "every bucket's row index starts at bucket-relative row 0" is a real invariant real casacore itself relies on for getInterval's own correctness (an unsigned-underflow landmine, not merely a convention this package's own writer happens to follow) — so _le_index's fallback-to-index-1 path is unreachable for any valid on-disk table, matching the existing Phase 8 plan note ("Every column has an entry at bucket-relative row 0") but now confirmed from the reader side too, not just the writer's own design choice.
No source or test change — this phase closes out two investigation leads with no bug found, continuing the established discipline of verifying an assumption against real casacore source before trusting it. Standalone suite unaffected.
Phase 162 — swept the multi-column tiled tie-break sort + Dysco/Stokes read paths once more; no new bug (one false alarm resolved)
Re-read _tile_order (src/datamanagers/tiled.jl) — its explicit by = i -> (-_canon(types[i]), -i) sort key initially looked backwards (a naive reading of "stable descending sort, ties keep binding order" from the Phase 11 plan text suggests ties should stay in ascending original-index order, which this key does not produce). Before "fixing" it, checked the existing test that specifically documents this case: test/tsm_multicol_tests.jl's "tile-block order — equal-size types (casacore tie-break)" testset's own comment states real casacore was found, during Phase 11's implementation, to order equal-canonical- size columns by descending binding index, not ascending — and that testset cross-checks both directions (casacore-authored → our reader, and our writer → casacore reader) against real CCT.Table. The current -i tie-break key produces exactly that descending-index order. So the Phase 11 plan's own prose summary ("ties keep binding order") was an imprecise gloss on what was actually verified live; the code and its real-oracle test already agree with each other and with real casacore. No bug — a false alarm caught before any code was touched, by checking the test before "fixing" anything.
Also re-verified, without finding an issue: _dysco_spec_from_source's antenna1/antenna2 row selection (inst.ant1[rows]) is correctly absolute-row-indexed since DyscoStMan.ant1/.ant2 are populated as full-table-length vectors at open time; mscal.stokes's Bool (FLAG) conversion path already matches casacore's any(coefficient≠0 && flag) per-output rule exactly (confirmed against the Phase 78/142 source citations already in the code); _mf_pack_index/_mf_unpack_index round-trip correctly for the empty- and single-block edge cases.
No source or test change. Standalone suite unaffected.
Phase 163 — found a real function-name mismatch: mscal.uvw_j2000() doesn't match real casacore's own spelling; added the correct alias
Read derivedmscal/DerivedMC/{Register,UDFMSCal}.cc directly (having already mined MSCalEngine.cc heavily in Phases 136-144) and found the real registered function name for "new uvw in J2000" is derivedmscal.UVWJ2000 — no underscore — matched case-insensitively via mscal being a genuine TaQL synonym for derivedmscal (tables/TaQL/TaQLStyle.cc's defineSynonym("mscal", "derivedmscal"), confirmed directly, closing a standing unstated assumption in this project's use of the mscal. prefix since Phase 77). This package spelled the function uvw_j2000 (Phase 79) without ever checking real casacore's own name for it — a query written against real casacore's mscal.uvwj2000() would have failed here with "unknown mscal function". Fixed by aliasing the real, underscore-free spelling onto the existing internal name in _make_func (src/taql/functions.jl) — both spellings now work identically, case-insensitively, live-verified against the sample MS (mscal.uvwj2000()/mscal.UVWJ2000() give byte-identical results to mscal.uvw_j2000()).
The same source read turned up two more findings, recorded but not acted on this phase: real casacore's derivedmscal library has no bare PA function at all (only PA1/PA2 are registered) — this package's suffix-less mscal.pa() (added for symmetry with ha/azel/ …) is a MeasurementSets-only extension with no real casacore counterpart, not a divergence from one, and none of the existing mscal.pa()-bare tests are real-TaQL cross-checks, so nothing needed fixing there. And real casacore has a whole family of wavelength-scaled uvw functions this package doesn't implement at all (UVWWVL/UVWWVLS/UVWJ2000WVL(S)/UVWAPP(WVL(S)) — the last also in the APP frame rather than J2000) — a genuine, real gap, left for a future phase.
Full standalone test/taql_mscal_tests.jl green (renders _HAVE_TAQL/ _HAVE_CASACORE stubbed false to run outside the dev machine's real- casacore setup — every one of its 500+ assertions, including the new Phase 163 parser-unit and query cross-check tests, passes with no regressions).
Phase 164 — swept derivedmscal's full registration table + MSCorrParse.cc directly; confirmed several existing findings with full certainty, no new bug
Read derivedmscal/DerivedMC/Register.cc's complete register_derivedmscal() function (every UDFBase::registerUDF call, not just the subset touched by Phases 77-163) and cross-checked it against this package's own _MSCAL_FUNCS/_MSCAL_DIR_FUNCS/_MSSEL_FUNCS name lists. No further naming mismatches beyond Phase 163's UVWJ2000 found; confirmed the Phase 163 findings independently from the registration table itself (no bare PA registration or help text anywhere; the wavelength-scaled uvw family — UVWWVL/UVWWVLS/UVWJ2000WVL(S)/UVWAPP(WVL(S)) — is real and genuinely unimplemented here).
Also read ms/MSSel/MSCorrParse.cc directly (the file mscal.corr() actually calls into) to close out the Phase 148 "not independently confirmed live" note. Confirmed precisely WHY that live confirmation has never been possible anywhere: UDFMSCal::makeCorr/makeFeed (derivedmscal/DerivedMC/UDFMSCal.cc) are real, working C++ factory functions, but Register.cc's register_derivedmscal() — the only place any derivedmscal.* name is ever wired to a factory — has no registerUDF call for either one, unlike every other selection type (BASELINE/TIME/SPW/UVDIST/FIELD/ARRAY/SCAN/STATE/OBS, all registered). This is a genuine, permanent dead-code path in upstream casacore itself, not an artifact of this environment's particular build — so the Phase 148 non-selectivity question about MSCorrParse::selectCorrType's unfiltered corrtype argument is untestable against real casacore anywhere Register.cc is used unmodified, not just here. Recorded as an addendum to the existing Phase 148/152 comments in src/taql/mscal.jl; no behaviour change — this package's own read-only mscal.corr()/mscal.feed() already stand on their own correctness, independent of what real casacore's (apparently non-selective, and separately confirmed to have a real MS-mutation side effect via a SELECTED_DATA column — Phase 143) own implementation does.
Comment-only change; no source/test behaviour affected.
Phase 165 — line-by-line re-verified Dysco's AFTimeBlockEncoder::fitToMaximum port (never independently re-checked since Phase 19); confirmed the frequency/wavelength unit grammars once more; no bug found
fitToMaximum was Phase 19's own explicitly-flagged "riskiest" numerical port (a greedy channel/antenna hill-climb over the quantizer's dynamic range) and had never been independently re-verified against source since. Read tables/Dysco/aftimeblockencoder.cc:100-263 directly, line by line, against src/datamanagers/dysco.jl's _af_fit_to_maximum!: the initial flat per-(channel,polarization) normalization pass, the per-channel "largest cross-correlation component" search (max(re,im,-re,-im), algebraically identical to casacore's max(max(re,im), -min(re,im))), the per-antenna maximum-component and hypothetical-increase computation, the antenna-vs-channel selection, and both stopping thresholds (1.01 for antenna scaling, 1.001 for channel scaling) all match exactly. One genuinely subtle behaviour was specifically checked and confirmed correct rather than assumed: changeAntennaFactor applies its scale factor twice to an autocorrelation row of the antenna being boosted (count = (a1==target) + (a2==target), both true for that antenna's own autocorrelation) — real casacore's own changeAntennaFactor (aftimeblockencoder.cc:81-98) does exactly the same (for repeat in 0..<count), confirming this is a faithful reproduction of a real (if easy to mistake for a bug) casacore quirk, not something introduced by the port.
Also re-verified two smaller items while in the area: mscal.spw's channel-frequency unit set (hz/khz/mhz/ghz/thz, Phase 151) still matches ms/MSSel/MSSpwGram.ll's FREQ token definition exactly (an optional case-insensitive k/m/g/t prefix + case-insensitive hz). And mscal.uvdist's wavelength-unit lexer (ms/MSSel/MSUvDistGram.ll's WAVELENGTHUNIT) turns out to be case-sensitive within the lambda/LAMBDA word itself (only those two exact spellings are valid, not a mixed-case Lambda), while this package lowercases every unit string before comparison — a benign over-permissiveness (accepts a spelling real casacore's stricter lexer would reject), not a correctness bug, in the same category as the already-documented >=/<= leniency on field/spw/state (Phase 147).
No source or test change. Standalone suite unaffected.
Phase 166 — swept TiledCellStMan's reader/writer indexing and update!'s masked-pair self-reference ordering; no bug found
TiledCellStMan (one hypercube per row, nrCube == nrrow) has the least test coverage of the three Tiled* wrappers and was flagged with a real risk in its own plan ("header size ∝ nrow — @warn only"). Traced write_tiledcellstman's per-row cube construction against _cube_for_row's :cell-kind read path (tsm.cubes[Int(rownr)], a direct 1-based index with no interval search, unlike TiledShapeStMan's row-map lookup) — the writer builds exactly one cube per row in row order and the reader indexes it the same way; consistent, no off-by-one found.
Also re-examined update!'s (D, M) = expr masked-pair write (Phase 59, reordered in Phase 149 so the mask entry evaluates before the data entry overwrites its inputs) for the specific case where expr references the target column itself (SET (D, M) = D + 1) — confirmed this is already handled correctly: the default mask (TQLMaskOf(_taqllite_parse(de, vn)), Phase 59/149) is pushed before the data entry precisely so it re-evaluates de against the original, pre-update cols snapshot rather than a value D's own write might already have clobbered — already documented in the existing Phase-149 comment in src/taql/commands.jl, now independently re-verified rather than taken on faith.
No source or test change. Standalone suite unaffected.
Phase 167 — found and fixed a real bug: write_concattable silently accepted an unpersisted (in-memory) part, corrupting the output table
Real bug found. write_concattable's part list is written by computing _strip_directory(p.path, dir) for each part — but a RefTable built purely in-memory by query() (never persisted to disk) has path == "" by design (Phase 22: "an in-memory, never- persisted query result can carry path=\"\" safely"). abspath("") resolves to pwd() (the current working directory) rather than erroring, so write_concattable(dst, [t1, query(t2, "...")]) used to succeed silently, writing a bogus part reference (the CWD) into the persisted table.dat — no error at write time. Only a subsequent readtable(dst) would fail, with a confusing SystemError: opening file "<cwd>/table.dat": No such file or directory — far from the actual mistake and easy to misdiagnose as a filesystem problem rather than a usage error.
Fixed with a new _ondisk_path dispatch in src/tables/table.jl: a plain Table/already-persisted ConcatTable always has a real path (both are only ever constructed from a real on-disk directory, confirmed structurally — ConcatTable(...) is called from exactly two places in src/, both given a genuine directory); a RefTable with an empty path now raises a clear ArgumentError naming the fix (persist it with write_reftable first); anything else (e.g. a GroupedTable from groupby/join, which has no .path field at all) raises an equally clear error rather than an opaque property-access failure. The validation runs before mkpath(dir), so a rejected call leaves no partial output directory behind.
Verified live: the exact scenario now errors immediately at the write_concattable call site instead of corrupting the output; persisting the RefTable first and retrying produces a correct, readable concatenation. write_reftable itself was independently confirmed NOT to have this bug — its own _flatten_to_root (Phase 132) always fully unwraps any RefTable chain down to a genuine Table/ ConcatTable root before touching .path, so it never reaches an empty-path object. New testset test/reftable_tests.jl "writeconcattable — non-on-disk part errors clearly (Phase 167)" (6 assertions): the RefTable-part and GroupedTable-part error cases, no partial directory on error, and the legitimate persist-then-concatenate path. Standalone `test/reftabletests.jl` green (all pre-existing testsets unaffected).
Phase 168 — audited every .path access site across src/ for a repeat of the Phase 167 bug; confirmed no other instances
Grepped every .path field access across src/ (excluding tests) after Phase 167's fix — a Table/RefTable/ConcatTable/GroupedTable type confusion where an in-memory (unpersisted) AbstractTable's .path silently resolves to a nonsensical value instead of erroring. Checked each site's actual type guarantee: Table.path is always structurally real (a Table object is only ever constructed by readtable from a genuine on-disk directory — confirmed for every internal reader/writer call site: datamanagers/{standard,incremental,tiled,dysco,forwardcol, container}.jl, resync.jl, edit.jl's EditTable.reader::Table field, concatedit.jl, refedit.jl); reference_copy and _cmd_path/update!/delete!/insert!/taql (taql/commands.jl) already require t isa Table before touching .path; edit(ct:: ConcatTable) already requires all(p -> p isa Table, ct.parts); edit(rt::RefTable) already requires rt.parent isa Table. The one place that read a part's .path with NO such guard was exactly the one Phase 167 fixed (write_concattable's part-name construction) — the readme-line construction in the same function reads p.path again further down, but by that point _ondisk_path.(parts) has already validated every part, so it's safe (confirmed by tracing the two lines' order, not merely by proximity).
One related, deliberately-safe (not a bug) case also confirmed: edit(rt::RefTable) rejects — rather than silently mishandling — a genuine on-disk RefTable-of-RefTable chain (rt.parent itself a RefTable, possible only via a hand-authored/real-casacore-written nested reference, since this package's own query() always flattens to a true Table root at construction time) with a clear error naming the type, instead of attempting anything with an object that isn't guaranteed a real path. A documented limitation, not a fix candidate.
No source or test change beyond Phase 167's own fix. Standalone suite unaffected.
Phase 169 — found and fixed a real bug: write_reftable also silently accepted a non-Table/RefTable/ConcatTable parent (the Phase 167 bug pattern, missed by Phase 168's own follow-up audit)
Real bug found — Phase 168's audit had a real gap. Phase 168 confirmed write_reftable was safe because its _flatten_to_root always fully unwraps any RefTable-of-RefTable chain down to a genuine on-disk Table/ConcatTable root before touching .path — true, but that check only covers the case where parent is already a RefTable to begin with. write_reftable(dir, parent::AbstractTable, rows; …) had no guard on parent's type at all: if parent is a GroupedTable (from groupby/join/a computed query select — no .path/.type/ .subtype/.readme fields, since _flatten_to_root's base case returns any non-RefTable input unchanged), the function used to throw a confusing KeyError: key "path" not found (from GroupedTable's property-routing getproperty trying to look up a column literally named "path") instead of a clear message — and, since mkpath(dir) ran before the failing access, left a half-created output directory behind.
Fixed with a single top-of-function guard in write_reftable (src/tables/table.jl): parent isa Union{Table,RefTable,ConcatTable}, matching the function's own documented contract, checked before mkpath(dir) runs. The existing _ondisk_path helper (Phase 167) is also applied to the flattened root as defense in depth. Live-verified: the exact GroupedTable-as-parent scenario now raises a clear ArgumentError with no directory left behind, while every legitimate case (a plain Table, and a chained RefTable parent) still works correctly.
New testset test/reftable_tests.jl "writereftable — non-Table/RefTable/ConcatTable parent errors clearly (Phase 169)" (4 assertions). Standalone `test/reftabletests.jland the fulltest/taqlquerytests.jlsuite both green (the latter exerciseswrite_reftabletransitively throughcopytable/SELECT … INTO`-style call paths) — no regressions.
Reinforces last batch's own methodology takeaway, with a twist: an "audit every other access site" follow-up (Phase 168) is valuable but not infallible — it correctly found the chained-RefTable case was safe, but didn't independently re-derive the full precondition (parent itself must already be one of the three supported kinds) before declaring the function safe. A targeted reproduction attempt (actually calling the function with a suspicious argument type) caught what a pure code-reading audit missed.
Phase 170 — applied Phase 169's "actually reproduce it" lesson to copytable/insert!/write_ms; confirmed no further instances of the pattern
Following directly from Phase 169's own reinforcement ("a code-reading audit can miss a precondition a direct reproduction catches"), rather than reasoning about copytable/Base.insert!/write_ms from source alone, actually called each with the two suspicious argument shapes that broke write_concattable/write_reftable: an in-memory (unpersisted, path == "") RefTable and a GroupedTable, in every position where a loosely-typed AbstractTable argument is accepted.
copytable(dst, rt::RefTable)withrt.path == ""— works correctly (its_copy_table(dir, rt::RefTable, ...)method never touchesrt.pathat all; it materialises through the flattened parentTableand column reads, which need no directory reference).copytable(dst, gt::GroupedTable)— has its own dedicated_copy_table(dst, gt::GroupedTable, ...)dispatch (Phase 30); works.insert!(target, source::GroupedTable)andinsert!(target, source::RefTable)with an empty-pathRefTable— both work; neither needssource.path(rows are pulled via the genericTables.jl/ column-read interface).write_ms/copymstake a typedms::MeasurementSet, andMeasurementSethas no public constructor that can wrap aGroupedTable(onlyreadtable'sTable/RefTable/ConcatTable) — confirmed by inspection, not independently reproducible as a call that type-checks in the first place._cmd_path(shared byupdate!/delete!/insert!'stargetargument andtaql) already requirest isa Table, consistently rejecting aGroupedTable/unpersisted-RefTabletargetbefore touching.pathanywhere.
All five reproductions confirmed already-correct behaviour — no new bug found. This closes out the "loosely-typed AbstractTable argument + persist function" sweep opened by Phases 167-169: write_concattable and write_reftable were the only two gaps, both now fixed.
No source or test change. Standalone suite unaffected.
Phase 171 — found and fixed a real gap: mscal.stokes() gave a raw MethodError on a masked-array argument instead of a clear error
Real bug found, via direct reproduction (continuing Phases 169-170's discipline of actually calling suspicious combinations, not just reading source). mscal.stokes() (Phase 78/109) predates the masked-array feature (Phase 60 — DATA[boolexpr] produces a TQLMArray, not a plain AbstractMatrix), and nobody had tried combining them: mscal.stokes(DATA[FLAG], 'I') fell through all three of _stokes_convert's AbstractMatrix{...} methods and threw a raw, uninformative MethodError naming three unrelated candidate methods.
Fixed with a dedicated _stokes_convert(::StokesSetup, ::TQLMArray) method (src/taql/mscal.jl) that raises a clear ArgumentError pointing at the workaround (convert first, then mask the result) rather than attempting to propagate a mask through the conversion — there is no unambiguous rule for that, since each output correlation is a linear combination of several inputs and masking one input doesn't obviously mask (or not mask) a given output. Live-verified: the masked call now errors clearly, the plain (unmasked) mscal.stokes(DATA, 'I') path is completely unaffected. New testset test/taql_mscal_tests.jl "mscal.stokes() vs a masked-array argument (Phase 171)" (2 assertions). Standalone mscal suite green (all 28 testsets, no regressions).
Phase 172 — swept CompressComplexSD's bit-packing and removecolumn!'s Hypercolumn-keyword handling; no bug found in either
Two source-reading investigations, following the same discipline as Phases 161-166 (re-verify a flagged-but-never-independently-checked risk, or a code path with no obvious test coverage).
1. CompressComplexSD bit-packing — flagged as a specific risk at Phase 19's own drafting time ("ported verbatim from CompressComplex.cc:740-846 + Casacore cross-check", never itself re-derived line-by-line in a later sweep). Read CompressComplex.cc's CompressComplexSD::scaleOnGet/scaleOnPut in full (this checkout's real path is tables/DataMan/CompressComplex.cc, not the tables/Dysco/ guess in the original plan) and compared every constant and branch against src/datamanagers/virtual.jl's _decode(::CompressComplexSD,...)/_encode(::CompressComplexSD,...): the even/odd LSB dispatch, the fullScale = scale/32768 / imagScale = scale*2 factors, the wrap-correction arithmetic shared with plain CompressComplex, and every clamp range (ENG_SD_EVEN_LO/HI = ∓32768·32768[-1], ENG_SD_REAL_MAX = 32767, ENG_SD_IMAG_LO/HI = -16384/16383) all match casacore's source exactly, including the <<1/+1 odd-flag bit convention. No divergence found — the existing Phase 19 CASA cross-check for this engine was already exercising correct code.
2. removecolumn!'s Hypercolumn-keyword handling — prompted by Phase 112's own _copy_table fix (_filter_hypercolumns, which drops a Hypercolumn_<name> private keyword on copy when a column it names is missing from the output) raising the question of whether edit()'s removecolumn! needed the same treatment for its own regenerated TableDesc (src/tables/edit.jl's _flush_regen, which reuses rd.desc.private verbatim with no filtering). Read TableDesc::removeColumn (tables/Tables/TableDesc.h:576) directly: it is a bare one-line pass-through to ColumnDescSet::remove, with no hypercolumn-declaration cleanup at all — real casacore itself leaves a stale Hypercolumn_<name> keyword referencing a since-removed column after a plain Table::removeColumn. So _flush_regen's verbatim-copy behaviour after removecolumn! matches real casacore exactly (both leave the same dangling declaration) — not a bug, and not a case Phase 112's _copy_table fix needs extending to (that fix addresses copytable's rename/drop-via-selection case, a genuinely different situation from edit()'s in-place column removal).
No production code changed either way; both are confirmed-correct findings, not fixes. Standalone engine + edit suites green (existing tests unaffected — no new test needed, since neither investigation produced a code path that wasn't already exercised).
Phase 173 — swept MultiFile's CRC32 + pack/unpack-index algorithms against casacore source; both confirmed correct, closing two previously-flagged uncertainties
Two more source-reading investigations, targeting the specific "never independently verified" language left in Phase 20/21's own Risks sections rather than a fresh area.
1. _mf_crc32 — Phase 20's own Risk (b) explicitly noted "no independent reference vector for casacore's nonstandard variant" (the CRC32 used for a MultiFile container's header integrity check, useCRC — never itself set by any real casacore write path, per Phase 21's own finding, so a bug here would be completely inert in practice, but worth closing anyway since the uncertainty was explicitly on record). Read casa/IO/MultiFile.cc's CRCTable() (the lookup-table construction, .cc:44-67) and MultiFile::calcCRC (.cc:694-719) in full and compared every step against src/datamanagers/container.jl's _MF_CRC_TABLE/_mf_crc32: the polynomial (0x04C11DB7, built MSB-first per byte), the custom crcinit = 0x46AF6449, the per-byte update (crc = ((crc<<8)|byte) ^ table[(crc>>24)&0xff]), the 4-round "augment with zero bytes" tail, and the final crc ^= 0xFFFFFFFF all match exactly, table-index off-by-one (0-based C++ vs 1-based Julia) correctly accounted for. (Noted in passing, not acted on: casacore's own MultiFile::writeHeader calls calcCRC on the same buffer twice in a row — .cc:245-246, crc = calcCRC(...); crc = calcCRC(...) — an apparent redundant/dead first call in casacore itself, harmless since the function is a pure, deterministic computation with no side effects.)
2. _mf_pack_index / _mf_unpack_index — Phase 21's own Risk (a) flagged the write-side run-length packer as "new, untested-against-a- real-fixture logic" (no real casacore-authored fixture in the test suite has ever needed more than one block per file, so the multi-run packing path was only ever exercised by this package's own round-trip tests). Read MultiFile::packIndex/unpackIndex (casa/IO/MultiFile.cc:734-786) directly and compared to _mf_pack_index/_mf_unpack_index step by step — the run-detection loop, the "count excludes the first block number" convention, and the trailing-run flush after the loop all match exactly. One structural question worth recording: _mf_unpack_index assumes every negative (run-length) entry immediately follows the positive value it extends — a real simplification versus casacore's own unpackIndex, which handles a fully general Vector<Int64> with no such assumption. Traced through packIndex's own emission logic and confirmed this is a safe simplification, not a latent bug: a negative entry is only ever emitted immediately after the positive that started the run it extends, and is always immediately followed by either the end of the list or the next run's positive start — packIndex can never itself emit two consecutive negatives, so any output it produces is unambiguously decodable by the simpler one-negative-per-positive scheme our unpacker implements.
No production code changed in either case — both close out an explicitly-recorded uncertainty with a real, line-by-line source comparison rather than leaving it as "should be fine." Standalone container test suite green (unaffected — no new test needed, since the existing round-trip + real-fixture tests were already exercising the confirmed-correct code).
Phase 174 — found and fixed a real bug: TaQL-lite's datetime() couldn't parse casacore's own dd-mm-yyyy dash-numeric date form
Real bug found via direct source reading + live reproduction. _tql_parse_datetime (Phase 69) covers ISO dates and dd-Mon-yyyy / ddMonyyyy (month-name) forms via a fixed Dates.DateFormat list, but casacore's real MVTime::read (casa/Quanta/MVTime.cc:465-497) also accepts a dash-numeric r-mm-dd form whose interpretation depends on the magnitude of the first number: r > 1000 means r is the year (yyyy-mm-dd, already covered); otherwise r is the DAY and the trailing number is the year (dd-mm-yyyy), with the same 2-digit-year expansion (<50 → +2000, <100 → +1900) as the month-name sibling format already implements. "12-02-2020" — a perfectly ordinary, common date string — used to throw "cannot parse datetime" outright. Live-verified against real casacore's own datetime() via tableCommand: "12-02-2020", "1-1-2020", "31-12-2020" and the 2-digit-year form "12-02-20" all resolve to 2020-02-12, matching the day/year-swap-plus-expansion rule exactly.
Fixing this surfaced a second, more subtle issue along the way: naively adding the new dash-numeric parser as a fallback, tried only after the existing _TQL_DT_FORMATS list, was not enough — Julia's Dates.DateFormat("yyyy-mm-dd") mis-parses "12-02-20" as year=12 (stopping at the first dash rather than requiring exactly 4 digits), succeeding with a garbage answer before the correct parser ever got a chance to run. Fixed by trying the new _tql_parse_dashnum_date (src/taql/functions.jl) first, ahead of the named-format list, for exactly the bare N-N-N shape; genuinely ISO strings (r > 1000) still resolve to the same correct value through the new parser's own branch, so no format-list matches are lost. Also extended the date/time separator set to / / - / (not just the ISO T) after confirming live that real casacore accepts all four (MVTime.cc:513, in.tSkipChar('/') || in.tSkipChar('-') || in.tSkipChar(' ')).
New testset "Phase 174 — dash-numeric dd-mm-yyyy date parsing" (13 assertions: ISO unaffected, day/year swap, 2-digit-year expansion, all four separators, an invalid-month/day still errors, and two query() row-selection checks) plus "Phase 174 — dash-numeric date, real-TaQL cross-check" (5 assertions, _HAVE_TAQL-gated, comparing this package's parse directly against real casacore's datetime() for five representative strings). Standalone taql_query_tests.jl green in full (every pre-existing testset in the file, including all of Phase 22-97's query-engine coverage, unaffected).
Phase 175 — found and fixed a real bug: hms()/dms() produced an invented output format that never matched real casacore at all
Real bug found via direct source reading + live reproduction, continuing straight on from Phase 174's date-parsing fix in the same file (src/taql/functions.jl). This package's _tql_hms/_tql_dms (Phase 69) had never actually been checked against real casacore's own hms()/dms() TaQL functions — Phase 69's own test only asserted a hand-invented format ("HH:MM:SS.sss" for hms, "+DD.MM.SS.sss" for dms, colon/dot separators throughout).
Read TableExprFuncNode::stringHMS/stringDMS (tables/TaQL/ExprFuncNode.cc:1315-1339, which format via MVAngle::print, casa/Quanta/MVAngle.cc:198-330) directly: real casacore's actual format is completely different — hms() produces "HHhMMmSS.sss" (letter separators h/m, no colons, and critically no leading sign at all — MVAngle::print only emits a sign character for the ANGLE branch or the DIG2 modifier, neither of which the TIME-type hms() sets), and dms() produces "+DDDdMMmSS.sss" (letter separators d/m, an always- present sign, and a 3-digit zero-padded degree field, not 2 — stringDMS's underlying separator-replace loop stops after replacing exactly the first two . occurrences, leaving the seconds' own decimal point untouched). Live-verified against real casacore's hms()/ dms() via tableCommand for a spread of angles (quadrant boundaries, negative, zero, π, near-2π, an arbitrary value): every one of the old assertions was simply wrong output, not a rounding/precision nuance — this package's TaQL-lite hms/dms never once produced a string a real casacore user or a downstream tool expecting the real format could have used.
Fixed both functions to match the verified format exactly (still computing the same quantised-to-milliseconds-first integer arithmetic that avoids a stray 60 from float rounding — that numeric core was already correct, only the string assembly was wrong). Updated the Phase 69 test's two format assertions to the correct strings and added a new "Phase 175 — hms()/dms() output format" unit testset (7 assertions covering the no-sign-on-hms / always-signed-dms / 3-digit- degree-field distinctions) plus "Phase 175 — hms()/dms(), real-TaQL cross-check" (16 assertions across 8 angles, _HAVE_TAQL-gated). Standalone taql_query_tests.jl green in full.
Phase 176 — found and fixed two more real bugs: ctime() was missing fractional seconds, and ctod()/cdatetime() used the wrong date format entirely
Two more real bugs found, continuing directly from Phase 175's hms/dms fix by checking the neighbouring date/time-string functions in the same source file (src/taql/functions.jl) the same way: read casacore source first, then live-verify against real casacore.
Read TableExprFuncNode's cdateFUNC/ctimeFUNC/ctodFUNC dispatch (tables/TaQL/ExprFuncNode.cc:1045-1054) and the underlying stringDate/stringTime/stringDateTime (.cc:1218-1227, which format via MVTime::print, casa/Quanta/MVTime.cc:366-434):
ctime()callsstringTime(dt, 9)— precision 9, i.e. 3 fractional-second digits. This package's implementation was a bareDates.format(..., "HH:MM:SS")with no fractional part at all.ctod()and its aliascdatetime()(confirmed the same function in real casacore —TableParseFunc.cc:571maps both names toctodFUNC) callstringDateTime(dt, 9), which usesMVTime'sYMDprint mode ("YYYY/MM/DD/HH:MM:SS.sss", slash-separated, 4-digit year first) — a completely differentMVTimemode fromcdate()'s ownDMYmode ("DD-Mon-YYYY", dash-separated, 3-letter month name). This package'sctod/cdatetimewere instead built fromcdate's DMY format with a bare/HH:MM:SSsuffix tacked on (no fractional seconds either) — the wrong day/month/year ORDER and separator, not just missing decimals.
cdate(), cmonth(), and cdow() were independently re-checked against MVTime::monthName/dayName (casa/Quanta/MVTime.cc:100-154) and confirmed already correct — no change needed there.
Fixed by adding a shared _tql_time_of_day_str(mjd) (the same quantise-to-milliseconds-then-carry-safely integer arithmetic as Phase 175's _tql_hms, but operating on an MJD's fractional day directly rather than a radian angle, and using plain colon separators with no sign) and wiring it into ctime, and into ctod/cdatetime alongside a corrected "yyyy/mm/dd" date prefix. Live-verified against real casacore across five MJD values spanning a day boundary, a midnight, a fractional-second rounding case, and an ordinary date — every one of the six c* functions now matches exactly.
New testset "Phase 176 — ctime()/ctod()/cdatetime() output format" (6 assertions) plus "Phase 176 — ctime()/ctod()/cdatetime(), real-TaQL cross-check" (30 assertions across 5 MJDs × 6 functions, _HAVE_TAQL- gated, covering the already-correct cdate/cmonth/cdow too as a regression guard). Standalone taql_query_tests.jl green in full.