Repository navigation
Phase 5 exit: every injected fault and every hostile manifest recovers or ends typed - #198
Merged
Merged
Conversation
…ompared The corpus record is about to be taken twice: once on the core-only player it has always described, and once on a player built with ADR-0011's ladder. The second cannot live beside the first — `HostileManifestCorpusTest` is `superplayer-testkit`'s, and a phase 2 module may not depend on a phase 5 one (`docs/modules.md`) — so the loop that plays an entry and says how far it got moves into testkit's main sources as `HostileObservation.observe`, with the outcome vocabulary as the public `HostileOutcome`. One observer rather than two copies, because otherwise a difference between the two tables would be a difference between two observers as much as between two players. `isTyped` now also recognises `SuperPlayerError`, which #183 made the cause a failure nothing rescued ends on: a session named that way is named exactly as `PRD.md` Part 4 asks, so it belongs in `FAILS_TYPED` with core's own two detections. It moves no row here — a player with no resilience has nobody to ask and produces none — which is itself one more count of ADR-0011 rule 14. RECORDED and GRADED are unchanged, row for row. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HeTWwmK1KcFMrfSAre23GS
…ne nothing rescues Phase 5's exit criterion as a test rather than a claim (`PRD.md` Part 4: every injected fault either recovers or produces a typed, actionable error; zero unclassified errors). Four parts. `FaultSweepTest` plays every `FaultScript` fault kind under HLS and under DASH — twenty sessions on a player a consumer builds, a profile and `Resilience.standard()` and nothing else — and records how each ended in SWEPT. The kinds are read off `FaultScript.Builder` by reflection, so a kind added later is visibly missing rather than silently unswept. Every cell recovered or ended in a named class: latency, a throughput cap and a cached on-demand manifest play on; a 500, a DNS failure, a TLS failure, a reset and a timeout end as `Transient.Network`; an unrefreshable token ends as `Transient.CdnEdge`. One kind answers differently by protocol, and it is Media3's answer rather than the ladder's: a truncated HLS segment is a failed load, while the fragmented-MP4 extractor reads the truncated DASH segment's boxes, finds no samples and moves on. Recorded rather than evened out. `HostileManifestLadderTest` is the hostile corpus on a player with the ladder, beside `HostileManifestCorpusTest`'s core-only tables and taken by the same observer. **No row moves**, and that is the finding rather than a disappointment: every entry there is a manifest that lies about media the origin then serves perfectly, so no load fails and the ladder is never asked a question. The two typed rows are core's own detections (rule 4) reaching rung 6 with the likely cause still on them. One entry neither recovers nor ends named — `dash-availability-start-time-skew`, whose `availabilityStartTime` an hour ahead means every segment is legitimately not published yet, so the player waits correctly and for ever — and it is recorded in CANNOT_RECOVER with that reason. Diagnosing it needs `PRD.md` §3.6's doctor reading a manifest against a clock, which is Phase 9's, not a rung reading a failure. `FallbackRungCoverageTest` is the register `PRD.md` Part 5 asks for: per `FallbackRung`, the test that forces that rung and the test that counts what a player without resilience pays for it. They are names rather than code because rungs 4 and 5 are forced in `superplayer-core`, whose test classes are not on this module's test classpath; the names are checked against the source tree, so a rename, a method that is no longer a `@Test`, or a seventh rung with no entry fails here. Rule 14's count grows the third slot: `SuperPlayerResilienceSeamTest` now asserts `playerStateRungs` is empty on a core-only player and filled on one with resilience. That slot is the one whose emptiness is a *registration* that does not happen (rule 13's addendum), and the test's KDoc, written before the slot existed, said there was nothing to register. The golden traces in `superplayer-telemetry` are unchanged, which is the rule's other half. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HeTWwmK1KcFMrfSAre23GS
A synthetic segment is 2 s, so 20 s of injected latency is ten of them rather than the four the comment claimed, and the stream the sweep plays is 12 s of media. Comments only. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HeTWwmK1KcFMrfSAre23GS
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #184
PRD.mdPart 4's Phase 5 criterion — every injected fault either recovers or produces a typed,actionable error. Zero unclassified errors — as three tests and one added count, plus the honest
list of what still cannot recover.
1. The fault sweep —
superplayer-resilience/…/FaultSweepTest.ktEvery
FaultScriptfault kind, under HLS and DASH, on a player a consumer builds (a profile andResilience.standard(), one source, noHeaderProvider). The kinds are read offFaultScript.Builderby reflection, so a kind added later is visibly missing rather thansilently unswept. Twenty sessions, all twenty recovered or ended named, none unclassified, none
stalled:
addLatencyMscapThroughputBpsfailWithHttpStatus(500)Transient.NetworkTransient.NetworktruncateAfterBytesTransient.NetworkfailDnsResolutionTransient.NetworkTransient.NetworkfailTlsHandshakeTransient.NetworkTransient.NetworkresetConnectionTransient.NetworkTransient.NetworktimeOutConnectionTransient.NetworkTransient.NetworkexpireTokenAtSegment(not refreshable)Transient.CdnEdgeTransient.CdnEdgeserveThroughCache(VOD manifest)The one asymmetry is Media3's answer rather than the ladder's: a truncated HLS segment is a failed
load, while the fragmented-MP4 extractor reads the truncated DASH segment's opening boxes, finds no
samples and moves on to the next one. Recorded rather than evened out — the criterion asks that each
end recovered or named, and each does.
2. The hostile corpus, re-recorded — and why the diff is empty
Where it had to go.
HostileManifestCorpusTestissuperplayer-testkit's, and a phase 2 modulemay not depend on a phase 5 one (
docs/modules.md), so the ladder cannot be installed in that fileat all. The ladder record is therefore a second table,
HostileManifestLadderTest.WITH_LADDERinsuperplayer-resilience, and both tables are taken by one observer —HostileObservation.observe, moved into testkit's main sources — so the only difference betweenthem is the
PlaybackResilienceargument.Row by row: nothing moved. All 15 rows of
WITH_LADDERequalRECORDED, andRECORDEDandGRADEDare unchanged row for row. That is a finding, not a shortfall: every corpus entry is amanifest that lies about media the origin then serves perfectly, so no load fails and the ladder is
never asked a question. The two named rows are core's own detection (ADR-0011 rule 4) reaching rung 6
with the likely cause still on them —
hls-cached-live-playlist(StaleLivePlaylistException,intermediary cache) and
dash-short-time-shift-buffer-depth(LiveWindowTooShortException). Thecorpus that does move the ladder is the injected one, above;
PRD.mdPart 4 asks for both becausethey are different kinds of defect.
isTypeddid needSuperPlayerError. #183 made it rung 6's cause, and a session named that wayis named exactly as the criterion asks, so it joins
StaleLivePlaylistExceptionandLiveWindowTooShortExceptioninFAILS_TYPED. It moves no core-only row — a player withoutresilience produces none — which is one more count of rule 14.
3. What still cannot recover
One entry, recorded in
CANNOT_RECOVERwith its reason rather than hidden:dash-availability-start-time-skew—NEVER_STARTS. AnavailabilityStartTimean hour aheadof the clock means every segment the manifest names is legitimately not published yet, so the
player waits, correctly, for ever. No load fails: there is nothing to retry, no other host to ask,
and no failure to classify. Diagnosing it needs a doctor reading a manifest against a clock
(
PRD.md§3.6'sMediaSourceDoctor, Phase 9), not a rung reading a failure.nothingListedAsBeyondTheLadderIsQuietlyPassingkeeps that list honest in the other direction: anentry that starts recovering must leave the list. And
noEntryEndsOnAFailureNothingNamedholds forevery entry without exception — a player with the module never ends a session on an unclassified
PlaybackException.Proposed follow-up issue (not filed): Diagnose a manifest whose media is not published yet —
give Phase 9's doctor the availability-window check, so a session that will never start says so
instead of spinning. Also worth a line in that issue: today the library has no way to report
"waiting, and here is why", since rung 6 exists only for failures.
4. Every rung has a forcing test, named in one place
FallbackRungCoverageTestnames, perFallbackRung, the test that forces that rung and the testthat counts what a player built without resilience pays for it:
RETRY_SAME_URLRetryPlaybackTest.aFaultThatRelentsIsRetriedAndPlaybackGoesOn…KeepsMedia3SOwnLoadErrorHandlingNEXT_HOSTFallbackPlaybackTest.aLocationThatFailsEveryAttemptIsLeftForTheOneTheManifestAlsoNames…IsLeftToMedia3SOwnFallbackTableEXCLUDE_VARIANTFallbackPlaybackTest.aRenditionThatKeepsFailingIsExcludedAndPlaybackGoesOnAtAnotherBitrate…IsLeftToMedia3SOwnFallbackTableNEXT_SOURCENextSourcePlaybackTest.aSourceThatFailsEveryRungBelowIsLeftForTheNextOneInTheRequest…OpensTheFirstSourceAndNothingElseRECREATE_DECODERSuperPlayerDecoderRecreationTest.aFailureTheLadderAnswersWithARecreatedDecoderIsRePreparedAndPlaysOn…RePreparesNothingTYPED_ERRORTypedErrorPlaybackTest.aSessionNothingRescuedEndsOnATypedError…EndsOnMedia3sOwnErrorUnchangedThey are names rather than code because rungs 4 and 5 are forced in
superplayer-core, whose testclasses are not on this module's test classpath. The names are checked against the source tree (the
file exists; the method is declared with a
@Testabove it and no declaration in between), followingsuperplayer-abr'sNoDeviceModelStringTest. A rename, a method demoted from@Test, or a seventhrung added with no entry fails there rather than leaving a rung silently unforced.
5. Rule 14, counted
SuperPlayerResilienceSeamTestnow assertsEngineConfiguration.playerStateRungsis null on a core-only player and non-null on one with resilience. That is the slot whose emptiness
rule 14 rests on since Fallback ladder rung 4: the next source in MediaRequest.sources, at the position already reached #181: core registers its player-error listener only where it is filled, so
an empty slot is a listener that does not exist. The test's KDoc, written before the slot existed,
claimed there was nothing to register; it now says what is registered and where the behaviour half
is counted.
DefaultLoadErrorHandlingPolicyin force is counted as behaviour inRetryPlaybackTest.aPlayerWithoutResilienceKeepsMedia3SOwnLoadErrorHandling.PlaybackFailure.classificationnull —TypedErrorPlaybackTestonAPlayerWithoutResilienceTelemetryReportsExactlyWhatItReportedBefore(Rung 6: surface a typed, actionable error, and let telemetry report the classification #183).superplayer-telemetry/src/test/golden/areunchanged;
./gradlew updateGoldenTraceswas not run, and nothing in the diff touches them.Review
Standards. Clean-room: no algorithm added; the one heuristic (finding a
@Testabove adeclaration) is documented where it lives. ADR-0011: the sweep and both corpus tables drive the
library through its public API only, and the new testkit API names no Media3 type —
updateApiSurfacerun, diff committed (
HostileObservation,HostileOutcome).docs/modules.md's direction isrespected, and is the reason the ladder record is where it is. Smells considered: the shared observer
exists precisely to avoid Duplicated Code across two modules;
FallbackRungCoverageTest'sstring-named tests are a deliberate trade (a reflective form is impossible across module test
classpaths) and are validated rather than trusted;
HostileObservation.STEP_MSandREADY_WINDOW_MSare public though only the enum's KDoc and the corpus's bound read them — kept public because a
reader of either table needs the window's width to read it.
Spec. All six acceptance criteria are met, with one deviation stated rather than glossed: "…
recorded tables are regenerated with the ladder in place" could not be done inside
HostileManifestCorpusTestfor the module-phase reason above; the tables were re-run (unchanged) andthe ladder's record is the sibling table, with
docs/testing.mdupdated to say a new pathology nowgets a row in three tables rather than two.
What was run, and what was skipped
./gradlew assemble checkgreen (≈40 s cold). The three new test classes cost about 5 s together —the harness's fake clock makes 20 swept sessions and 15 corpus entries cheap, so
checkcan affordthem. Emulator and demo runs skipped (host memory), as the batch instructed; nothing here touches
runtime code, only tests, one test-support file and docs.
CI: https://github.com/ramesh130/superplayer/actions/runs/35069057888 — success (dispatched on
issue-184-phase-exit;.github/workflows/ci.ymlisworkflow_dispatch:only, so nothing runs on the PR itself).🤖 Generated with Claude Code
https://claude.ai/code/session_01HeTWwmK1KcFMrfSAre23GS