docs(#4892): publish the Bolt driver compatibility matrix (page + badge)#5133
Conversation
…wiring Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Tick the box to add this pull request to the merge queue (same as
|
There was a problem hiding this comment.
Code Review
This pull request implements the publication of ArcadeDB's Bolt driver compatibility matrix by adding a Python rendering script, comprehensive unit tests, a README badge, and CI workflow integration. The review feedback focuses on enhancing the robustness of the Python script by introducing defensive programming guards—such as fallback empty dictionaries and safe nested lookups—to prevent potential runtime crashes when parsing empty or malformed YAML and JSON inputs.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| spec = yaml.safe_load(fh) | ||
| scenarios = [] | ||
| for entry in spec.get("scenarios", []): |
There was a problem hiding this comment.
If spec.yaml is empty or invalid, yaml.safe_load(fh) can return None. Calling .get() on None will raise an AttributeError. Adding a fallback empty dictionary ensures robust execution.
| spec = yaml.safe_load(fh) | |
| scenarios = [] | |
| for entry in spec.get("scenarios", []): | |
| spec = yaml.safe_load(fh) or {} | |
| scenarios = [] | |
| for entry in spec.get("scenarios", []): |
References
- Enforce defensive programming: ensure appropriate null/nil/None checks or other language-idiomatic guards exist before object property accesses to handle invalid inputs or states safely.
There was a problem hiding this comment.
Applied in b6199dc: spec = yaml.safe_load(fh) or {}, so an empty/invalid spec.yaml yields [] instead of raising AttributeError. New test loads an empty spec file and asserts load_scenarios returns [].
| runtime = (matrix.get("scenarios", {}).get(scenario["id"], {}) | ||
| .get(column.language, {}).get(column.version)) |
There was a problem hiding this comment.
If any intermediate key in the nested matrix dictionary is explicitly null in the JSON, the lookup will return None and subsequent .get() calls will raise an AttributeError. Using a safer nested lookup pattern prevents potential runtime crashes.
| runtime = (matrix.get("scenarios", {}).get(scenario["id"], {}) | |
| .get(column.language, {}).get(column.version)) | |
| runtime = (((matrix.get("scenarios") or {}).get(scenario["id"]) or {}) | |
| .get(column.language) or {}).get(column.version) |
References
- Enforce defensive programming: ensure appropriate null/nil/None checks or other language-idiomatic guards exist before object property accesses to handle invalid inputs or states safely.
There was a problem hiding this comment.
Applied in b6199dc with the or {} idiom guarding each level. The matrix JSON is produced by our own merge_matrix.py, which never emits null intermediates, but the guard is cheap and a new test now feeds an explicit-null intermediate ({"scenarios": {"TYPE-011": None}}) to confirm resolve_cell returns the neutral no-result glyph rather than raising.
| languages = len(matrix.get("languages", [])) or 5 | ||
| if matrix.get("has_failures"): | ||
| fails = 0 | ||
| for langs in matrix.get("scenarios", {}).values(): | ||
| for versions in langs.values(): | ||
| fails += sum(1 for status in versions.values() | ||
| if status == "fail") | ||
| fails += len(matrix.get("missing_cells", [])) | ||
| fails += len(matrix.get("empty_cells", [])) | ||
| fails += len(matrix.get("unexpected_cells", [])) |
There was a problem hiding this comment.
If any of the expected keys (languages, scenarios, missing_cells, empty_cells, unexpected_cells) are explicitly null in the JSON, calling len() or .values() on them will raise a TypeError or AttributeError. Adding fallback guards ensures the badge computation is robust against partially populated or malformed JSON inputs.
| languages = len(matrix.get("languages", [])) or 5 | |
| if matrix.get("has_failures"): | |
| fails = 0 | |
| for langs in matrix.get("scenarios", {}).values(): | |
| for versions in langs.values(): | |
| fails += sum(1 for status in versions.values() | |
| if status == "fail") | |
| fails += len(matrix.get("missing_cells", [])) | |
| fails += len(matrix.get("empty_cells", [])) | |
| fails += len(matrix.get("unexpected_cells", [])) | |
| languages = len(matrix.get("languages") or []) or 5 | |
| if matrix.get("has_failures"): | |
| fails = 0 | |
| for langs in (matrix.get("scenarios") or {}).values(): | |
| for versions in (langs or {}).values(): | |
| fails += sum(1 for status in (versions or {}).values() | |
| if status == "fail") | |
| fails += len(matrix.get("missing_cells") or []) | |
| fails += len(matrix.get("empty_cells") or []) | |
| fails += len(matrix.get("unexpected_cells") or []) |
References
- Enforce defensive programming: ensure appropriate null/nil/None checks or other language-idiomatic guards exist before object property accesses to handle invalid inputs or states safely.
There was a problem hiding this comment.
Applied in b6199dc: languages/scenarios/missing_cells/empty_cells/unexpected_cells are all guarded with or {} / or [], and the nested .values() iterations guard langs/versions too. New test passes an all-null-valued matrix with has_failures: true and asserts a red badge is returned without raising.
Review: Publish the Bolt driver compatibility matrixNice, self-contained piece of work. The renderer is pure-stdlib + PyYAML, well-factored into small pure functions, and the 23 unit tests genuinely exercise the interesting logic (cell precedence, badge thresholds, not-applicable exclusion, fallback baseline, area-drift guard). The two-vocabulary design (runtime pass/fail/skip vs spec current_status) is cleanly separated, and the no-op-on-missing-artifact behavior is the right call to avoid publishing a false green. Overall this is close to mergeable; comments below are mostly minor. Correctness / consistency
Robustness
Test coverage gaps (minor)
Nits
Security No injection surface found: scenario text/footnotes render from the in-repo spec.yaml, and the downloaded artifact only contributes status strings and cell-name sets (counted or used for glyph selection, never interpolated as markdown). run-url/repo/timestamp all come from trusted GitHub context. contents: write scope is appropriate and confined to the publish job. Good work overall - the design doc and plan are thorough and the fallback/no-op reasoning is sound. Reviewed with Claude Code. |
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Complexity | 128 |
🟢 Coverage ∅ diff coverage
Metric Results Coverage variation Report missing for 65dddf21 Diff coverage ✅ ∅ diff coverage Coverage variation details
Coverable lines Covered lines Coverage Common ancestor commit (65dddf2) Report Missing Report Missing Report Missing Head commit (a5616c9) 168116 114254 67.96% Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch:
<coverage of head commit> - <coverage of common ancestor commit>Diff coverage details
Coverable lines Covered lines Diff coverage Pull request (#5133) 0 0 ∅ (not applicable) Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified:
<covered lines added or modified>/<coverable lines added or modified> * 100%1 Codacy didn't receive coverage data for the commit, or there was an error processing the received data. Check your integration for errors and validate that your coverage setup is correct.
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
…blish churn+rebase, tests
Review response (cycle 1) - all addressed in b6199dc1. Badge vs page divergence on missing/empty cells - Fixed. The page now renders a 2. Timestamp defeats "only when changed" - Fixed. The publish step now detects changes with 3. not-applicable not excluded from the badge fail count - Fixed. 4. Rebase-retry could wedge - Fixed. The retry now runs 5 & 6. Test coverage gaps - Added tests for the 7. Green badge "N/N passing" ambiguity - Green message changed to 8. Unpinned pyyaml in a contents:write job - Pinned Gemini's three defensive null-guard suggestions (empty-YAML, null-valued-JSON) are also in this batch. All 32 tests pass with pristine output; |
| @@ -1 +1 @@ | |||
| pyyaml>=6.0 | |||
| pyyaml==6.0.2 | |||
|
Review: publish the Bolt driver compatibility matrix Overall this is a clean, well-structured change. The renderer is pure-stdlib + PyYAML, the two status vocabularies (runtime pass/fail/skip vs spec current_status) are kept properly distinct, the null-defensive 1. (Primary) The only-if-changed guard suppresses the pending -> verified transition; the page can stay "pending first nightly run" forever In When the first real nightly is all-green, Consequences:
The intent (avoid a daily no-op commit) is reasonable, but the guard cannot distinguish "placeholder -> first real verification" from "pure timestamp bump." Consider letting the transition through, e.g. only ignore timestamp deltas once the page is already in the verified form, or refresh the timestamp on a lower cadence rather than never. 2. (Operational) publish-matrix pushes directly to main The job commits and 3. (Minor) Badge/page can disagree, and a red badge can read "0 failing"
4. (Minor) No test guards the committed page/badge against renderer drift Nothing asserts that the committed None of these block core functionality; issue 1 is the one I would fix before merge since it directly affects what the published artifact says. |
…l push, drift test
Review response (cycle 2) - fixed in f9ff1131. (Primary) only-if-changed guard suppressed the pending -> verified transition - Correct, and it was a regression from the cycle-1 change-detection I added. The 2. (Operational) publish-matrix pushes directly to main - 3. (Minor) badge/page divergence on band-restriction / "0 failing" red - Both require a pipeline-impossible state: a 4. (Minor) no test guards the committed page/badge against renderer drift - Added |
Review: Bolt driver compatibility matrix publication (#5133)Reviewed the renderer, tests, and both workflow changes. Overall this is clean, well-structured work: pure stdlib + PyYAML (no new heavyweight deps), the two status vocabularies are kept properly distinct, One issue I think will actually break CI, plus a few minor notes. 🔴
|
Review response (cycle 3) - fixed in 6acb57e🔴 Badge message wording - The code has emitted Direct push to
32 tests pass with pristine output; actionlint clean. |
Review: publish the Bolt driver compatibility matrix (page + badge)Overall this is a clean, well-structured, and unusually well-tested change. A few things worth considering before merge: 1. The "Last verified" timestamp defeats the "only when changed" commit guard (main concern)The PR describes the publish step as committing "only when changed", guarded by: if git diff --quiet -- bolt/conformance/COMPATIBILITY.md bolt/conformance/badge.json; then
echo "Rendered matrix is byte-identical ...; nothing to publish."But every nightly render injects a fresh Because that line changes on every run,
2. Retry loop can waste attempts after a failed rebasegit pull --rebase -X theirs origin main || git rebase --abort || trueIf the rebase fails and is aborted, the loop continues and re-runs 3. Badge can read red "0 failing" (contradictory) in one edge caseIn 4. Unverified scenarios do not downgrade the badge
5. No CI guard that the committed page/badge stay in sync with the renderer
Nits
None of these are blockers; #1 is the one I would most want confirmed as intended before merge. Good, careful work overall. Note: I could not execute the Python test suite in this sandbox (python invocation was blocked here), so the test-behavior comments are from reading the code rather than a live run. |
Review response (cycle 4) - fixed in a5616c91. "Last verified" timestamp defeats "only when changed" (main concern) - The daily commit is intentional: for a certification artifact the 2. Retry loop can waste attempts after a failed rebase - Acknowledged, kept as-is. 3. Badge can read red "0 failing" - Fixed. 4. Unverified scenarios do not downgrade the badge - Fixed. The yellow/partial condition now covers 5. Add a committed-page/badge drift test - Respectfully pushing back: this exact test existed and was removed in the previous review round. It asserted the committed 34 tests pass with pristine output; actionlint clean; committed page/badge are unchanged by 3 and 4 (today is all-green). |
|
Review: publish the Bolt driver compatibility matrix (page + badge) Reviewed the renderer, its tests, the workflow wiring, and the committed page/badge. This is a clean, well-factored piece of work: a pure stdlib + PyYAML renderer with a clear split between the two status vocabularies, defensive A few things worth confirming or tightening: Operational
Correctness / consistency (low severity)
Nits
None of these block the core value here, and the ACs look satisfied. The branch-protection question is the one I would resolve before relying on the nightly publish. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #5133 +/- ##
============================================
+ Coverage 65.84% 66.21% +0.37%
- Complexity 913 916 +3
============================================
Files 1692 1692
Lines 136278 136278
Branches 29129 29129
============================================
+ Hits 89736 90241 +505
+ Misses 34336 33728 -608
- Partials 12206 12309 +103 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Closes #4892. Part of epic #4882 (Bolt Driver Compatibility Certification), Group D - publication. This completes Group D, so #4882 can be closed once this merges.
What
Turns the machine-readable matrix that #4891 / #5115 already produces into the human-facing certification artifact the epic exists to deliver: a committed compatibility page and a README badge, both auto-generated from CI.
bolt/conformance/tools/render_matrix.py(+test_render_matrix.py, 23 unit tests) - a pure-stdlib + PyYAML renderer that reads the nightlybolt-compat-matrix.json,spec.yaml(scenario metadata), anddriver-versions.md(the column set) and emits:bolt/conformance/COMPATIBILITY.md- the matrix page: every scenario (rows, grouped by the 9 feature areas) x every driver language x pinned version (15 columns), with a legend, per-scenarioknown_limitationfootnotes, and a "Last verified" timestamp + source-run link.bolt/conformance/badge.json- a shields.io endpoint (bolt drivers: greenall passing/ yellowpartial/ redN failing).bolt-nightly.ymlgains apublish-matrixjob that regenerates the page/badge from the nightly artifact and commits them tomain([skip ci]; a nightly freshness-stamp commit, plus real result changes).bolt-conformance-spec.ymlruns the new renderer unit tests.How it works
pass/fail/skip) and each scenario'sspec.yamlcurrent_status(passing/expected-fail/not-applicable/unverified). Cells resolve by combining them.fail/expected-failcell links the scenario'stracking_issue, falling back to the nightlybolt-compat-regressionissue.not-applicablescenarios (today only ERR-003) are excluded from the denominator, so the badge stays green; it only goes yellow on anexpected-failand red on any real fail / missing / empty / unexpected cell.spec.yamlcurrent_status. That is how the committed initial page is produced (today: all-green,5/5 passing). In the nightly, a missing artifact (crashedmerge-matrix) is a no-op - it leaves the last good page in place rather than publishing a false-green, whilereport-regressionstill alerts. The push rebase-retries so a concurrent commit tomaincannot red the run.Verification
test_render_matrix.py: 23 tests (cell precedence, badge thresholds incl. not-applicable exclusion, fallback baseline, issue-link normalization, unavailable-column wiring, area-drift guard, pristine output), wired intobolt-conformance-spec.yml.junit_to_matrix.py -> merge_matrix.py -> render_matrix.pycomposes; the committed page/badge are byte-reproducible from the renderer.actionlintclean on both edited workflows.🤖 Generated with Claude Code