Skip to content

feat(perf): Sprint 2 — data-shape benchmark matrix (10 dtypes × 4 tiers) - #31

Merged
wolfiesch merged 3 commits into
masterfrom
feat/perf-data-shape
Apr 27, 2026
Merged

feat(perf): Sprint 2 — data-shape benchmark matrix (10 dtypes × 4 tiers)#31
wolfiesch merged 3 commits into
masterfrom
feat/perf-data-shape

Conversation

@wolfiesch

Copy link
Copy Markdown
Collaborator

Summary

  • Sprint 2 of the 7-Dimension Extension. Extends the throughput-fixture pipeline with a parametric (dtype × scale) matrix so the dashboard can answer "how does library X handle int vs string vs formula at 1M cells?" — the highest-signal question for users picking between Excel libraries.
  • Same generator/runner extension pattern as the existing throughput plumbing — no new infrastructure, ~1000 LOC of additive code across 7 files. Discovery (committed to the plan & DEC-019) showed the original "build new generator from scratch" framing was wrong: the seam already existed.

Replaces #30 — that PR auto-closed when its base branch feat/perf-mem-honesty was deleted on Sprint 1 (#28) merge. This PR has been rebased directly onto master (which now contains the squashed S1 with all review fixes) so the resulting diff is the S2 surface alone.

What ships

  • Runner: 7 new value_type branches in _run_workload_write (float, date, datetime, boolean, formula_simple, formula_cross_sheet, mixed_realistic). string_short / string_long fold into the existing string op via string_length=16 / string_length=512.
  • Generator: generate_data_shape_scenarios() emits 10 dtypes × 3 tiers (1k / 10k / 100k) by default = 60 manifest rows; the 1M tier is gated behind --include-1m. New --shape-only flag skips legacy scenarios for fast iteration.
  • CLI: excelbench perf-shape subcommand with --rows, --types, --regenerate, --memory-mode, --adapter. Inherits Sprint 1's three-mode memory plumbing for free. Auto-regenerates fixtures when the manifest is older than the generator script or the requested 1M tier is missing.
  • Dashboard: new Data Shape (dtype × scale) tab — two heatmaps (read, write), rows = library sorted by overall median latency, columns = 10 dtypes, cell = ms-per-100k-cells at the largest tier each (library, dtype) was run at, log-scale color normalized per dtype-column.
  • DEC-019 documents the rationale (matrix shape, mixed_realistic 60/30/5/3/2 ratio, alternatives considered, deferred follow-ups). Calibration provenance at fixtures/synthetic_calibration/sample_set.md.

Verification

  • uv run pytest tests/ ✓ 1140 passed, 32 skipped, 6 xfailed
  • uv run ruff check src/ tests/ scripts/
  • uv run mypy src/
  • Coverage 65.44% (above 65% gate) — improved organically after rebase absorbed S1 fixes.
  • excelbench perf-shape --rows 1000 --types int --iters 1 ✓ generates fixtures + emits results.json/matrix.csv/history.jsonl.

Test plan

  • All tests pass post-rebase
  • Coverage above 65% gate
  • CI green on lint, test 3.11/3.12, benchmark, rust_smoke (will land after push)
  • Address review comments from Copilot/Gemini/Greptile
  • Flip TRACKER.md S2 row to Shipped after merge

🤖 Generated with Claude Code

Extends the throughput-fixture pipeline with a parametric (dtype × scale)
matrix so the dashboard can answer "how does library X handle int vs string
vs formula at 1M cells?" — the most actionable question for users picking
between libraries.

What ships:
- 7 new value_type branches in _run_workload_write (float, date, datetime,
  boolean, formula_simple, formula_cross_sheet, mixed_realistic);
  string-short/long fold into existing string op via string_length.
- generate_data_shape_scenarios() in the throughput generator: 10 dtypes ×
  3 tiers (1k/10k/100k) by default, +1M tier behind --include-1m. New
  --shape-only flag for fast-iteration runs.
