Skip to content

Fix upstream questions: out-of-memory, dropped non-detects, rivers in the wrong basin, and a distance filter that did nothing - #55

Merged
prayaslashkari merged 14 commits into
developmentfrom
fix/fused-seed-order-and-obs-joins
Sep 20, 2026
Merged

prayaslashkari merged 14 commits into
developmentfrom
fix/fused-seed-order-and-obs-joins

Conversation

@prayaslashkari

@prayaslashkari prayaslashkari commented Sep 19, 2026 •

Copy link
Copy Markdown
Collaborator

All of this came out of one question: "What facilities are upstream from PFOS samples?" with block A left as plain Facilities (no filters, no region) and block C set to PFOS samples in York County, Maine.

That question did not work. It ran out of memory, and when a smaller version of it did run, it returned too few samples and drew rivers that have nothing to do with Maine.

Six bugs, in the order a user hits them. The first four are the report; sections 5 and 6 are what verifying the fix turned up.

Verifying section 6 turned up a seventh, one level further up: adding an industry filter to block A broke the question again, because a filter counted as "constrained" while meaning every facility in that industry nationwide. That is #58, which also corrects two claims in the docs below.


1. The query ran out of memory

The query always wrote block A first. For an upstream question block A is the facilities side, and here it had no filters and no region, so the query started from every facility in the graph and traced the national river network looking for samples in one Maine county.

QLever does not try every join order on a body this size, so it follows the order the query is written in.

Before (facilities first, nothing to narrow them):

?s2anchor rdf:type kwg-ont:S2Cell_Level13 .
?s2anchor kwg-ont:sfContains ?facilityA .          # every facility in the USA
?facilityA fio:ofIndustry ?industryCodeA .
...then trace the rivers, then look for Maine samples

After (the narrowed side leads):

?s2target rdf:type kwg-ont:S2Cell_Level13 .
?s2target spatial:connectedTo kwgr:administrativeRegion.USA.23031 .   # York County
?spC rdf:type coso:SamplePoint ; spatial:connectedTo ?s2target .
...then trace the rivers, then look for facilities

Same triples, same variables, different order.

The reorder only happens when one side is narrowed and the other is wide open. We measured the unconditional version: it rescued 5 shapes and broke 7, because "narrowed" does not mean "small" (a county of wells is narrowed and still huge). Distance-limited traces keep the old order too, because that shape was never measured reordered.

Before After
Find samples out of memory at 29s 200 rows, 14s
Find facilities out of memory at 26s 1,497 rows, 15s

Measured across all 72 hydrology shapes: 5 rescued, 0 regressions, 0 rows lost, faster in 50 of 66 runs.


2. Non-detects were thrown away even with the box ticked

In SPARQL, every triple you ask for is also a filter. If you ask for something a record does not have, that record disappears.

Ticking any sample filter pulled in the whole measurement record, including the unit. A non-detect has no unit recorded. So checking "PFOS" quietly deleted every non-detect, while "Include non-detects" sat there ticked.

Before (11 lines, to produce a list of sample IDs):

?observationC rdf:type coso:ContaminantObservation ;
    coso:observedAtSamplePoint ?spC ;
    coso:ofDSSToxSubstance ?substanceC ;
    coso:analyzedSample ?sampleC ;
    coso:hasResult ?resultC .
?sampleC coso:sampleOfMaterialType ?matTypeC .
?matTypeC rdfs:label ?matTypeLabelC .
?resultC coso:measurementUnit ?unitC .        # <-- deletes every non-detect
OPTIONAL { ?resultC qudt:quantityValue/qudt:numericValue ?numericResultC }
OPTIONAL { ?resultC qudt:quantityValue ?_qvC . ... }
BIND(COALESCE(...) AS ?result_valueC)

After (only what the filter actually reads):

