Conjunction SSA plugin (tested rewrite) + agent scaffolding - #9
Conjunction SSA plugin (tested rewrite) + agent scaffolding#9jakexcosme wants to merge 4 commits into
Conversation
Co-Authored-By: Jake Cosme <jake@cognition.ai>
Original prompt from Jake
|
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
…ss window Co-Authored-By: Jake Cosme <jake@cognition.ai>
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Supersedes the original implementation on this branch with a rewrite
that is unit tested and separates concerns across providers.
The previous version bundled object seeding, layout config and telemetry
into large modules with no test coverage. This replaces them with:
- propagator.js Keplerian + J2 propagation, geodetic conversion
- pc.js Foster/Alfano-style probability of collision
- tles.js seed TLE set and TLE parsing
- models.js domain object models for roots, folders, views
- ConjunctionEngine.js screening, TCA refinement, history buffers
- Ssa{Object,Metadata,Telemetry}Provider.js Open MCT providers
Adds 35 unit tests across five spec files covering propagation,
Pc computation, pair screening, and plugin registration.
Also adds agent scaffolding (.devin/, .agent/, AGENTS.md) documenting
project conventions, and registers the plugin in plugins.js and the
index.html demo host.
Verified: npm run lint clean; unit suite failure set identical to
master baseline (no regressions), 1003 passing vs 968 on master.
jakexcosme
changed the title
The engine emits NaN missKm/tcaOffsetS for pairs with no local minimum of separation in the look-ahead window. This is a deliberate sentinel, not an error, and the worst-case summary excludes such pairs. Undocumented until now, and the omission caused a unit test to compute Math.min over unfiltered values and assert against NaN.
| const DEFAULT_HARD_BODY_RADIUS_M = 10; | ||
| const DEFAULT_POSITION_SIGMA_M = 100; | ||
|
|
||
| function probabilityOfCollision(missKm, options = {}) { | ||
| const hardBodyRadiusM = options.hardBodyRadiusM ?? DEFAULT_HARD_BODY_RADIUS_M; | ||
| const positionSigmaM = options.positionSigmaM ?? DEFAULT_POSITION_SIGMA_M; | ||
| const missM = missKm * 1000; | ||
| const sigmaSq = positionSigmaM * positionSigmaM; | ||
| const radiusSq = hardBodyRadiusM * hardBodyRadiusM; | ||
| const exponent = -(missM * missM) / (2 * sigmaSq); | ||
| return (radiusSq / (2 * sigmaSq)) * Math.exp(exponent); |
There was a problem hiding this comment.
🔍 RED watch level likely unreachable with default Pc sigma
probabilityOfCollision with the default 100 m sigma yields pc > 1e-4 only for miss below ~0.28 km, but the RED condition needs missKm < 5 AND pc > 1e-4. The ISS/CHASER pair is only asserted to close within 200 km, so the RED indicator the README promises to demonstrate can stay dark.
Was this helpful? React with 👍 or 👎 to provide feedback.
| if (Number.isNaN(datum.missKm)) { | ||
| return; | ||
| } | ||
| if (!worst || datum.pc > worst.pc || datum.missKm < worst.missKm) { |
There was a problem hiding this comment.
📝 Info: Worst-case OR selection is redundant but safe
In summarize the worst pair is chosen when pc is larger OR missKm is smaller. Since pc is a strictly decreasing function of missKm under shared options, the two clauses never disagree, so this always yields the smallest-miss pair.
Was this helpful? React with 👍 or 👎 to provide feedback.
| for (let t = startMs; t <= endMs; t += stepMs) { | ||
| const primaryState = propagateEci(primaryEntry.elements, t); | ||
| const secondaryState = propagateEci(secondaryEntry.elements, t); | ||
| const d = distanceKm(primaryState.position, secondaryState.position); | ||
|
|
||
| if ( | ||
| previousPreviousDistance !== Infinity && | ||
| previousDistance < previousPreviousDistance && | ||
| previousDistance < d && | ||
| previousDistance < bestBracketMiss | ||
| ) { | ||
| bestBracketMiss = previousDistance; | ||
| bestBracket = [previousPreviousTime, t]; | ||
| } | ||
|
|
||
| previousPreviousDistance = previousDistance; | ||
| previousPreviousTime = previousTime; | ||
| previousDistance = d; | ||
| previousTime = t; | ||
| } |
There was a problem hiding this comment.
📝 Info: Close approach at window start reported as no-conjunction
screenPair needs three coarse samples to bracket a minimum, so a pair already closest at or one step past the start time is never bracketed and emits NaN miss. Acceptable given the look-ahead intent, but forward-passing conjunctions near now are missed.
Was this helpful? React with 👍 or 👎 to provide feedback.
| getMetadata(domainObject) { | ||
| const values = domainObject.type === TRACKED_TYPE ? TRACKED_VALUES : PAIR_VALUES; | ||
| return { ...domainObject.telemetry, values }; | ||
| } |
There was a problem hiding this comment.
📝 Info: Correct metadata depends on provider registration order
getMetadata supplies the value list only because addProvider unshifts it ahead of the core default provider, which also matches these objects via their plain telemetry field. Correct today, but order-dependent.
Was this helpful? React with 👍 or 👎 to provide feedback.
|
❌ Cannot revive Devin session - the session is too old. Please start a new session instead. |
2 participants
Summary
Supersedes the original Conjunction SSA implementation on this branch with a
unit-tested rewrite, and adds repository agent scaffolding.
This PR contains two related but separable bodies of work. They were combined
at the requester's direction; reviewers may wish to read them as two passes.
1. Conjunction SSA plugin — rewritten
The previous version on this branch bundled object seeding, layout config and
telemetry into large modules with no test coverage. It has been replaced
with a provider-separated design:
propagator.jspc.jstles.jsmodels.jsConjunctionEngine.jsSsaObjectProvider.js/SsaMetadataProvider.js/SsaTelemetryProvider.jsRemoved:
ConjunctionTelemetryProvider.js,OrbitPropagator.js,conditionSetConfig.js,layoutConfig.js,seedObjects.js.36 unit tests across four spec files cover propagation, Pc computation,
pair screening, the worst-case summary, and plugin registration.
2. Agent scaffolding
AGENTS.md,.devin/rules/,.devin/workflows/, and.agent/knowledge/documenting project conventions, plugin anatomy, telemetry datum shapes, and
the time API. Documentation only — no runtime effect.
Notable behaviour: NaN miss distance
When a pair has no close-approach bracket inside the look-ahead window, the
engine emits
missKm: NaN/tcaOffsetS: NaNandpc: 0.summarize()deliberately excludes those pairs from the worst-case summary. This is the
intended contract and is now covered by two explicit tests.
Test plan
npm run lint— clean (js, vue, spelling)npm test— failure set identical to master baseline; no regressionsnpm startand confirm the Conjunction SSA root rendersBaseline comparison
The 7–8 failures are pre-existing on master (Object API Search x4, Image
Exporter, URLIndicator clock, fps NaN) plus one flaky Imagery test that appears
intermittently on master too — it showed up in baseline run 2 and not run 1, so
it is not attributable to this change.
Test breakdown
pluginSpec.jspropagatorSpec.jsConjunctionEngineSpec.jspcSpec.jsDefects found and fixed while preparing this branch
The rewrite did not arrive green. Everything below was broken and has been
fixed in this PR:
func-styleviolationsfixed by hand (arrow-assigned consts converted to function declarations).
pluginSpectests timing out at 6000ms. The outerbeforeEachawaited the
startevent whilestartHeadless()was only called in nestedbeforeEachblocks, which run afterwards — the event could never fire.Every other spec in the codebase pairs
on('start', done)withstartHeadless()in the same block; this one didn't.ConjunctionEngineSpecasserting againstNaN. It ranMath.minoverunfiltered pair miss distances. The engine intentionally emits
NaNfornon-converging pairs and
summarize()excludes them; the test didn't mirrorthat contract. Now documented in the plugin README and covered by two new
tests.
toBeUndefined()onopenmct.types.get(), whichreturns an
UNKNOWN_TYPEsentinel and neverundefined.request()test with no time bounds — the fixed 2024 test timestampfell outside the Time API's real-now default window, so it got zero rows.
lint:spellingis part ofnpm run lintand would have failed CI.e2e/playwright-ci.config.jsthat added anunused
lodashimport, violating the repo'syou-dont-need-lodash-underscorerule.Known CI failure (not caused by this PR)
e2e-couchdbfails on every PR in this fork. All 17 e2e tests pass; the jobthen fails at the Codecov upload step with
Token required - not valid tokenless uploadandToken length: 0.codecov/codecov-action@v5runs withfail_ci_if_error: trueand this fork hasno secrets configured (
gh secret listreturns empty), so both the DockerHublogin and the Codecov upload fail. Fixing it requires adding
CODECOV_TOKENasa repository secret, or setting
fail_ci_if_error: falsefor forks — arepo-settings change, not a code change.
Generated with Devin