Answer the review: a short answer makes more survivors, and must not be skipped
Three things this got wrong. The direction: truncating Gaia leaves the HYG rows whose counterpart it dropped without one, so survivors rise — 10 886 today, 12 711 at half the rows, 16 258 at a third — which the comment claimed was the other way, and which decides whether the 15 000 ceiling can be leaned on at all (it catches a truncation past about two thirds, and nothing shallower). The throw: `fetchStars` catches everything a source throws and skips it, so a truncated CSV was reported as "the archive was unreachable" one step after `writeStarAssets` had already overwritten the published catalogue. Marked with `GaiaAnswerError` and rethrown there, so an answer that cannot be worked with fails the run where it happened. Measured end to end in a throwaway working directory, 300 000 rows in the cache: fails, names the cache file to delete, assets untouched. With the rethrow taken back out again: assets written, then "the archive was unreachable". The row limit: `rows.length >= ROW_LIMIT` is true for every reduced ETL_GAIA_ROW_LIMIT, so the tripwire fired on exactly the deliberate slice the override exists for — and told the operator to raise it. Gated on the same flag as its neighbour. `ETL_GAIA_ROW_LIMIT=20000` now runs through; without the gate it dies on the limit it was given. Also: the row floor names the one cache file it is about rather than a glob that takes the Hipparcos cross-match with it, and says an edited query is a third reason it can fire — DEFAULT_QUERY_ROWS now sits under the query it counts. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016jxMkwA2rbicdGxHosecYi
This commit is contained in:
+5
-2
@@ -73,8 +73,11 @@ function validateStars(stars: StarRecord[]): void {
|
||||
* lacks: bright stars it saturates on, red dwarfs past its magnitude cut. So the headroom left to
|
||||
* the ceiling tracks the gap between those two cutoffs as much as Gaia's completeness.
|
||||
*
|
||||
* This bounds a merge that went wrong. It cannot bound a Gaia download that came back short: that
|
||||
* makes *fewer* survivors, not more, and is guarded where it can be seen, in `fetchGaiaStars`.
|
||||
* This bounds a merge that went wrong, and — loosely — a Gaia download that came back short: a
|
||||
* truncated answer leaves the HYG rows whose counterpart it dropped without one, so survivors go
|
||||
* *up*, not down. Measured against the published catalogue: 10 886 today, 11 004 at nine tenths of
|
||||
* the rows, 12 711 at half, 16 258 at a third. So this ceiling only catches a truncation past about
|
||||
* two thirds, and `fetchGaiaStars` catches the shallower ones with its own row floor.
|
||||
*/
|
||||
const MAX_UNMERGED_TWINS = 100;
|
||||
const MAX_HYG_SURVIVORS = 15_000;
|
||||
|
||||
@@ -3,7 +3,7 @@ import { writeFileSync } from 'node:fs';
|
||||
import { mergeStarCatalogues, placementDistancePc } from '../../src/app/shared/astro/star-merge';
|
||||
import { encodeStarCatalog } from '../../src/app/shared/models/star-catalog';
|
||||
import { StarRecord, SUN_STAR_ID } from '../../src/app/shared/models/star.model';
|
||||
import { fetchGaiaDistancesByHip } from './sources/gaia';
|
||||
import { fetchGaiaDistancesByHip, GaiaAnswerError } from './sources/gaia';
|
||||
import { positionalSources } from './sources/registry';
|
||||
import { PARALLAX_PRECISION_MAS } from './sources/star-sources';
|
||||
import { parseCsvObjects, parseOptionalNumber } from './lib/csv';
|
||||
@@ -142,7 +142,9 @@ export async function fetchStars(): Promise<StarRecord[]> {
|
||||
*
|
||||
* A source that cannot be reached is reported and skipped here rather than thrown, so a run still
|
||||
* gets as far as validation and says what it has. Whether that may be published is decided
|
||||
* there: `validateMerge` in build.ts refuses a catalogue Gaia contributed nothing to.
|
||||
* there: `validateMerge` in build.ts refuses a catalogue Gaia contributed nothing to. A source
|
||||
* that answered with something unusable ({@link GaiaAnswerError}) is a different matter, and stops
|
||||
* the run where it happened rather than being reported later as an outage.
|
||||
*/
|
||||
async function mergeWithOtherSources(hygStars: StarRecord[]): Promise<StarRecord[]> {
|
||||
const others = positionalSources().filter((source) => source.id !== 'hyg');
|
||||
@@ -160,6 +162,11 @@ async function mergeWithOtherSources(hygStars: StarRecord[]): Promise<StarRecord
|
||||
stars: await source.fetch!()
|
||||
});
|
||||
} catch (error) {
|
||||
// An answer that cannot be worked with is not an outage: skipping it would write a
|
||||
// half-catalogue over the published assets before the merge gate got to say so.
|
||||
if (error instanceof GaiaAnswerError) {
|
||||
throw error;
|
||||
}
|
||||
console.log(` skipping ${source.name}: ${error instanceof Error ? error.message : error}`);
|
||||
}
|
||||
}
|
||||
|
||||
+36
-23
@@ -45,23 +45,6 @@ const DISTANCE_CUTOFF_PC = Number(process.env['ETL_GAIA_DISTANCE_PC'] ?? DEFAULT
|
||||
const MAGNITUDE_LIMIT = Number(process.env['ETL_GAIA_MAGNITUDE_LIMIT'] ?? DEFAULT_MAGNITUDE_LIMIT);
|
||||
const ROW_LIMIT = Number(process.env['ETL_GAIA_ROW_LIMIT'] ?? DEFAULT_ROW_LIMIT);
|
||||
|
||||
/**
|
||||
* How many rows the scheduled job's own query holds: 412 765, and DR3 is a finished data release,
|
||||
* so that number only moves when the query does.
|
||||
*
|
||||
* Checked because a short answer looks exactly like a complete one. The TAP service truncates on
|
||||
* its own timeout and still serves a well-formed CSV with a 200, and the rows are ordered by
|
||||
* magnitude, so what comes back is the bright half — the half HYG overlaps. The merge gate in
|
||||
* `build.ts` would then see Gaia stars present, fewer HYG survivors and fewer unmerged twins, and
|
||||
* pass a catalogue missing two hundred thousand stars, which the weekly job would publish and the
|
||||
* runner would cache for the weeks after it. Same failure, and same guard, as
|
||||
* {@link MIN_USABLE_HIP_DISTANCES} below.
|
||||
*
|
||||
* Only checked for that query: the environment overrides exist to fetch a smaller slice on purpose.
|
||||
*/
|
||||
const DEFAULT_QUERY_ROWS = 412_765;
|
||||
const MIN_ROW_SHARE = 0.95;
|
||||
|
||||
/**
|
||||
* Relative parallax error above which a star is dropped: a parallax measured to worse than 20%
|
||||
* gives a distance that is not worth plotting, and inverting a noisy parallax biases it badly.
|
||||
@@ -87,6 +70,35 @@ function buildQuery(): string {
|
||||
].join(' ');
|
||||
}
|
||||
|
||||
/**
|
||||
* How many rows the query above holds when nothing is overridden: 412 765, and DR3 is a finished
|
||||
* data release, so that number only moves when the query does. It lives here, under the query, so
|
||||
* that an edit to any of its filters is made with the count it invalidates in view.
|
||||
*
|
||||
* Checked because a short answer looks exactly like a complete one. The TAP service truncates on
|
||||
* its own timeout and still serves a well-formed CSV with a 200, and the rows are ordered by
|
||||
* magnitude, so what comes back is the bright half — the half HYG overlaps. The merge gate in
|
||||
* `build.ts` would then see Gaia stars present and a survivor count barely moved, and pass a
|
||||
* catalogue missing two hundred thousand stars, which the weekly job would publish and the runner
|
||||
* would cache for the weeks after it. Same failure, and same guard, as
|
||||
* {@link MIN_USABLE_HIP_DISTANCES} below.
|
||||
*
|
||||
* Only checked when nothing is overridden: the environment overrides exist to fetch a smaller
|
||||
* slice on purpose.
|
||||
*/
|
||||
const DEFAULT_QUERY_ROWS = 412_765;
|
||||
const MIN_ROW_SHARE = 0.95;
|
||||
|
||||
/**
|
||||
* An answer the archive gave that cannot be worked with, as against an archive that gave none.
|
||||
*
|
||||
* `fetchStars` skips a source it cannot reach and leaves the merge gate to judge the result. That
|
||||
* is right for an outage and wrong for a truncated CSV, which would be skipped, cached, and land
|
||||
* as "the archive was unreachable" long after the assets had been overwritten — so these throws
|
||||
* are marked, and rethrown there.
|
||||
*/
|
||||
export class GaiaAnswerError extends Error {}
|
||||
|
||||
/**
|
||||
* Gaia publishes no spectral classifications, but `bp_rp` is a colour index on the same footing
|
||||
* as HYG's `ci` — so the app's existing colour and spectral-class handling works unchanged, and
|
||||
@@ -110,17 +122,18 @@ export async function fetchGaiaStars(): Promise<StarRecord[]> {
|
||||
// Keyed by the whole request, so a response cached for other columns, another order, or
|
||||
// another endpoint can never be mistaken for this one — the cache records only that some
|
||||
// response arrived, not what it answered.
|
||||
const csv = await fetchTextCached(url, `gaia-dr3-${createHash('sha1').update(url).digest('hex').slice(0, 8)}.csv`);
|
||||
const cacheKey = `gaia-dr3-${createHash('sha1').update(url).digest('hex').slice(0, 8)}.csv`;
|
||||
const csv = await fetchTextCached(url, cacheKey);
|
||||
const rows = parseCsvObjects(csv);
|
||||
const jobsQuery = DISTANCE_CUTOFF_PC === DEFAULT_DISTANCE_CUTOFF_PC && MAGNITUDE_LIMIT === DEFAULT_MAGNITUDE_LIMIT && ROW_LIMIT === DEFAULT_ROW_LIMIT;
|
||||
if (jobsQuery && rows.length < DEFAULT_QUERY_ROWS * MIN_ROW_SHARE) {
|
||||
throw new Error(
|
||||
`Gaia returned ${rows.length} rows, not the ~${DEFAULT_QUERY_ROWS} this query holds — the answer was cut short, ` +
|
||||
'or was an error page served with a 200; delete tools/etl/.cache/gaia-dr3-*.csv once the archive answers properly'
|
||||
throw new GaiaAnswerError(
|
||||
`Gaia returned ${rows.length} rows, not the ~${DEFAULT_QUERY_ROWS} this query holds — the answer was cut short, it was an error page ` +
|
||||
`served with a 200, or the query was edited without updating DEFAULT_QUERY_ROWS; delete tools/etl/.cache/${cacheKey} once the archive answers properly`
|
||||
);
|
||||
}
|
||||
if (rows.length >= ROW_LIMIT) {
|
||||
throw new Error(`Gaia returned the query's own ${ROW_LIMIT}-row limit, so it is the limit deciding what the map holds; raise ETL_GAIA_ROW_LIMIT.`);
|
||||
if (jobsQuery && rows.length >= ROW_LIMIT) {
|
||||
throw new GaiaAnswerError(`Gaia returned the query's own ${ROW_LIMIT}-row limit, so it is the limit deciding what the map holds; raise ETL_GAIA_ROW_LIMIT.`);
|
||||
}
|
||||
const stars: StarRecord[] = [];
|
||||
|
||||
|
||||
Reference in New Issue
Block a user