jeswr / jeswr/json-split-transformer

Triage: open issues, planned fixes & performance findings

Open
#107 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
TypeScript
Stars
0
Forks
0
PR merge metrics
No merged PRs in 30d

Description

Full pass over the repository: every open issue read, baseline verified (install / lint / build / test all green on Node 22, 100% coverage threshold enforced), fixes prepared as minimal draft PRs, and a lightweight performance assessment of the transform hot path.

## Open issues

| Issue | Title | Category | Addressed-by-PR |
|---|---|---|---|
| #1 | Add tests | fixed-in-this-wave (close-candidate) | #103 |
| #3 | Better outline usage with asynciterator in docs | fixed-in-this-wave | #105 |
| #5 | add reset method | fixed-in-this-wave | #104 |

Notes:

- **#1** predates the merged test PR #6; the suite has existed since then and CI enforces 100% statement/branch coverage, so this was arguably already done. #103 adds regression tests for the previously untested cross-chunk escape states (the source even carried a `// Current error is that escapes are not being maintained accross chunks` note). Suggest closing #1 when #103 lands.
- **Not previously filed, fixed in this wave:** that known cross-chunk escape bug had three concrete failure modes, each silently dropping or corrupting output — a string literal closing in a later chunk, a chunk ending with a complete `\\` escape pair, and an object continuing past an escaped chunk boundary. All fixed in #103 and verified with a 2000-trial randomized round-trip fuzz (old code fails 1957/2000 trials, fixed code 0/2000).

## Performance findings

Microbenchmark: 4.8 MiB of realistic telemetry-style concatenated JSON, 16 KiB chunks, Node 22, median of 9 interleaved runs (2-core box shared with a live server — absolute numbers noisy, relative deltas stable across runs):

| variant | throughput | status |
|---|---|---|
| current scanner (post-#103) | ~51 MiB/s | baseline |
| `charCodeAt` integer compares | ~59 MiB/s (+14–23% across runs) | **PR #106** (stacked on #103) |
| `indexOf` string-literal skip + backslash-run parity | ~91 MiB/s (~1.9x) | prototype only, fuzz-clean, not PR'd |

- The hot path is the per-character scan; replacing `str[i]` single-character string comparisons with `charCodeAt` integer comparisons was the cheapest win (#106 — control flow unchanged).
- The bigger win is skipping string literals with `indexOf('"')` and deciding escapedness by counting the backslash run before each candidate quote (parity is equivalent to the current pair-stepping walk, including at chunk boundaries). The prototype passes the same 2000-trial fuzz, but it restructures the scanner and has subtle edge cases (empty chunk while mid-escape, runs truncated by chunk starts), so it deserves deliberate review rather than a drive-by change. Happy to turn it into a follow-up PR on request.
- Everything else is already lean: output is produced by `slice`/concat only, no regexes, no sync IO, no per-item allocations beyond the emitted strings.

## Needs your decision

- **Stray top-level `}` handling** — a `}` outside any object drives `nesting` negative, after which the *next* object's opening `{` no longer registers (nesting goes −1 → 0) and that object is silently lost. Options: clamp (`if (nesting > 0)`, treating a stray `}` like inter-object noise) or document as garbage-in/garbage-out. Either way changes behaviour for malformed input, so it was not fixed in this wave.
- **Top-level JSON arrays / primitives** — `[{...},{...}]` happens to emit the inner objects (brackets are skipped like whitespace), but a top-level array of primitives emits nothing. #105 documents objects-only as a limitation; decide whether array support is in scope.
- **`indexOf` scanner (~1.9x)** — see performance findings; want it as a follow-up PR stacked on #106?
- **Dependency staleness** — 10 dependabot PRs are open (some from 2022) and GitHub reports 58 vulnerabilities on the default branch, all in dev/tooling deps (jest 27, TypeScript 4.1, semantic-release 17 era). Probably worth one consolidated tooling-refresh pass instead of merging the piecemeal bumps.

Review timing: prepared with Claude; @jeswr will personally review before it progresses — expect active review Wed-Fri.

Contributor guide

No contributing guide indexed for this repository

Research direction

Review this triage issue alongside PRs #103–#106 and the repository's install, lint, build, and test commands. The work is not a single newcomer-sized change: completion requires maintainer decisions on malformed input, top-level arrays, the proposed scanner optimization, and dependency updates, followed by focused PRs and preserved coverage and benchmark results.

Written by the indexing model from the issue text.

Assessment

Tech stack
node.js, typescript
Domain
performance, testing, tooling
Issue type
Refactor
Difficulty
5/5
Estimated time
Over a week
Activity status
Quiet
Clarity
Needs clarification
Newbie friendliness
20/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.