- excelbench perf-shape subcommand with --rows/--types/--regenerate/
  --memory-mode (inherits Sprint 1's memory-mode plumbing).
- _section_data_shape dashboard heatmap (read + write), per-dtype-column
  log-scale color so slow columns don't wash out fast ones.
- DEC-019 with mixed_realistic 60/30/5/3/2 ratio rationale and
  fixtures/synthetic_calibration/sample_set.md calibration provenance.

Branched off feat/perf-mem-honesty (S1) since #28 hasn't merged; once it
does, this PR will retarget master.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 5e4553146a

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/excelbench/cli.py Outdated
_DATA_SHAPE_TIER_CAPS: list[tuple[str, int]] = [
("1k", 1_000),
("10k", 10_000),
("100k", 100_000),

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Use actual 100k tier size for --rows gating

_resolve_shape_features gates the 100k tier at 100_000 cells, but the generated tier is actually 316×316 (99,856) cells (as defined in the fixture generator/dashboard constants). Because of this, runs like excelbench perf-shape --rows 99856 incorrectly skip data_shape_*_100k_* features even though they should satisfy the documented <= --rows behavior, so experiments can silently execute a smaller matrix than requested.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in c9c0107 — 100k cap now 99_856 to match the actual 316×316 fixture; added a comment pointing at DATA_SHAPE_TIERS so the two definitions stay in lockstep.

Comment thread scripts/generate_throughput_fixtures.py Outdated
Comment on lines +261 to +262
v = r * cols + c
ws.write_datetime(r, c, base + timedelta(days=v), date_fmt)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Align date/datetime fixture seeds with write workload

The data-shape generator uses a zero-based counter for date/datetime values (v = r * cols + c), while the paired write workload is emitted with start=1 and _run_workload_write derives these types from that 1-based v. That means the read fixtures and write payloads for the same dtype are offset (first date/datetime cell differs), so the new read-vs-write matrix is not using equivalent cell content as intended.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in c9c0107 — generator now uses v = r * cols + c + 1 for boolean/date/datetime, matching the runner's start=1 convention so read fixtures and write payloads use equivalent cell content.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds Sprint 2 “data-shape” performance benchmarking across a dtype × scale matrix, including on-demand fixture generation, a dedicated CLI entrypoint, and a new dashboard section to visualize results.

Changes:

  • Extend throughput fixture generation with data_shape_<dtype>_<tier> scenarios (10 dtypes × 1k/10k/100k, optional 1m).
  • Add excelbench perf-shape CLI command to select tiers/types, auto-regenerate fixtures when stale, and run the perf harness with Sprint 1 memory modes.
  • Render a new “Data Shape (dtype × scale)” dashboard tab with per-dtype log-normalized heatmaps for read/write.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 7 comments.

Show a summary per file
File Description
src/excelbench/results/html_dashboard.py Adds data-shape section rendering + nav link and heatmap computation.
src/excelbench/perf/runner.py Extends bulk write workload value generation to cover additional dtypes and mixed_realistic.
src/excelbench/cli.py Introduces perf-shape subcommand with fixture staleness detection and feature filtering.
scripts/generate_throughput_fixtures.py Adds data-shape dtype/tier matrix generation, --shape-only, and optional --include-1m.
fixtures/throughput_xlsx/README.md Documents the new data-shape matrix, tiers, dtypes, and naming pattern.
fixtures/synthetic_calibration/sample_set.md Documents provenance for the mixed_realistic ratio calibration.
decisions.md Adds DEC-019 describing the design rationale and consequences for Sprint 2.
TRACKER.md Updates Sprint 2 status to “In Progress”.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread scripts/generate_throughput_fixtures.py Outdated
base = datetime(2020, 1, 1)
for r in range(rows):
for c in range(cols):
v = r * cols + c

Copilot AI Apr 27, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The datetime fixture uses v = r * cols + c (0-based), but the runner's value_type == "datetime" path uses start=1 and writes datetime_epoch + timedelta(seconds=v) (1-based). Align the generator (e.g., v = r * cols + c + 1) or the runner so the declared “mirrors the runner” property holds.

Suggested change
v = r * cols + c
v = r * cols + c + 1

Copilot uses AI. Check for mistakes.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in c9c0107 — generator now uses v = r * cols + c + 1 for boolean/date/datetime, matching the runner's start=1 convention so read fixtures and write payloads use equivalent cell content.

Comment thread src/excelbench/cli.py
Comment on lines +381 to +386
_DATA_SHAPE_TIER_CAPS: list[tuple[str, int]] = [
("1k", 1_000),
("10k", 10_000),
("100k", 100_000),
("1m", 1_000_000),
]

Copilot AI Apr 27, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

_DATA_SHAPE_TIER_CAPS treats the 100k tier as 100,000 cells, but the generator defines that tier as 316×316 (= 99,856). This makes excelbench perf-shape --rows 99856 unexpectedly skip the 100k tier. Consider using the exact cell count (99_856) here (or compute caps from the tier (rows, cols) definitions) to keep tier selection consistent with the fixtures.

Copilot uses AI. Check for mistakes.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in c9c0107 — 100k cap now 99_856 to match the actual 316×316 fixture; added a comment pointing at DATA_SHAPE_TIERS so the two definitions stay in lockstep.

Comment thread fixtures/throughput_xlsx/README.md Outdated
uv run python scripts/generate_throughput_fixtures.py --shape-only --include-1m
```

The 40 shape feature names follow the pattern

Copilot AI Apr 27, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This section says “The 40 shape feature names…”, but with 10 dtypes × 4 tiers and separate read/write features there are 80 feature names (40 scenarios). Please clarify the wording (e.g., “40 scenarios” or “80 features”) to avoid confusion when users try to filter by feature name.

Suggested change
The 40 shape feature names follow the pattern
The 40 shape scenarios produce 80 shape feature names following the pattern

Copilot uses AI. Check for mistakes.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in c9c0107 — README now says "40 scenarios produce 80 shape feature names" with the read+write split called out.

Comment thread scripts/generate_throughput_fixtures.py Outdated
Comment on lines +501 to +502
print(f"✓ Wrote {len(files)} data-shape fixture(s) to {out}")
print(f" Manifest: {out / 'manifest.json'}")

Copilot AI Apr 27, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In --shape-only mode files contains manifest entries (read + write rows), not fixture files. Printing “Wrote {len(files)} data-shape fixture(s)” is misleading (e.g., default will print 60 even though it generated 30 .xlsx files). Consider reporting both counts or renaming to “manifest row(s)”.

Copilot uses AI. Check for mistakes.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in c9c0107 — print line now reports both the scenario count (xlsx files) and the manifest-row count separately.

perf_op = (entry.get("perf") or {}).get(op) or {}
wall = perf_op.get("wall_ms") or {}
p50 = wall.get("p50")
if not isinstance(p50, int | float) or p50 <= 0:

Copilot AI Apr 27, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

isinstance(p50, int | float) will raise TypeError at runtime (PEP604 union types aren't valid in isinstance). Use isinstance(p50, (int, float)) (or numbers.Real) so the dashboard can render when data-shape perf results are present.

Suggested change
if not isinstance(p50, int | float) or p50 <= 0:
if not isinstance(p50, (int, float)) or p50 <= 0:

Copilot uses AI. Check for mistakes.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in c9c0107 — switched to isinstance(p50, (int, float)). Note: int | float in isinstance is actually valid on Python 3.10+ (project is 3.11+), so this was working, but the tuple form is universally compatible and equally readable.

Comment thread scripts/generate_throughput_fixtures.py Outdated
if dtype == "boolean":
for r in range(rows):
for c in range(cols):
ws.write_boolean(r, c, bool((r * cols + c) % 2))

Copilot AI Apr 27, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The boolean fixture uses bool((r * cols + c) % 2) (starting with False), but the runner's value_type == "boolean" writes bool(v % 2) with start=1 (starting with True). If the intent is for read/write scenarios to model the same dtype distribution, generate boolean values from idx + 1 (or adjust the runner) so the two stay in sync as stated in the docstring.

Suggested change
ws.write_boolean(r, c, bool((r * cols + c) % 2))
v = r * cols + c + 1
ws.write_boolean(r, c, bool(v % 2))

Copilot uses AI. Check for mistakes.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in c9c0107 — generator now uses v = r * cols + c + 1 for boolean/date/datetime, matching the runner's start=1 convention so read fixtures and write payloads use equivalent cell content.

Comment thread scripts/generate_throughput_fixtures.py Outdated
base = date(2020, 1, 1)
for r in range(rows):
for c in range(cols):
v = r * cols + c

Copilot AI Apr 27, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The date fixture uses v = r * cols + c (0-based), but the runner's value_type == "date" path uses start=1 and writes date_epoch + timedelta(days=v) (1-based). If the generator is meant to mirror the runner (per the docstring), switch to v = r * cols + c + 1 (or align the runner's start).

Suggested change
v = r * cols + c
v = r * cols + c + 1

Copilot uses AI. Check for mistakes.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in c9c0107 — generator now uses v = r * cols + c + 1 for boolean/date/datetime, matching the runner's start=1 convention so read fixtures and write payloads use equivalent cell content.

wolfiesch and others added 2 commits April 27, 2026 04:54
Brings PR #31 above the 65% coverage gate by adding 31 tests for the
data-shape benchmark matrix:

- Parametrized value_type writes (10 dtypes including formula_simple,
  formula_cross_sheet, mixed_realistic, sparse cells)
- Unsupported value_type surfaces in row.notes (catch-all branch)
- _resolve_shape_features (rows-to-tiers, types filtering, error paths)
- _shape_fixtures_stale (missing manifest, generator-newer-than-manifest,
  needs_1m gate, fresh-manifest-no-regen)
- _section_data_shape dashboard helper (heatmap rendering, color scaling)
- perf_shape CLI (invalid memory_mode, invalid dtype, unknown adapter,
  happy path writing results.json/matrix.csv/history.jsonl)

Local coverage 67.64% (above 65% gate); Linux CI estimated ~65.9%.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- cli.py: tier cap for 100k from 100,000 → 99,856 (matches actual 316×316
  generated fixture, so --rows 99856 doesn't silently drop the tier)
- generate_throughput_fixtures.py: align boolean/date/datetime fixture
  seeds with the runner's start=1 convention (was 0-based, runner was
  1-based, causing read fixtures and write payloads to differ for the
  same dtype)
- generate_throughput_fixtures.py: distinguish "scenarios" (xlsx files)
  from "manifest rows" in the --shape-only output line
- README.md: clarify "40 scenarios produce 80 features" (read + write)
- html_dashboard.py: switch isinstance(p50, int | float) to tuple form
  (universally compatible; Copilot incorrectly flagged PEP 604 form)

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@wolfiesch
wolfiesch merged commit cbb530f into master Apr 27, 2026
5 checks passed
@wolfiesch
wolfiesch deleted the feat/perf-data-shape branch April 27, 2026 12:07
wolfiesch added a commit that referenced this pull request Apr 27, 2026
Sprint 2 (data-shape benchmark matrix) merged in #31. Status table row
flipped to Shipped with commit range 373c896..cbb530f, full acceptance
note appended (newest-first ordering).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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