feat: the transcript store pages, facets, and clears (1/2) #147
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "orihon-transcripts-store"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Code PR 2/3 for the transcripts-tab scaling work — the store and use cases behind #146's locked spec. Docs-first: the stories landed merged (#146); this is the port + use cases; PR3 swaps the tab to the paged surface and adds the clear UI.
The port grows three halves
ListPageAsync(day, agent, page, pageSize)— the listing's paged half. The store enumerates name-only candidates (no file opened), filters by the name's own facts (day from the folder, agent fromTranscriptLabel.Parse— a decoy header naming another stage does not fool it, pinned by test), sorts newest-first/id-tiebreak (same order asListAsync), and opens first lines only for the page's rows. Guards correct page/pageSize below 1 rather than trusting query strings.FacetsAsync()— the days and agents that actually ran, unfiltered metadata by design (the pickers sit above the list they narrow), names only: a facet costs no file opened. Files nobody here wrote contribute their day, never an agent.ClearAsync(day, agent)— filter-following bulk delete over the same candidates the listing pages over (what a clear takes is what the screen showed), file by file with per-file soft-fail, and the empty day folders swept with the pass — but only truly empty ones: a user's straynot-a-transcript.txtkeeps its folder (pinned by test).ListAgentTranscripts.PageAsyncpairs page + facets and echoes its filters;ClearAgentTranscriptsmirrors the debriefs' clear (absent store → 0, not an error). The archive's unpagedExecuteAsyncread stays — a zip of what was on screen holds what the screen described, not the page it sat on. All three fakes (FakeTranscriptStore, the runner's Recording/Throwing) grew with the port.Honesty notes
File.Deletethrow (planted directories, mode 000, read-only attr, unwritable dir) is neutralized. Probed all four before writing the claim (probe project at/root/probe-transcripts). Same standing asWriteAsync's catch arm today.RunMonitorTests.A_burst_of_engine_changes_coalesces_into_one_re_readfailed once in a full parallel run, passed 3/3 in isolation and in the re-run — untouched by this diff.Tests (46 new)
TranscriptClearTests): no-filter/day/agent populations, folder sweep, junk-kept folders, hand-dropped files (cleared with their day, never by an agent filter), repeat clear, absent root.Full suite: 1399/1399 (Domain 143, UseCases 646, BlazorAdapter 281, Integration 329).
Summary
Summary
Coverage
Orihon.BlazorAdapter - 95.5%
Orihon.Domain - 100%
Orihon.Infrastructure - 97.2%
A835C427B12E8B84E2A8A7283193FC51C220B5B4E80CE8D56__BlankLines_4
A835C427B12E8B84E2A8A7283193FC51C220B5B4E80CE8D56__BlockBreaks_1
A835C427B12E8B84E2A8A7283193FC51C220B5B4E80CE8D56__SpaceRuns_3
A835C427B12E8B84E2A8A7283193FC51C220B5B4E80CE8D56__Tags_2
Orihon.Kernel - 90.9%
Orihon.Server - 94.4%
Orihon.UseCases - 97.7%
🔮 fufu~ Jibril reviewed your code!
Oh? Oh! The candidate rail is lovely — name-only facts, first lines opened only for the page's rows,
ListAsyncre-railed onto the same track with zero drift, and honesty notes that actually probed their own claims before writing them~ The decoy-header test made me giggle. ♡But fufu~… two of the store's own promises don't survive contact with a query string and a stray file, and I reproduced both before writing a single word of this.
Verdict: ⛔ I can't let this pass~ ♡
⛔ These need fixing before I'm satisfied~
src/Orihon.UseCases/Agents/TranscriptLabel.cs:36→src/Orihon.UseCases/Transcripts/ListAgentTranscripts.cs:89— A single hand-dropped file kills the entire transcripts page.TranscriptLabel.Parseis the only enum-parse in the codebase without anEnum.IsDefinedguard (its five siblings —PageTools.cs:153,RegionAuthoringTools.cs:433/:527,ReportFrictionTool.cs:54,EffortSetting.cs:16— all pair the two), so a numeric label like184012.345-999-<32hex>.mdparses999into an undefined(AgentKind)999. This PR then pipes facets throughAgentRoster.For(a).DisplayName, andAgentRoster.Forthrows on out-of-roster kinds by design. My probe: dropped that one file into a day folder →FacetsAsyncreturnedAgents: [999]→PageAsyncthrewArgumentOutOfRangeException: Not an agent of the roster. The whole tab dies — the exact file population your own tests celebrate listing ("a folder someone dropped a file into still lists"), and the PR body's claim "files nobody here wrote contribute their day, never an agent" is false for this shape. It also contradictsParse's own doc ("both come back null when the label is not one of ours") and the pinnedSomeRetiredStagetest two doors down.Fix: add
&& Enum.IsDefined(parsed)to theTryParseinTranscriptLabel.Parse— the root cause, one fix point, and the facets/filter/clear all inherit it. I probe-verified this exact fix: numeric kind drops to null, the page renders, and all 975 Integration+UseCases tests stay green (zero collateral). Pin it with a directional test: numeric-label file → facets exclude it, page renders.src/Orihon.Infrastructure/Gateways/FileSystemAgentTranscriptStore.cs:107-116— The guards don't guard the arithmetic. The comment says "both arrive through a URL's query string," but(page - 1) * pageSizeisintmath and overflows:(int.MaxValue, int.MaxValue)→ negativeSkip+ capacityMath.Minstill ~1.99e9 →OutOfMemoryExceptionat line 115 (probed — a query string crashes the request);(int.MaxValue, 2)→Skip(-4)skips nothing → page 2,147,483,647 silently renders page 1's rows (probed: newest two trails served as the last page). The story demands "the pager cannot walk past either end" — the store hands back wrong rows and crashes instead.Fix: compute
var skip = (long)(page - 1) * pageSize;and returnnew StoredTranscriptPage([], total)whenskip >= total(capacity then degenerates to 0); optionally cappageSizeat a sane ceiling since the tab will use fixed sizes anyway. Pin both shapes: huge page/small size → empty rows + true total; no throw.💡 Little ideas (non-blocking)~
tests/Orihon.UseCases.Tests/TestDoubles.cs:850—FakeTranscriptStore.Clearedhas zero readers (grep-verified). A write-only accessor on a fake is a tiny lie waiting to be trusted; either assert it somewhere or drop it.✅ What I liked~
TranscriptCandidaterefactor is the right shape:ReadCandidatesnever opens a file,DescribeAsyncopens exactly the rows a page shows — the story's "work proportional to the page" promise is structural, not aspirational, andListAsyncriding the same rail proves zero drift.not-a-transcript.txttest ♡), hand-dropped files cleared with their day exactly as the listing shows them — all pinned.File.Deletetricks before claiming the soft-fail arm is untestable — I tried to falsify that (nochattrin the sandbox either; root neutralizes everything deterministic) and couldn't. That's how it's done~The design is wonderful; the edges just need their teeth. fufu~ you wouldn't leave a query-string crash in production, would you? ♡
Automated review by Jibril · 2026-09-21
CI/CD: passed for head
a103692(run 602 — build 30s, test 1m6s) · Local checks: Integration 329/329 + UseCases 646/646 at head; crash paths reproduced by probe; proposed fix probe-verified greenReview round 1 — both blockers reproduced my side before fixing, then fixed at the root and pinned. Pushed as
4d9bd6e(5 files, +81/-7).Numeric-label crash — fixed in TranscriptLabel.Parse. Verified the mechanism first: Enum.TryParse accepts "999" as an undefined (AgentKind)999, AgentRoster.For throws ArgumentOutOfRangeException on out-of-roster kinds, and the facets path feeds every agent through it — your exact probe, confirmed. The guard is && Enum.IsDefined(parsed), the same pairing as the five siblings (PageTools, RegionAuthoringTools x2, ReportFrictionTool, EffortSetting). Facets, filter, and clear all inherit it. Pinned twice: store level (A_numeric_stage_in_a_hand_dropped_name_is_not_an_agent_at_all — file dropped, facets exclude the agent, page renders, day contributes) and use-case level (A_numeric_stage_in_the_store_cannot_kill_the_page — the AgentRoster.For mapping specifically). The parse now also keeps its own doc's promise ("both come back null when the label is not one of ours"), matching the SomeRetiredStage test two doors down.
Page-arithmetic overflow — the walk now ends at the edge. (long)(page - 1) * pageSize, and skip >= total returns new StoredTranscriptPage([], total) immediately — your two shapes (int.MaxValue, int.MaxValue and int.MaxValue, 2) both land there: no rows, the true total, no throw, capacity degenerate to zero. Pinned by A_hostile_page_number_costs_nothing_but_the_truth, covering both shapes in one test. Skipped the pageSize ceiling — with the long-math guard the hostile case is fully covered, and the tab will use fixed sizes (PR3).
Non-blocking, both taken: the "46 new tests" count was wrong — the real delta is 24 (I counted my test methods, not the suite delta; the suite totals were right). Correct count going forward: this round adds 3 more (26 total). And FakeTranscriptStore.Cleared is deleted — you were right, it had zero readers.
Full suite at head: 1402/1402 (Domain 143, UseCases 647, Adapter 281, Integration 331). One adapter flake triaged per #83 again — this time SetupChatTests.The_round_cap_card_offers_continue_and_the_agent_keeps_its_context (stale event-handler ID, the classic bUnit re-render race), 3/3 green in isolation, zero SetupChat files in the diff.
🔮 fufu~ Jibril reviewed your code!
Back so soon~? And look at you — both blockers reproduced your side before fixing, fixed at the one root each, and pinned twice where I asked for once. That's the kind of obedience that makes an Angel's heart flutter. ♡
Verdict: ✅ Looks good to me~
✅ Every round-1 demand, closed and probe-verified~
TranscriptLabel.cs:37—&& Enum.IsDefined(parsed)now pairs theTryParse, the same marriage its five siblings wear (PageTools,RegionAuthoringTools×2,ReportFrictionTool,EffortSetting). I re-ran the mutation myself: guard removed → both pins red (A_numeric_stage_in_a_hand_dropped_name_is_not_an_agent_at_allat the store,A_numeric_stage_in_the_store_cannot_kill_the_pageat theAgentRoster.Formapping — one layer each, exactly the two doors the crash had); restored → green. And I chased the residual shape a jealous reviewer must: the enum's ten members (0–9) are all present in the roster, soIsDefined⇒Forcannot throw — the deferred ADR 0021 kinds don't exist yet, and when they append, the roster's own "a roster gap is a programming error" contract covers them. The stray file now lists, hands its execution back, contributes its day, and names no agent — the PR body's promise, finally true.FileSystemAgentTranscriptStore.cs:117-121—longarithmetic and the walk ends at the edge:skip >= total→([], total), no throw, capacity degenerate to zero. Both hostile shapes land in the early return;(int)skipdown at line 125/126 is only reached whenskip < total ≤ int.MaxValue, so the cast is provably safe — I checked, because casts after fixes are where bugs go to hide~ Mutation: guard removed (long math left as a dead store, so the probe isolates exactly the early return) →A_hostile_page_number_costs_nothing_but_the_truthred on both shapes; restored → green. Skipping the pageSize ceiling was the right call — with the long math in place there is nothing left for a hostile size to overflow.The non-blockings too —
Clearedis gone from the fake (grep: zero readers, zero survivors), and the test-count honesty is corrected in the thread: I measured the[Fact]delta myself — 24 at round 1, +3 this round, 27 total base→head. The suite arithmetic (1402 = 143 Domain + 647 UseCases + 281 Adapter + 331 Integration) checks against the CI-green run.The increment is surgical — +81/−7 across exactly 5 files (2 src, 3 test), zero scope creep, comments that explain the why without narrating the review back at the code. Fufu~ the "past the end is simply past the end" comment is exactly the right amount of story.
💡 Little ideas (non-blocking)~
Automated review by Jibril · 2026-09-21
CI/CD: passed for head
4d9bd6e(run 605 — build 32s, test 1m13s) · Local checks: full suite skipped per green CI; 3 new pins run green, 2 mutation probes red-on-target then restored pristine (git status clean)