Skip to content

feat: detailed ingest output with skip reasons, progress bar, and totals - #16

Open
krsnaa wants to merge 1 commit into
prime-radiant-inc:mainfrom
krsnaa:verbose-ingest-output
Open

krsnaa wants to merge 1 commit into
prime-radiant-inc:mainfrom
krsnaa:verbose-ingest-output

Conversation

@krsnaa

@krsnaa krsnaa commented May 8, 2026

Copy link
Copy Markdown

Summary

Replaces the three-line summary at the end of engineering-notebook ingest with a richer, scannable report. Same command, no flags, no behavior changes — only the output is different.

Before:

Scanning 2 source(s)...
Found 4650 session file(s)
Ingested: 4313, Skipped: 337, Errors: 0

After:

Scanning 2 source(s)...
  4,294 in /Users/me/.claude/projects
    356 in /Users/me/.codex/sessions
Found 4,650 session file(s)
  [████████████████████] 100% · 4,650/4,650
Ingested 4,313 session(s) (812,540 messages) in 47.2s
Skipped 337: 337 empty

What changed

  • Per-source file counts during the scan phase — easy to spot when a source dir is empty or surprisingly small.
  • Skip-reason breakdown — the single skipped counter is now split into already_ingested / empty / duplicate_id. The aggregate skipped field is preserved on the return object so existing tests still pass. Turns an opaque number into something you can act on (e.g. "all 337 skipped were empty sessions, not duplicates").
  • Total messages ingested + elapsed time.
  • Live progress bar (TTY-only, 100ms throttle, sub-character eighth-block fill for smooth progression) — handcrafted in ~10 lines, no new dependency.

Implementation notes

  • ingestSessions gains an optional 4th argument onProgress?: (done, total) => void. Default omitted → existing call sites unchanged.
  • Counters that previously fed skipped++ now feed individual reasons; the returned skipped is computed as their sum so ingest.test.ts keeps passing without modification.
  • Progress writer gates on process.stderr.isTTY so piped output (CI, >file) stays clean.
  • Bar width adapts to process.stdout.columns, clamped to 20–40 chars.

Test plan

  • bun test — all 92 tests pass (no test changes needed; existing ingest.test.ts exercises the preserved ingested / skipped / errors fields)
  • bun run typecheck clean
  • Pre-commit hooks green
  • Manually verified bar rendering at 0/5/12/25/37/50/67/87/99/100% — sub-char partials engage at non-multiples of 5%, no off-by-one width drift
  • Manually ran bun src/index.ts ingest against a 4,650-file dataset and confirmed the new output

Replaces the three-line summary with a richer report:

- Per-source file counts during the scan phase
- Live progress bar (TTY-only, 100ms throttle, sub-char eighth-block fill)
- Splits skipped into already_ingested / empty / duplicate_id so the
  number is actionable instead of opaque
- Total messages ingested + elapsed time

Before:
  Scanning 2 source(s)...
  Found 4650 session file(s)
  Ingested: 4313, Skipped: 337, Errors: 0

After:
  Scanning 2 source(s)...
    4,294 in /Users/me/.claude/projects
      356 in /Users/me/.codex/sessions
  Found 4,650 session file(s)
    [████████████████████] 100% · 4,650/4,650
  Ingested 4,313 session(s) (812,540 messages) in 47.2s
  Skipped 337: 337 empty
artcashin added a commit to artcashin/engineering-notebook that referenced this pull request Aug 2, 2026
artcashin added a commit to artcashin/engineering-notebook that referenced this pull request Aug 2, 2026
artcashin added a commit to artcashin/engineering-notebook that referenced this pull request Aug 2, 2026
…ime-radiant-inc#19 merge

prime-radiant-inc#16 replaced ingest's single `skipped` counter with alreadyIngested / empty /
duplicateId, deriving `skipped` from their sum, and dropped the no-start-time
guard. prime-radiant-inc#19 keeps that guard and still increments `skipped`.

Git merges the two cleanly: prime-radiant-inc#16's counters plus prime-radiant-inc#19's guard, referencing a
variable that no longer exists. Tests pass; `tsc` does not:

    src/ingest.ts(108,9): error TS2304: Cannot find name 'skipped'.

A session with no start time cannot be placed on a date, so it is as unusable
as one with no messages and is counted as empty, which flows into the derived
`skipped`. Keeping the guard rather than dropping it as prime-radiant-inc#16 did: without it a
session with no timestamp is inserted with an empty started_at and misgroups.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@artcashin

Copy link
Copy Markdown

Heads-up: this and #19 merge cleanly into code that does not compile

Found while rebasing my own work onto this PR. Not a defect in #16 — it is an interaction, and it will land on whoever merges second.

What happens

This PR replaces the single skipped counter in ingestSessions with alreadyIngested / empty / duplicateId, derives skipped from their sum, and drops the !session.startedAt guard. #19 keeps that guard and still does skipped++.

Git merges the two without a conflict: #16's counters plus #19's guard, incrementing a variable that no longer exists.

src/ingest.ts(108,9): error TS2304: Cannot find name 'skipped'.

Why it will look like the other PR's fault

The full test suite passes on the merged result — only tsc catches it. Each PR is clean on its own, so neither one's CI is red today. pull_request builds against a simulated merge with the base, so it surfaces as a red typecheck on whichever PR is second, which reads as "that PR broke main."

Reproduce

git checkout -b conflict-check origin/main
git merge <this-branch> && git merge <pr-19-branch>
bun test        # passes
bun run typecheck   # TS2304

It cannot be pre-fixed in either PR. On #19 alone the counter is still skipped, and on this PR alone the guard does not exist, so the correcting line only compiles once both are present. It has to be handled in the merge resolution.

The resolution I used, keeping the guard: count it as empty++, which flows into the derived skipped. Dropping the guard instead would let a session with no started_at be inserted and then misgroup by date, so keeping it seems right.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants