Skip to content

Add deterministic multi-family admissibility SVG rendering#139

Merged
ProfRandom92 merged 1 commit into
mainfrom
codex/add-deterministic-multi-family-svg-rendering
May 19, 2026
Merged

Add deterministic multi-family admissibility SVG rendering#139
ProfRandom92 merged 1 commit into
mainfrom
codex/add-deterministic-multi-family-svg-rendering

Conversation

@ProfRandom92
Copy link
Copy Markdown
Owner

Motivation

  • Provide a deterministic, repo-native SVG visualization of all manifest-registered admissibility benchmark families so artifacts can be versioned and regression-checked.
  • Reuse existing deterministic ordering and curve semantics while keeping rendering independent of browser/JS/runtime metadata.

Description

  • Add a deterministic renderer scripts/render_multi_family_admissibility_svg.py that reads artifacts/multi_family_admissibility_results.json, sorts families lexicographically, maps fixture IDs to the four degradation levels (baseline, mild, moderate, severe), and emits a plain SVG with fixed dimensions, stable element ordering, fixed numeric formatting, and a hardcoded color palette.
  • Commit the generated SVG artifact at artifacts/multi_family_admissibility_curves.svg containing one polyline + point per family plus axes, labels, and a deterministic legend.
  • Add focused tests tests/test_multi_family_svg_renderer.py that assert determinism, artifact parity, presence of both current families, presence of all four degradation levels, and absence of nondeterministic tokens.
  • Add an npm helper script generate:multi-family-svg that runs python scripts/render_multi_family_admissibility_svg.py for deterministic regeneration.

Testing

  • Ran python scripts/render_multi_family_admissibility_svg.py to generate the artifact and verify output was produced successfully.
  • Ran pytest tests/test_multi_family_svg_renderer.py -q and observed 5 passed for the new renderer tests.
  • Ran pytest tests/test_artifact_reproducibility.py -q and observed 1 passed for artifact reproducibility checks.
  • Ran full verification via npm run check which completed the repository checks and test suite with 218 passed in the run reported locally.

Codex Task

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request introduces a Python script to generate SVG visualizations of multi-family admissibility degradation curves, accompanied by a new npm script and comprehensive unit tests. The reviewer identified a potential KeyError when processing incomplete data sets and recommended refactoring the coordinate logic to handle missing points more gracefully. Additionally, feedback was provided regarding the legend's fixed vertical layout, which may cause overflow issues as the number of data families grows.

Comment on lines +89 to +90
legend_height = 30 + 18 * len(families)
elements.append(f' <rect x="{legend_x}" y="{legend_y}" width="270" height="{legend_height}" fill="#f8f8f8" stroke="#cccccc"/>')
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.

medium

The legend height calculation and placement do not account for the total SVG height. If the number of families grows significantly (e.g., beyond 18-20), the legend box will overlap with the X-axis labels or extend beyond the bottom margin of the SVG (560px). Consider implementing a multi-column legend or adding a check to ensure the legend fits within the available vertical space.

Comment on lines +96 to +111
points_by_level = {
_level_from_fixture_id(point["fixture_id"]): point
for point in family["curve"]["points"]
}

polyline = " ".join(
f"{_fmt(x_by_level[level])},{_fmt(MARGIN_TOP + ((1.0 - float(points_by_level[level]['overall_admissibility_score'])) * plot_height))}"
for level in LEVELS
)
elements.append(f' <polyline points="{polyline}" fill="none" stroke="{color}" stroke-width="3"/>')

for level in LEVELS:
score = float(points_by_level[level]["overall_admissibility_score"])
x = x_by_level[level]
y = MARGIN_TOP + ((1.0 - score) * plot_height)
elements.append(f' <circle cx="{_fmt(x)}" cy="{_fmt(y)}" r="4" fill="{color}"/>')
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.

medium

The rendering logic assumes that every family contains a data point for every level defined in LEVELS. If a point is missing for a specific level, the script will raise a KeyError on lines 102 and 108. Additionally, the Y-coordinate calculation is duplicated. It is safer to pre-calculate the coordinates and handle missing points gracefully.

Suggested change
points_by_level = {
_level_from_fixture_id(point["fixture_id"]): point
for point in family["curve"]["points"]
}
polyline = " ".join(
f"{_fmt(x_by_level[level])},{_fmt(MARGIN_TOP + ((1.0 - float(points_by_level[level]['overall_admissibility_score'])) * plot_height))}"
for level in LEVELS
)
elements.append(f' <polyline points="{polyline}" fill="none" stroke="{color}" stroke-width="3"/>')
for level in LEVELS:
score = float(points_by_level[level]["overall_admissibility_score"])
x = x_by_level[level]
y = MARGIN_TOP + ((1.0 - score) * plot_height)
elements.append(f' <circle cx="{_fmt(x)}" cy="{_fmt(y)}" r="4" fill="{color}"/>')
coords: list[tuple[float, float]] = []
for level in LEVELS:
point = points_by_level.get(level)
if not point:
continue
score = float(point["overall_admissibility_score"])
x = x_by_level[level]
y = MARGIN_TOP + ((1.0 - score) * plot_height)
coords.append((x, y))
if not coords:
continue
polyline_pts = " ".join(f"{_fmt(cx)},{_fmt(cy)}" for cx, cy in coords)
elements.append(f' <polyline points="{polyline_pts}" fill="none" stroke="{color}" stroke-width="3"/>')
for cx, cy in coords:
elements.append(f' <circle cx="{_fmt(cx)}" cy="{_fmt(cy)}" r="4" fill="{color}"/>')

@ProfRandom92 ProfRandom92 merged commit 511f57c into main May 19, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant