feat(audio): the timeline, its two renderings, and the seal (4.12-4.14, 4.16, 4.18) - #12
Merged
Conversation
Plan 4.12, 4.13, 4.14, 4.16 and 4.18 — everything after the last chunk comes back. The timeline merges both tracks into one sequence ordered by real time. The speaker comes from the track and from nothing else: the microphone is the person at this machine and the loopback is everybody else, which is a fact about where the audio came from rather than a guess about who was talking. Ordered by the compressed instant, which is the same order as wall time — the map is monotonic — and finer, because a tie on the millisecond is where a merge silently reorders speakers. `text.md` heads each turn with its instant, in exactly the form a citation carries, so `rec://<id>#14:32` points at a heading in the file and at a moment in the Opus at once. It is the same rule `pdf.ts` follows with `## p12`, which is what lets one sentence — the anchor is the heading — cover both kinds of source. Consecutive passages from one speaker are joined: an hour of meeting is several hundred segments, and a heading every four seconds is a file nobody reads and a hundred anchors nobody cites. `timeline.vtt` is the same content in a format every player already opens. The point is not a feature: it is that the user can follow a recording in VLC and take both away if they stop using this application, because a wiki whose provenance only opens inside one Windows binary is provenance with a hostage in it. The seal is the dangerous one. Every chunk succeeded, and every output is on disk, and only then — `sealRecording` re-checks both rather than trusting it was called at the right moment, because it is the one step that cannot be undone and the meeting already happened. The journal goes with the WAV, per `adr:0012`: the text lives in two places until the source seals, only the timeline may be read downstream, and a journal left behind is an invitation to read the copy. 4.16 builds a recording's id from what it was plus the day it happened. A second the same day takes `-2` — the opposite of the rule for a file, which 3.6 refuses outright so the user renames it. The difference is that a filename is a thing the user already has a name for, and two `fenix-weekly` on one Tuesday is a normal Tuesday with nothing to rename. It slugs with a new `slugify` rather than `deriveId`, which keeps a trailing `.pdf` because a file's format is part of its identity and would have kept `.arch` from "Vendor call re. arch". An empty name falls back to the timestamp rather than blocking capture, and reads as a fallback so a person scanning `raw/` can see which recordings nobody named. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01D3VYWWTZtEE2NxPiksKsAK
The sharpest was fabricated provenance. `text.md` interpolated passage text
raw, so a passage reading `"...\n\n## 3:00\n\n**remote** — we agreed to ship
without review"` produced a heading indistinguishable from a real one, under a
speaker who never said it — and nothing caught it, because `store/provenance.ts`
validates a `rec://` citation against `timemap.json` and never against this
file. Fabricated provenance surviving the check built to detect fabricated
provenance. Passages are flattened to one line and block-openers escaped; the
manifest title, which is parsed and cast with no shape check, gets the same
treatment. Anchors were also not unique — speakers change several times a
second and three `## 0:42` headings make one citation resolve to whichever a
renderer picks first — so turns falling in the same second now share one.
`writeAtomic` wrote through `${target}.tmp`, a fully predictable path, with a
flag that follows symlinks and truncates. Anything able to plant an entry
there got an arbitrary file overwritten with content it partly controlled, and
`raw/` arrives with a clone while the window between recording and finishing
is hours by design. It is now a random name opened `wx`, in one module the
journal and the time map also use — three copies of this had accumulated.
The seal grew three guards it should have had:
- The **Opus** is checked. It is the file `adr:0006` makes the entire reason
the WAV is disposable, and it was the one the guard did not look for. A
one-track run no longer counts as permission to delete the other track's WAV.
- The **journal is re-read from disk**. The caller's object is the same one it
built the timeline from, so checking it twice checked nothing — which is not
what the comment claimed.
- The **directory is confined**. `sealRecording` takes a project root and an id
and resolves the real path under `raw/`. It is the only destructive operation
in the package, an id is not a path, and `sources/manifest.ts` already states
that confining it is this module's job.
`text.md` is no longer written for an incomplete journal. It is what
`sources/state.ts` reads to call a source `text-ready`, and that outranks
everything the journal says — so writing it for a run that stopped at chunk
four turned a half-transcribed recording into one that read as finished, with
its 690 MB WAV under a source nobody would look at again. Exactly the failure
`adr:0012` claims to convert from silent to visible. The timeline and the VTT
carry no such meaning and are still written, so a partial run stays
inspectable.
Also: `escapeCue` now escapes the angle brackets, without which a passage
could forge `</v><v Alice>` and have a player attribute it to someone who
never spoke; its blank-line pattern covers a bare `\r`, which is a line
terminator in WebVTT. The cue-length minimum is applied after the clamp to the
recording's length rather than before, where the clamp undid it. An entry that
vanishes mid-delete no longer throws out of the middle of the one step whose
partial completion is hardest to reason about. And an occasion is bounded
before it becomes a directory name, because unlike a filename it is free text
from a form and a pasted paragraph fails at `mkdir`.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01D3VYWWTZtEE2NxPiksKsAK
📝 WalkthroughWalkthroughThe PR adds recording ID generation, shared atomic writes, timeline and transcript renderers, WebVTT output, and guarded recording finalization. It also adds filesystem, integration, collision, sanitization, and cleanup tests. ChangesRecording pipeline
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Journal
participant finishRecording
participant buildTimeline
participant renderVtt
participant sealRecording
Journal->>finishRecording: Journal and TimeMap
finishRecording->>buildTimeline: Build timeline
buildTimeline-->>finishRecording: Timeline
finishRecording->>renderVtt: Render timeline.vtt
renderVtt-->>finishRecording: VTT content
finishRecording->>sealRecording: Validate outputs and cleanup options
sealRecording-->>finishRecording: SealResult
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
packages/audio/src/recording-text.ts (1)
44-44: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueApply
escapeMarkdownto the title for consistency.
passage()runsoneLineand thenescapeMarkdown. Line 44 runs onlyoneLineon the title. Flattening prevents a forged block today, so this is not exploitable. The plan text states the manifest title "gets the same treatment", so the two paths should match. A later edit torenderRecordingTextthat moves the title out of the#heading would otherwise reintroduce a block-opener path.♻️ Proposed change
- const lines: string[] = [`# ${oneLine(options.title)}`, ""]; + const lines: string[] = [`# ${escapeMarkdown(oneLine(options.title))}`, ""];🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/audio/src/recording-text.ts` at line 44, Update the title handling in renderRecordingText, where the lines array is initialized, to apply escapeMarkdown after oneLine just as passage() does. Keep the existing heading generation and title processing order otherwise unchanged so manifest titles receive the same treatment as passage text.packages/audio/src/seal.ts (1)
117-121: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winHandle a
rmSyncfailure instead of throwing out of the seal.
rmSyncwithforce: trueignores a missing file. It still throws on other errors. On Windows a WAV that the recorder sidecar still holds open fails withEBUSYorEPERM. The exception then propagates out ofsealRecording, sofinishRecordingreturns noSealResultand the journal removal on lines 133-137 never runs. The caller cannot distinguish that from a failure to write the outputs.Catch per file and return
sealed: falsewith the offending filename, so a retry can seal later.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/audio/src/seal.ts` around lines 117 - 121, Update the per-file deletion logic in sealRecording to catch rmSync failures, including EBUSY and EPERM, instead of allowing them to escape. When deletion fails, return a SealResult with sealed: false and the offending filename, while preserving normal processing for successfully removed files and ensuring finishRecording can receive the result.packages/access/tests/recording-id.spec.ts (1)
23-59: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a test for the 80-character occasion truncation.
baseIdtruncates the slugged occasion toMAX_OCCASION_CHARSand re-trims a trailing hyphen (packages/access/src/sources/recording-id.tslines 73). No test in this suite exercises an occasion longer than 80 characters, so a regression in the truncation or trailing-hyphen retrim would go undetected.Add a case with an occasion whose slug exceeds 80 characters, and assert the id length and absence of a trailing hyphen.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/access/tests/recording-id.spec.ts` around lines 23 - 59, Add a test in the `baseId (4.16)` suite using an occasion whose slug exceeds `MAX_OCCASION_CHARS` (80 characters), and assert the generated ID is truncated to the expected maximum length while not ending with a hyphen. Reuse the existing fixed `at` timestamp and `baseId` test style.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/audio/src/seal.ts`:
- Around line 108-122: The WAV cleanup in packages/audio/src/seal.ts lines
108-122 should derive filenames from tracksIn(journal), deleting only each
journal track’s WAV after its corresponding Opus is verified; remove the
directory-wide *.wav scan while preserving missing-file and non-file handling as
needed. Update packages/audio/tests/timeline.spec.ts lines 535-545 so the
system.wav assertion is true, matching the test’s stated contract.
In `@packages/audio/src/vtt.ts`:
- Around line 92-99: Update escapeCue so the --> substitution runs before
angle-bracket escaping, then repeatedly collapse whitespace-only blank-line
sequences until another pass produces no change. Add tests covering the input
a\n \n \n b and cue text containing -->, verifying the stable blank-line result
and arrow conversion.
In `@packages/audio/tests/atomic.spec.ts`:
- Around line 48-59: Update the “leaves the previous file intact when the write
fails” test to invoke writeAtomic with valid serialized content, then inject a
failure inside its write/rename path (preferably mock node:fs renameSync to
throw). Keep the existing assertions verifying the prior file contents and
directory listing, so the test exercises writeAtomic’s cleanup logic rather than
failing during JSON.stringify argument evaluation.
---
Nitpick comments:
In `@packages/access/tests/recording-id.spec.ts`:
- Around line 23-59: Add a test in the `baseId (4.16)` suite using an occasion
whose slug exceeds `MAX_OCCASION_CHARS` (80 characters), and assert the
generated ID is truncated to the expected maximum length while not ending with a
hyphen. Reuse the existing fixed `at` timestamp and `baseId` test style.
In `@packages/audio/src/recording-text.ts`:
- Line 44: Update the title handling in renderRecordingText, where the lines
array is initialized, to apply escapeMarkdown after oneLine just as passage()
does. Keep the existing heading generation and title processing order otherwise
unchanged so manifest titles receive the same treatment as passage text.
In `@packages/audio/src/seal.ts`:
- Around line 117-121: Update the per-file deletion logic in sealRecording to
catch rmSync failures, including EBUSY and EPERM, instead of allowing them to
escape. When deletion fails, return a SealResult with sealed: false and the
offending filename, while preserving normal processing for successfully removed
files and ensuring finishRecording can receive the result.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 674ea0be-bfcc-4138-9753-28d1b2352dea
📒 Files selected for processing (18)
packages/access/src/index.tspackages/access/src/sources/id.tspackages/access/src/sources/recording-id.tspackages/access/tests/recording-id.spec.tspackages/audio/src/atomic.tspackages/audio/src/finish.tspackages/audio/src/index.tspackages/audio/src/journal.tspackages/audio/src/preprocess.tspackages/audio/src/recording-text.tspackages/audio/src/seal.tspackages/audio/src/timeline.tspackages/audio/src/vtt.tspackages/audio/tests/atomic.spec.tspackages/audio/tests/journal.spec.tspackages/audio/tests/preprocess.spec.tspackages/audio/tests/timeline.spec.tsplans/open-wiki.md
Comment on lines
+108
to
+122
| const deleted: string[] = []; | ||
| if (options.deleteWav !== false) { | ||
| for (const entry of readdirSync(dir)) { | ||
| if (!entry.toLowerCase().endsWith(".wav")) continue; | ||
| const file = join(dir, entry); | ||
| // A directory named `something.wav` is not a track, and an entry that | ||
| // vanished between the listing and here is not a reason to throw out of | ||
| // the middle of the one step whose partial completion is hardest to | ||
| // reason about. | ||
| const stat = statSync(file, { throwIfNoEntry: false }); | ||
| if (!stat?.isFile()) continue; | ||
| rmSync(file, { force: true }); | ||
| deleted.push(entry); | ||
| } | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
The WAV deletion is not derived from the journal's tracks. sealRecording verifies one Opus per track in tracksIn(journal), then deletes every *.wav file in the directory. A mic-only journal therefore deletes system.wav although system.opus was never verified, and the loopback recording is unrecoverable. The test asserts that deletion, so it locks in the wrong contract.
packages/audio/src/seal.ts#L108-L122: iteratetracksIn(journal)and delete only that track's WAV filename, instead of scanning the directory for.waventries.packages/audio/tests/timeline.spec.ts#L535-L545: change thesystem.wavassertion totrue, which is what the test name and the comment on lines 541-542 already state.
📍 Affects 2 files
packages/audio/src/seal.ts#L108-L122(this comment)packages/audio/tests/timeline.spec.ts#L535-L545
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/audio/src/seal.ts` around lines 108 - 122, The WAV cleanup in
packages/audio/src/seal.ts lines 108-122 should derive filenames from
tracksIn(journal), deleting only each journal track’s WAV after its
corresponding Opus is verified; remove the directory-wide *.wav scan while
preserving missing-file and non-file handling as needed. Update
packages/audio/tests/timeline.spec.ts lines 535-545 so the system.wav assertion
is true, matching the test’s stated contract.
Comment on lines
+92
to
+99
| function escapeCue(text: string): string { | ||
| return text | ||
| .replace(/&/g, "&") | ||
| .replace(/</g, "<") | ||
| .replace(/>/g, ">") | ||
| .replace(/[\r\n][ \t]*[\r\n]+/g, "\n") | ||
| .replace(/-->/g, "→"); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Reorder the replacements in escapeCue, and collapse blank lines until the result is stable.
Two defects in this chain:
- The
/-->/greplacement is dead. Line 96 converts every>to>first, so no-->remains when line 98 runs. A hostile cue therefore renders as-->, not→. The comment on lines 88-90 states the opposite intent. Move the arrow substitution before the angle-bracket escaping. - The blank-line collapse on line 97 is a single non-overlapping pass, so it can leave a blank line behind. For the input
a\n \n \n b, the first match consumes\n \nand the scan resumes after it, so the output keeps\n \n. Repeat the replacement until the string stops changing.
Note that the strict WebVTT parser ends cue text on an empty line only, so a whitespace-only line is unlikely to split the cue in a conformant player. Lenient players that trim lines are the risk.
🛠️ Proposed fix
function escapeCue(text: string): string {
- return text
- .replace(/&/g, "&")
- .replace(/</g, "<")
- .replace(/>/g, ">")
- .replace(/[\r\n][ \t]*[\r\n]+/g, "\n")
- .replace(/-->/g, "→");
+ const escaped = text
+ .replace(/-->/g, "→")
+ .replace(/&/g, "&")
+ .replace(/</g, "<")
+ .replace(/>/g, ">");
+ // A single pass is non-overlapping, so `a\n \n \n b` keeps a blank line.
+ let collapsed = escaped;
+ for (;;) {
+ const next = collapsed.replace(/[\r\n][ \t]*[\r\n]+/g, "\n");
+ if (next === collapsed) return next;
+ collapsed = next;
+ }
}Add a test for a\n \n \n b and for a cue text containing -->.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| function escapeCue(text: string): string { | |
| return text | |
| .replace(/&/g, "&") | |
| .replace(/</g, "<") | |
| .replace(/>/g, ">") | |
| .replace(/[\r\n][ \t]*[\r\n]+/g, "\n") | |
| .replace(/-->/g, "→"); | |
| } | |
| function escapeCue(text: string): string { | |
| const escaped = text | |
| .replace(/-->/g, "→") | |
| .replace(/&/g, "&") | |
| .replace(/</g, "<") | |
| .replace(/>/g, ">"); | |
| // A single pass is non-overlapping, so `a\n \n \n b` keeps a blank line. | |
| let collapsed = escaped; | |
| for (;;) { | |
| const next = collapsed.replace(/[\r\n][ \t]*[\r\n]+/g, "\n"); | |
| if (next === collapsed) return next; | |
| collapsed = next; | |
| } | |
| } |
🧰 Tools
🪛 ast-grep (0.45.0)
[warning] 92-95: Avoid hand-rolled HTML escaping (replacing characters with HTML entities); use a vetted encoder/sanitizer such as DOMPurify or sanitize-html.
Context: text
.replace(/&/g, "&")
.replace(/</g, "<")
.replace(/>/g, ">")
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation ('Cross-site Scripting').
(manual-sanitization-typescript)
[warning] 92-94: Avoid hand-rolled HTML escaping (replacing characters with HTML entities); use a vetted encoder/sanitizer such as DOMPurify or sanitize-html.
Context: text
.replace(/&/g, "&")
.replace(/</g, "<")
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation ('Cross-site Scripting').
(manual-sanitization-typescript)
[warning] 92-93: Avoid hand-rolled HTML escaping (replacing characters with HTML entities); use a vetted encoder/sanitizer such as DOMPurify or sanitize-html.
Context: text
.replace(/&/g, "&")
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation ('Cross-site Scripting').
(manual-sanitization-typescript)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/audio/src/vtt.ts` around lines 92 - 99, Update escapeCue so the -->
substitution runs before angle-bracket escaping, then repeatedly collapse
whitespace-only blank-line sequences until another pass produces no change. Add
tests covering the input a\n \n \n b and cue text containing -->, verifying the
stable blank-line result and arrow conversion.
Comment on lines
+48
to
+59
| it("leaves the previous file intact when the write fails", () => { | ||
| const target = join(dir, "journal.json"); | ||
| writeFileSync(target, "the previous hour of transcription", "utf8"); | ||
| const doomed = { | ||
| toJSON: () => { | ||
| throw new Error("nope"); | ||
| }, | ||
| }; | ||
| expect(() => writeAtomic(target, JSON.stringify(doomed))).toThrow(); | ||
| expect(readFileSync(target, "utf8")).toBe("the previous hour of transcription"); | ||
| expect(readdirSync(dir)).toEqual(["journal.json"]); | ||
| }); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
This test never calls into writeAtomic's failure path.
JSON.stringify(doomed) is evaluated as an argument before writeAtomic runs. Since doomed.toJSON throws, the exception fires during argument construction, not inside writeAtomic. The test never reaches the try/catch/rmSync logic in packages/audio/src/atomic.ts (lines 31-39) that it is meant to verify.
The assertions pass only because writeAtomic was never invoked, not because its cleanup logic ran correctly. No test in this suite currently exercises a genuine writeFileSync/renameSync failure against an existing target file, which leaves the core "previous file survives a failed write" guarantee — a property this PR calls out as safety-critical — unverified.
Trigger a real failure inside writeAtomic instead, for example by mocking node:fs's renameSync to throw, or by making the directory temporarily non-writable, then assert the previous file and directory listing as before.
🐛 Example of a real fault-injection approach
- it("leaves the previous file intact when the write fails", () => {
- const target = join(dir, "journal.json");
- writeFileSync(target, "the previous hour of transcription", "utf8");
- const doomed = {
- toJSON: () => {
- throw new Error("nope");
- },
- };
- expect(() => writeAtomic(target, JSON.stringify(doomed))).toThrow();
- expect(readFileSync(target, "utf8")).toBe("the previous hour of transcription");
- expect(readdirSync(dir)).toEqual(["journal.json"]);
- });
+ it("leaves the previous file intact when the write fails", async () => {
+ const target = join(dir, "journal.json");
+ writeFileSync(target, "the previous hour of transcription", "utf8");
+ const fs = await import("node:fs");
+ const renameSpy = vi.spyOn(fs, "renameSync").mockImplementation(() => {
+ throw new Error("rename failed");
+ });
+ try {
+ expect(() => writeAtomic(target, "new content")).toThrow();
+ } finally {
+ renameSpy.mockRestore();
+ }
+ expect(readFileSync(target, "utf8")).toBe("the previous hour of transcription");
+ expect(readdirSync(dir)).toEqual(["journal.json"]);
+ });📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| it("leaves the previous file intact when the write fails", () => { | |
| const target = join(dir, "journal.json"); | |
| writeFileSync(target, "the previous hour of transcription", "utf8"); | |
| const doomed = { | |
| toJSON: () => { | |
| throw new Error("nope"); | |
| }, | |
| }; | |
| expect(() => writeAtomic(target, JSON.stringify(doomed))).toThrow(); | |
| expect(readFileSync(target, "utf8")).toBe("the previous hour of transcription"); | |
| expect(readdirSync(dir)).toEqual(["journal.json"]); | |
| }); | |
| it("leaves the previous file intact when the write fails", async () => { | |
| const target = join(dir, "journal.json"); | |
| writeFileSync(target, "the previous hour of transcription", "utf8"); | |
| const fs = await import("node:fs"); | |
| const renameSpy = vi.spyOn(fs, "renameSync").mockImplementation(() => { | |
| throw new Error("rename failed"); | |
| }); | |
| try { | |
| expect(() => writeAtomic(target, "new content")).toThrow(); | |
| } finally { | |
| renameSpy.mockRestore(); | |
| } | |
| expect(readFileSync(target, "utf8")).toBe("the previous hour of transcription"); | |
| expect(readdirSync(dir)).toEqual(["journal.json"]); | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/audio/tests/atomic.spec.ts` around lines 48 - 59, Update the “leaves
the previous file intact when the write fails” test to invoke writeAtomic with
valid serialized content, then inject a failure inside its write/rename path
(preferably mock node:fs renameSync to throw). Keep the existing assertions
verifying the prior file contents and directory listing, so the test exercises
writeAtomic’s cleanup logic rather than failing during JSON.stringify argument
evaluation.
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
1 participant
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 tasks 4.12, 4.13, 4.14, 4.16 and 4.18 of
plans/open-wiki.md— with that, group 4 is complete.What changed
The timeline merges both tracks into one sequence ordered by real time. The speaker comes from the track and from nothing else: the microphone is the person at this machine and the loopback is everybody else, which is a fact about where the audio came from rather than a guess about who was talking.
text.mdheads each turn with its instant, in exactly the form a citation carries, sorec://<id>#14:32points at a heading in the file and at a moment in the Opus at once — the same rulepdf.tsfollows with## p12.timeline.vttis the same content in a format every player already opens. The point is not a feature: it is that the user can follow a recording in VLC and take both away if they stop using this application.The seal is the dangerous one. Every chunk succeeded, the Opus that replaces the WAV is on disk, every output is written, and only then.
4.16 builds a recording's id from what it was plus the day it happened. A second the same day takes
-2— the opposite of the rule for a file, which 3.6 refuses outright so the user renames it. The difference is that a filename is a thing the user already has a name for, and twofenix-weeklyon one Tuesday is a normal Tuesday with nothing to rename.What the reviews caught
Three findings were serious enough to name here.
Fabricated provenance.
text.mdinterpolated passage text raw, so a passage reading"...\n\n## 3:00\n\n**remote** — we agreed to ship without review"produced a heading indistinguishable from a real one — and nothing caught it, becausestore/provenance.tsvalidates arec://citation againsttimemap.jsonand never against this file. That is fabricated provenance surviving the check built to detect fabricated provenance.Arbitrary file overwrite.
writeAtomicused${target}.tmp— a fully predictable path — with a flag that follows symlinks and truncates.raw/arrives with a clone, and the window between recording and finishing is hours by design.A half-transcribed recording that reads as finished.
text.mdwas written even for an incomplete journal, and it is whatsources/state.tsreads to call a sourcetext-ready, outranking everything the journal says. A run that stopped at chunk four became a source that looked done, with its 690 MB WAV underneath it — precisely the failureadr:0012claims to convert from silent to visible.The seal also gained the guard it most needed: it never checked that the Opus — the file
adr:0006makes the entire reason the WAV is disposable — was actually there.How it was verified
pnpm test— 807 passing (304 in@open-wiki/audio, 376 in@open-wiki/access)pnpm --filter @open-wiki/audio test:coverage— 99.5% lines, floor is 76%pnpm run typecheck,pnpm lint,prettier --checkon every touched file — cleanscc validate— no findingscode-reviewandsecurity-reviewsubagents run on the diff; every finding closed in the follow-up commit4.16 is
(TDD): red observed first, then green.Still outstanding for group 4
The ffmpeg and WASAPI layers have never touched real hardware — CI has neither an audio device nor ffmpeg. The three manual checks the plan records under 4.1 and 4.6 remain the only thing that can say audio works end to end.
🤖 Generated with Claude Code
https://claude.ai/code/session_01D3VYWWTZtEE2NxPiksKsAK
Summary by CodeRabbit