?observationC coso:observedAtSamplePoint ?spC .
?observationC coso:ofDSSToxSubstance ?substanceC .
VALUES ?substanceC { <http://w3id.org/DSSTox/v1/DTXSID3031864> }

A concentration range still keeps the unit join, because that filter genuinely reads it.

Measured on Cumberland PFOS: 774 of 1,688 observations were being dropped. 23 sample points and 13 facilities became 32 and 15.

Nothing looked broken. You just got fewer dots on the map than the data supports.


3. Sample popups hid most of their rows

Same bug, different place. The popup query required the unit before it read the result, so it showed 474 of 3,334 observations. The unit lookup now happens after the result is read, with the symbol lookup nested inside, and costs what it did before.


4. The map drew rivers in the wrong basin

This is the one reported from outside: the map showed the entire Merrimack River for a question about York County.

The river layer is a separate query. It took the facilities we found, widened each one's cell, and followed the rivers downstream to the end of the network. Nothing in it mentioned the samples at all. So a facility near a drainage divide dragged in the whole neighbouring basin.

Before (one-sided, runs to the sea):

?upstream_flowline rdf:type hyf:HY_FlowPath ;
      spatial:connectedTo ?s2cellus ;              # cells near a found facility
      hyf:downstreamFlowPathTC ?flowline .         # ...and everything downstream, forever

After (must also reach a sample we actually found):

{ SELECT DISTINCT ?flowline WHERE {          # reachable FROM a facility
    ?_flSeed spatial:connectedTo ?s2cellus ;
             hyf:downstreamFlowPathTC ?flowline .
} }
{ SELECT DISTINCT ?flowline WHERE {          # ...and reaches a SAMPLE
    VALUES ?spC { ...the sample points step 1 resolved... }
    ?spC spatial:connectedTo ?s2celltarget .
    ?_flTarget spatial:connectedTo ?s2celltarget .
    ?flowline hyf:downstreamFlowPathTC ?_flTarget .
} }

The obvious one-line version, ?flowline hyf:downstreamFlowPathTC? ?_flTarget, leaves ?flowline unbound on the left and times out after 31s. Two intersected subqueries run in the same 7s the broken version took.

Stream Before After
Merrimack River 122 0
Presumpscot River 40 0
Suncook River 37 0
Winnipesaukee River (130km away) 21 0
Powwow River 16 0
Total flowlines 3,271 2,516

755 removed, 0 added. Nothing correct was lost. Of those 755, 684 are in a different basin and 71 are the stretch of river below a sample, which now stops at the sample instead of running on to the sea.

The distance-limited version got the same treatment. Worth noting it previously drew a Merrimack segment straight through its own 25km cap, so a distance limit was never a substitute for this.


5. Cleanup pass

A review of the above turned up one more live bug and some duplication.

"Is this side narrowed?" was written three times (scope.ts, sparqlErrors.ts, and a new copy here). The executor uses its copy to pick how to split a query, so the two drifting apart means it splits on a side the query already led with. Now one definition.

The new copy also had a flaw the original did not. It counted includeNondetects: true as a filter, but that is the default ticked state and narrows nothing, so a samples block with only that set was reordering the whole query for no reason:

Samples block Before After
"Include non-detects" ticked, nothing else reordered not reordered
"Include non-detects" unticked reordered reordered
A substance chosen reordered reordered

The river layer re-checked facilities it had already found. It pinned the resolved IRIs in a VALUES list and then re-applied the block's filters on top, which can only re-confirm what the list already says. For a samples anchor that was the whole measurement chain again, unit join included.

Also folded in: reuse of two existing helpers instead of new copies, a dead branch where both paths returned the same value, and one guard in the new check script that only covered half the cases it claimed to.

Verified after cleanup: steps 1 and 2 emit a byte-identical query, step 3 returns the same 2,516 flowlines.


6. The distance dropdown did nothing on this question

Picking any "within N km of flow" on the same question returned nothing. Every option, 5, 10 and 30 km, both projections: six queries, six identical failures, each a 30s timeout in query planning rather than execution.

The reorder in section 1 deliberately skipped bounded queries, with a comment saying that shape had never been measured either way. It has now. boundedTrace duplicates its seed inside an aggregate that sums flow-path lengths across two closure hops, so seeding the wide-open side sums them over the national flowline graph.

One variable isolates it. Same bound, same target, only block A's scope differing:

Block A Result
wide open 500, tried to allocate 4.3 GB
scoped to York County 1,429 rows in 4s

A bounded trace is now seeded from whichever side is constrained, the same rule everything else follows. The York question answers in 20s and 3s, 132 samples and 918 facilities, against two timeouts before.

Correctness was checked before speed. Where both forms run they return identical IRI sets on both projections; the bounded answer is a strict subset of the unbounded one (918 of 1,494 facilities, 0 added); and across 5, 10, 30 and 50 km the answers nest, each bound's set inside the next.

Stated rather than buried: step 0 of the statewide sweep shapes still fails, both rescued rows are step 1, so "bounded upstream questions work now" would be false. What is true is that the bound no longer fails because block A is open. facilities upstream(30km) streams changed failure mode rather than passing, and wells is unchanged on all four bounded combinations.


Guardrails

Three new scripts, offline, in CI, all run in about a second:

  • scripts/check-query-joins.mts (307 checks) asserts the join order, the per-filter rule, and that the bounded aggregate seeds from the same side the body leads with. A query can read target-first while the cost stays where it was, so both halves are checked.
  • scripts/check-flowline-scope.mts (400 checks) asserts the river layer is bounded at both ends for all 100 shapes that draw it.
  • scripts/check-query-snapshots.mts (375 shapes) is a golden-file diff of the SPARQL every shape generates. It existed untracked since 2026-09-17 with another branch's package.json pointing at it; it is committed here with its grid extended from 118 shapes to 375, adding a region-placement axis and the flow bound. Measured on section 6's fix, the old grid reports 1 moved shape, the new one reports 74.

check-trace-direction.mts had to be rebuilt rather than relaxed. It kept ?_flEnd hyf:downstreamFlowPathTC ?_flMid on a blacklist of reversed traces, and a block seeded from the target writes that exact triple while tracing correctly. Deleting the entry would have disarmed the only guard against the direction bug fixed on 2026-09-16. Queries naming both ends are now checked by reachability through the flow graph instead. Its check count drops from 2,068 to 960: fewer assertions, proving a stronger property. Both it and the new seed assertion are mutation-tested, and reintroducing either bug fails them.

Existing checks still pass: step labels (651), cache key (19), substance labels (13).

Known limit of the snapshot, recorded so it is not discovered later: it drives everything through planPipeline, so it covers shapes a user can ask for and is blind to builder arguments the planner never produces. The guarantee is "no reachable shape moved unintentionally", not "no emitted SPARQL moved".


How the numbers here were measured

Two traps caught us, both worth knowing before reading any table above.

Cold against warm. The first sweep of the new phase was its first ever run, so every query missed the engine's cache, and the sweep after the fix was warm. Compared directly that credits section 6 with three fixed rows instead of two: the extra row is unbounded and anchor-constrained, its SPARQL byte-identical at 1,950 characters either side, timing out cold and answering in 15s warm. Every before-and-after above is warm against warm, with the pre-fix engine re-run warm for the purpose. Run-to-run noise, once both halves are warm, is zero: three consecutive sweeps at identical code agreed on every row.

Your own load. Three bounded queries were first measured failing with out-of-memory, one reporting 2.9 MB available, while a pipeline run of ours was hammering the same endpoint. Re-run alone they failed differently.

Four sweeps are committed under docs/query-matrix/, and the cold one carries a .note saying it should not be diffed on its own.


Enhancement, not in this PR: the one-cell widening

Worth flagging because it is the largest remaining gap, and it is drafted in docs/plans/drafts/2026-09-19-symmetric-cell-widening.md.

To connect a sample or a facility to a river, there has to be a river in its S2 cell (about 1km across). Often there isn't. So the query widens by one ring of neighbouring cells:

?s2anchor kwg-ont:sfTouches | owl:sameAs ?s2neighbor .

The problem is that this is applied to exactly one side, and which side is an accident of how the query is written rather than a decision.

In York County:

Side Total Has a river in its own cell Lost if not widened
PFOS sample points 361 282 (78%) 78 (22%)
Facilities 1,639 1,343 (82%) 289 (18%)

About one in five on each side. We widen the facility side, so we lose a fifth of the samples:

Sample points Facilities
Widen samples 324 1,060
Widen facilities (what we ship) 200 1,497
Union of both 330 1,507

The 130 sample points we currently drop are not marginal: 82 of them have measured PFOS detections, up to 2,200 ng/L, which is about 110x Maine's 20 ng/L interim drinking water standard. Sample points with a confirmed detection would go from 132 to 214.

Widening both sides in one query does not work (2.6 GB allocation failure, tried two ways), so the plan is to run both and union them. Added cost measured at 2 to 3 seconds.

This is deliberately separate: it touches all 72 hydrology shapes, moves counts upward everywhere, and is an executor change rather than a query-text change. It needs its own measured sweep.

One open question for the team rather than for the code: should the map draw the river below the sample, down to the sea? This PR stops it at the sample, on the grounds that "facilities upstream from samples" means the path between them. That is 71 of the 755 removed flowlines, and it is easy to put back.

Both surfaced on "What facilities are upstream from PFOS samples in York and
Cumberland counties (Maine)?", which failed at step 1 with 500 out-of-memory,
sliced into its two counties, and failed both slices.

Join order. buildFusedWhereBody always wrote the anchor side first, and an
upstream question puts block A there. With no region and no industry filter on
block A the query started from every facility in the graph and traced the
national flowline network to find samples in two Maine counties. QLever does
not search join orders exhaustively at these body sizes, so it follows the
query text: writing the constrained side first is the whole fix, same triples
and same variables in a different order. Measured across all 72 hydrology
shape/config pairs: 5 rescued, 0 regressions, 0 rows lost, faster in 50 of 66
comparable runs. Distance-bounded traces and IRI-pinned slices keep the
anchor-first order, the first because boundedTrace embeds the seed twice and
was never measured reordered, the second because a pin already made the anchor
the small side.

Observation joins. bindEntityInCell emitted the whole observation chain
whenever any sample filter was set, including coso:measurementUnit, which a
non-detect does not have. A substance-only question with "include non-detects"
ticked therefore dropped every non-detect: 774 of 1,688 Cumberland PFOS
observations, turning 23 sample points and 13 facilities into 32 and 15 once
only the joins an active filter reads are emitted. The same required join hid
2,860 of 3,334 popup rows in buildSampleDetailByIriQuery; moving it after
resultValueClauses() with the symbol lookup nested inside costs what it did
before. buildEntityProbeQuery now uses the same lean bind, since the probe list
becomes the chunk membership and a narrowing join there removes entities from
every slice.

scripts/check-query-joins.mts asserts both rules on the emitted SPARQL (265
checks) and runs in CI. docs/DEBUGGING.md records the measurements and the two
designs that were tried and rejected, including the sub-SELECT that rescued 5
shapes and broke 7.
@railway-app

railway-app Bot commented Sep 19, 2026 •

Copy link
Copy Markdown

🚅 Deployed to the explorer-app-pr-55 environment in sawgraph-explorer

Service Status Web Updated
sawgraph-api ✅ Success (View Logs) Web Sep 20, 2026 at 5:14 am UTC
sawgraph-web ✅ Success (View Logs) Web Sep 20, 2026 at 5:14 am UTC

@railway-app
railway-app Bot temporarily deployed to sawgraph-explorer / explorer-app-pr-55 September 19, 2026 19:32 Destroyed
GET_FLOWLINE_GEOMETRIES traced a one-sided transitive closure: it seeded from
the cells of every resolved anchor, widened each by sfTouches, and followed the
network to its end. Nothing in the query referenced the targets, so an anchor
sitting near a drainage divide pulled in the whole of the neighbouring basin.

On "What facilities are upstream from PFOS samples in York County, ME" that
drew 122 segments of the Merrimack and 21 of the Winnipesaukee, ~130km away in
a basin no York sample drains from, plus the Presumpscot, the Suncook and the
Powwow. Nothing looked broken. The map just had extra rivers on it, and they
were real rivers.

The closure is now intersected with the flowlines that reach the resolved
targets. Measured live with the real answer sets (1,497 facilities, 200 sample
points): 3,271 flowlines down to 2,516, in the same 7s. 755 removed, 0 added,
so the fix can only take away flowlines that were never on a path from a
facility to a sample. Of those, 684 are in a different basin and 71 lie below
a sample, which is the one visible behaviour change: the drawn river now stops
at the sample instead of running on to the sea.

Written as an intersection of two DISTINCT closures. The direct membership
test, `?flowline hyf:downstreamFlowPathTC? ?_flTarget`, leaves ?flowline
unbound on the left and QLever times out joining on it (31s, "Join on
?flowline"). The reach is a bare TC rather than reflexive: `TC?` was measured
and recovers 0 flowlines while costing 8s.

The bounded branch gets the same intersection. At 25km it went 3,034 -> 2,516,
matching the unbounded set, because connectivity dominates the distance cap at
that range; at 5km the cap still bites (2,314), so the two constraints compose.
The shipped bounded query still drew a Merrimack segment through its cap.

scripts/check-flowline-scope.mts asserts both ends are bound for all 100 shapes
that draw the layer, bounded and unbounded, and runs in CI.
fix(engine): bound the flowline layer at both ends
@railway-app
railway-app Bot temporarily deployed to sawgraph-explorer / explorer-app-pr-55 September 19, 2026 19:56 Destroyed
The one-cell sfTouches expansion that connects an entity to the river network
is applied to exactly one side of a hydrology trace, and which side is an
artefact of how buildFusedWhereBody orders its blocks rather than a modelling
decision. Measured in York County: 22% of PFOS sample points and 18% of
facilities have no flowline in their own S2 cell, so whichever side is not
widened silently loses about a fifth of its entities.

On "What facilities are upstream from PFOS samples?" scoped to York County,
widening the sample side instead of the facility side gives 324 sample points
and 1,060 facilities against the shipped 200 and 1,497. The union of both is
330 and 1,507, and recovers 130 sample points of which 82 carry quantified PFOS
detections up to 2,200 ng/L. Points with a confirmed detection go from 132 to
214.

The plan proposes running both variants and unioning them, because widening
both sides inside one query does not run: two formulations, flat and with the
cell set materialised first, both fail with "tried to allocate 2.6 GB". Added
cost is 2-3s rather than double, since the second variant caches at 1.3s while
the shipped form does not.

Drafted rather than active: it touches all 72 hydrology shapes, moves counts
upward across them, and is the second executor change queued against the same
code path as the retry hook in the 2026-09-17 plan. The two should be designed
together so there is one mechanism rather than two similar ones.

Also records two QLever measurement traps hit while producing the figures: a
nested OPTIONAL containing a BIND returned a count of 0 alongside a non-zero
MAX over the same variable, and an OPTIONAL used for predicate coverage
under-reported silently. Every figure was re-derived with flat COUNT queries.
Brings in docs/plans/drafts/2026-09-19-symmetric-cell-widening.md, which
documents the follow-up work that falls out of the two fixes on this branch.
No code change.
@railway-app
railway-app Bot temporarily deployed to sawgraph-explorer / explorer-app-pr-55 September 19, 2026 20:45 Destroyed
Cleanup pass over the two fixes on this branch. No intended behaviour change
except where noted; the York question emits a byte-identical body for steps 1
and 2 and the same 2,516 flowlines for step 3, verified live.

sideIsConstrained was a third copy of a predicate the repo already had twice,
in scope.ts as hasBlockFilters and in sparqlErrors.ts as isNarrowed. scope.ts
asks the identical question to pick a chunking axis, so the two drifting apart
means the executor slices on an axis the template already led with. scope.ts
imports fusedQueries, so the shared predicate lands here as blockIsFiltered and
scope.ts uses it. sparqlErrors keeps its own, which is this plus the region
check.

That copy also had a defect the original did not: a generic "any field is
non-null" scan counts includeNondetects: true, which is the UI's default
checked state and narrows nothing. A samples block with only that set was read
as constrained and reordered the whole body. sampleJoinsNeeded already draws
the line correctly, so samples delegate to it. Verified: that block now leads
with ?s2anchor, includeNondetects: false and a substance filter still lead with
?s2target.

buildFusedFlowlineQuery re-derived the anchor block over IRIs its own VALUES
list had already pinned, which is exactly what targetReachClause eight lines
below refuses to do. Same bare-block treatment now. For a facilities anchor
that drops a redundant industry VALUES; for a samples anchor it drops the
entire observation chain, including the coso:measurementUnit join this branch
exists to keep out of queries that never read it.

Also: pinValues instead of a fourth hand-rolled VALUES block (it guards the
empty list, the inline form emitted "VALUES ?spC {  }"); needsUnitJoin instead
of restating the range predicate the comment already pointed at; a dead branch
in check-query-joins where both arms returned 'other'; and the reflexive-reach
assertion in check-flowline-scope, which matched the downstream spelling only
and left the upstream shape unguarded by a check whose whole point is the TC?
regression.

Two stale comments corrected: the sampleObservations doc named hydrate queries
that were deleted in 66f3fda, and buildFusedWhereBody still described the
hasSampleFilters override the same commit removed.

Deferred to the plan docs rather than done here, both recorded as tasks:
emitting the body from an ordered fragment list instead of two hand-maintained
renderings (worth doing with the widen option, which would otherwise make it
four), and bounding targetReachClause by region when the target list is too
large to inline. Also added a task to measure the distance-bounded shape
reordered, since !maxDistanceKm is the one exclusion in the gate that is a "we
did not look" rather than a measured "it was worse".
@railway-app
railway-app Bot temporarily deployed to sawgraph-explorer / explorer-app-pr-55 September 19, 2026 20:57 Destroyed
@prayaslashkari prayaslashkari changed the title fix(engine): stop two silent filters in the fused queries Fix upstream questions: out-of-memory, dropped non-detects, and rivers in the wrong basin Sep 19, 2026
…join rule

The two fixes on this branch were recorded as bugs but not as behaviour. Someone
reading the app had no way to learn what the "Include non-detects" checkbox does,
and the wiki's only mention was three sentences saying it exists.

Wiki: new page "Samples Non Detects", linked from the sidebar, Home and the
Dropdowns index. What a non-detect is (it is a measurement, not a gap: the record
carries detection limits instead of a value, which is why it has no
coso:measurementUnit), that the box is ticked by default, and that expanding
"+ Add Filters" writes nothing and leaves the SPARQL byte-identical.

The behaviour the page exists for: the checkbox filters at two levels, not one.
York County, all substances, 451 sample points and 23,530 observations ticked
against 367 and 7,110 unticked. The 367 surviving points hold 21,822 observations
between them, so unticking also strips 14,712 non-detect rows from popups that
stay on the map. Non-detects are 70% of observations in York and 77% across
Maine, so this is most of the data rather than a trim. Also the four-way point
classification, and why 14 York points have no measurements at all: they are all
EGAD soil sampling locations whose results are not loaded, while every well at
the same site has 24 to 30 observations.

DEBUGGING.md: a 2026-09-19 entry for the stream layer drawing the Merrimack, with
the one-sided closure that caused it, the intersection that fixed it (755
flowlines removed, 0 added), and the two forms measured and rejected. Records
that a distance cap is not a substitute, since the shipped bounded query drew a
Merrimack segment through its own 25km cap. Also the includeNondetects: true
defect from the cleanup pass, where the UI's default ticked state was read as a
narrowed side and reordered the whole body.

ARCHITECTURE.md: a new Key Design Decision on why each filter declares its own
joins, since "every required triple is also a filter" is the principle behind
both bugs and the thing a newcomer needs before writing a template. Pattern 3
gains a note that a closure needs both ends bound.

Corrects an explanation I had wrong in three places. `TC?` recovering 0 extra
flowlines is not because the segment arrives via another target; it is because
hyf:downstreamFlowPathTC is already reflexive, which ARCHITECTURE.md Pattern 3
had said all along. Verified: X TC X holds for 2,104 of 2,104 York County
flowlines, 434,501 self-pairs graph-wide. The measurement and the conclusion are
unchanged, only the reason. One consequence worth having right: the segment a
target sits on is drawn, and what the intersection excludes is the river below it.
@railway-app
railway-app Bot temporarily deployed to sawgraph-explorer / explorer-app-pr-55 September 19, 2026 22:26 Destroyed
…tream layer

Entry 13 covers the join order and the unit join fixed on 2026-09-17, including
that dashboard counts will move up when it deploys and that figures taken off
the app before that date understate PFAS presence.

Entry 14 covers the stream layer drawing the Merrimack, the includeNondetects
defect found while reviewing it, the new wiki page, and the cell-widening gap
that is measured but deliberately not fixed here.
@railway-app
railway-app Bot temporarily deployed to sawgraph-explorer / explorer-app-pr-55 September 19, 2026 22:33 Destroyed
The dot sat at the top of the content box rather than on the text beside it, and
the running step carried its own padding, so a step visibly shifted when it
started. Padding moves onto .timeline-content so every step has it, and the
marker gains a matching top offset.
@railway-app
railway-app Bot temporarily deployed to sawgraph-explorer / explorer-app-pr-55 September 19, 2026 23:32 Destroyed
`maxDistanceKm` appeared zero times in the harness. M4, which the header called
a distance sweep, sweeps the `near` relationship's hop distance in miles; the
hydrology relationship's flow-distance bound in km was swept by nothing, and the
only bounded shape the matrix ever ran was one dashboard prebuilt in M5.

Worse, `mk` attached the region to block A unconditionally. For an upstream
question the planner maps block A to the anchor, so a region on A always left
the anchor as the constrained side: the target-first join order was unreachable
from this harness, and so was every bounded question that leaves block A wide
open, which is the shape users build.

`mk` now takes `{ km, regionOn }`, and MD runs four shapes bounded and unbounded
with the region on each block. It measures both discovery steps rather than step
0 like M1-M4, because the two projections share a WHERE clause but not a fate:
at 30 km the samples projection answered in 25s while the facilities projection
ran out of memory at 32s, and a step-0-only phase would have called that working.

The baseline (32 rows, 11 pass) shows the rule cleanly: a bounded query works
when the region sits on the block that becomes the anchor and fails when it sits
on the target. Upstream makes block A the anchor, downstream makes block C the
anchor, and the mirror pair `facilities upstream(30km) samples [on A]` and
`samples downstream(30km) facilities [on C]` return identical counts, 2,399 and
2,523. Wells is the exception and fails on all four bounded combinations for its
own reasons, as in W38 entry 12.
The constrained-side-first reorder skipped distance-bounded queries, with a
comment saying that shape had not been measured either way. Measured now: every
bounded shape whose constrained side was the target failed, and the bound was
the thing breaking them. "Facilities upstream from PFOS samples in York" with
block A left wide open, at 30km: 429 in query planning at 30s on both
projections. Unbounded, the same question answers.

boundedTrace embeds its seed inside an aggregate that sums path lengths over two
closure hops, so seeding the wide-open side sums them across the national
flowline graph: 1,506,326 facilities. Seeded from the 132 samples instead, the
same question answers in 20s and 3s, returning 132 samples and 918 facilities.

The reorder is the same rule the unbounded path already followed, so the guard
loses one clause. boundedTrace gains a seed side, and the trace, the GROUP BY
and the "+1" fringe all follow it: the fringe still extends away from the seed,
which means the physical end that gets the extra segment now follows the seed
side. Checked across all 1,152 hydrology shape and config pairs: 144 queries
change text, exactly the bounded ones whose constrained side is the target, and
the other 1,008 are byte-identical.

Correctness, before trusting the speed. Where both forms run, they return
identical IRI sets on both projections. Against the unbounded answer the bounded
one is a strict subset, 918 of 1,494 facilities with 0 added. Across 5, 10, 30
and 50km the answers nest, each bound's set contained in the next.

Two checks had to change, and both were rebuilt rather than relaxed.
check-query-joins asserted that bounded traces keep the anchor-first order,
which was the defect; it now asserts the same constrained-side rule as every
other shape, and separately that the aggregate seeds from that side too, since a
query can read target-first while the cost stays where it was.

check-trace-direction is the more delicate one. It kept
`?_flEnd hyf:downstreamFlowPathTC ?_flMid` on a blacklist of reversed traces,
and a target-seeded block writes that exact triple while tracing correctly. That
string cannot decide it any more, so queries naming both ends are now checked by
reachability: is there a directed path from the anchor's flowline to the
target's. Verified by mutation - reintroducing the 2026-09-16 direction bug
(traceDirection derived from the relationship name) still fails the check, and a
half-applied reorder that moves the text but leaves the aggregate seeding from
the anchor fails the new one.
@railway-app
railway-app Bot temporarily deployed to sawgraph-explorer / explorer-app-pr-55 September 20, 2026 04:19 Destroyed
The script existed since 2026-09-17 but was never committed, so package.json on
feat/demo-questions-prewarmed registered `check-query-snapshots` against a file
nobody else had and that branch's CI never ran it. Committing it here, with the
grid extended to cover what this week showed it was blind to.

Why it earns a place next to the six rule-checks: those assert properties of a
query, so they see a shape only if someone thought to assert something about it.
This one notices when a change moves a shape nobody was thinking about, which is
the failure mode of a builder shared by every relationship. Three of the bugs
fixed this week were invisible in behaviour and obvious in the emitted SPARQL.

The grid gains two axes, both of which hid something real:

Region placement. It put the region on block A always. For a downstream
question that makes the target the constrained side, so the target-first join
order was recorded; for an upstream question it makes the anchor constrained, so
that path never was, and neither was the question behind #55 and #57 - block A
wide open, block C scoped to a county.

The flow-distance bound. A bounded trace is a different query rather than a
filtered one: it seeds an aggregate. The grid had two bounded shapes, both with
the region on block A. Measured on the fix in ec221d9: the old grid reports one
moved shape, `VARIANT: downstream maxDistanceKm=10`. The new one reports 74, and
buckets them as bounded 72, region on A 36, region on C 36.

6 x 6 x 3 x 2 unbounded, 6 x 6 x 2 x 2 bounded, plus 15 variants for hop counts,
per-type filter blocks and the two questions that have actually broken in the
app. 375 shapes.

Two things keep the file readable rather than enormous. The PREFIX preamble is
byte-identical in every query and was 66% of it, so it is stripped and recorded
once; a change to the prefix list still moves exactly one entry. And query
bodies are written once each in a table at the end and referenced by hash: 375
shapes reference 2,078 step queries but only 603 distinct ones, the
region-boundary query alone being identical in 373 of them. Together those take
the snapshot from 1.7MB to 688KB and turn a one-line change to a shared query
from 373 lines of diff into one.

Known limit, worth stating rather than discovering later: it drives everything
through planPipeline, so it covers shapes a user can ask for and is blind to
builder arguments the planner never produces. A reordering of the anchor-seeded
`upstream` branch of boundedTrace passes all seven checks, because the planner
always passes 'downstream'. The guarantee is "no reachable shape moved
unintentionally", not "no emitted SPARQL moved".
Four MD sweeps. The result is 2 shapes fixed, 0 regressed, and it took three
extra runs to be able to say that honestly.

The first sweep (002e529, committed earlier) was cold, and the sweep taken after
the fix was warm, so comparing them credited the fix with three flips. One of
those three is `facilities upstream samples [ME on A] step1`, which is unbounded
and anchor-constrained: the reorder cannot reach it, and the generated SPARQL is
byte-identical at 1,950 characters either side of ec221d9. It times out cold and
answers in 15s warm. That row is the cache, not the code. The cold sweep now
carries a .note saying so.

Re-running the pre-fix engine warm (e84a11f's src, current harness) gives the
comparison that means something:

  pre-fix warm   12/32 pass
  post-fix warm  14/32 pass, identical in three consecutive runs

  FIXED  facilities upstream(30km) samples [ME on C] step1   oom -> 3213 rows, 17s
  FIXED  samples downstream(30km) facilities [ME on A] step1 oom -> 3213 rows, 15s
  REGRESSED  none
  row-count drift where both pass  none

Both fixed rows are exactly the condition the reorder covers: bounded, with the
constrained side on the target. All 8 bounded rows whose constrained side is the
anchor are unchanged, which is the guard working.

Three things the sweep says that the spot-check could not.

Run-to-run noise is zero here. Three sweeps at identical code disagree on no row
and report row counts identical to the unit, so a single flip in this file is
signal, provided both halves are warm.

Step 0 of both fixed shapes still fails. The fix lands on step 1 both times.
These are statewide Maine with no filters, where the samples projection is the
far heavier half; the York County question this started from passes both steps
in 20s and 3s. Question size, not the reorder, but it means "bounded upstream
questions work now" is false as a general claim.

`facilities upstream(30km) streams [ME on C]` did not start passing. It changed
failure mode, timeout to out-of-memory on step 0 and to a sort-estimate timeout
on step 1. Worth knowing before anyone reads the two fixes as covering the
bounded target-constrained case generally.

Wells is unchanged on all four bounded combinations, as it was before, for its
own reasons (W38 entry 12).
QUERY-MATRIX.md gains Part 10: what the flow-distance bound was doing, the
one-variable isolation (block A wide open gives a 4.3 GB allocation failure,
block A scoped to York County gives 1,429 rows in 4s), the warm-against-warm
before and after, and what the fix does not do.

DEBUGGING.md gains a 2026-09-20 entry with the root cause, the reachability
tripwire that replaced a string blacklist a correct query now trips, and the two
measurement traps this cost: comparing a cold sweep against a warm one, and
measuring an endpoint while your own pipeline run is hammering it.

W38 entries 15 and 16. Entry 15 states the limits in the entry rather than
leaving them to be found: step 0 of the statewide shapes still fails, both
rescued rows are step 1, the streams shape changed failure mode rather than
passing, and wells is untouched.
@railway-app
railway-app Bot temporarily deployed to sawgraph-explorer / explorer-app-pr-55 September 20, 2026 05:14 Destroyed
@prayaslashkari
prayaslashkari merged commit 98190de into development Sep 20, 2026
3 checks passed
@prayaslashkari prayaslashkari changed the title Fix upstream questions: out-of-memory, dropped non-detects, and rivers in the wrong basin Fix upstream questions: out-of-memory, dropped non-detects, rivers in the wrong basin, and a distance filter that did nothing Sep 20, 2026

This branch was successfully deployed

No deployments
sawgraph-explorer / explorer-app-pr-55 — 0abdc499 Deployed Sep 20, 2026 by railway-app[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant