Repository navigation
Conversation
|
|
|
@bestony The contributor reports completing the CLA as Could you please verify the CLA Assistant signature association/configuration or re-run the check from the maintainer side? This is now the only external status blocker; the repository test check is green. |
bestony
left a comment
There was a problem hiding this comment.
Reviewed exact head d9002d136d6b211e0c1fca78565db632de20d585. The responsive presentation, escaping, print treatment, and local/hosted machine-link handling are sound, but the redesign drops two source-bound parts of the atr-1 contract from the human report.
Validation: npm test (52/52), npm run check, and git diff --check passed. I also rendered the checked example at 1440px and 390px without horizontal overflow.
license/cla remains pending; that is separate from this code verdict.
|
|
||
| function renderDimensionLedger(dimension, index) { | ||
| const tone = dimensionTone(dimension); | ||
| return `<article class="dimension-row tone-${tone.className}"> |
There was a problem hiding this comment.
[P1] Preserve blockers outside the Top 3
This ledger is now the only per-dimension view, but it never renders dimension.must_fix_blocker. top_3_fixes is capped at three while the contract permits one blocker in each of six dimensions. Scanning fixtures/no-instructions produces five dimension blockers; the rendered HTML contains only the first three and silently drops the shared-context and handoff blockers. That changes report meaning rather than presentation. Please keep every dimension blocker and its evidence accessible, and add a regression using a report with more than three blockers.
There was a problem hiding this comment.
Fixed in ee73b21. Every dimension ledger detail now renders its must_fix_blocker with severity, impact, and complete blocker evidence, independently of the Top 3 chapters. The no-instructions regression asserts that all five dimension blockers render even though top_3_fixes contains only three. I also verified the two outside-Top-3 blockers interactively at 1440px and 390px and in the 8-page Letter print output.
| const tone = dimensionTone(dimension); | ||
| return `<article class="dimension-row tone-${tone.className}"> | ||
| <div class="dimension-row-main"> | ||
| <span class="dimension-number">${String(dimension.score).padStart(2, "0")}</span> |
There was a problem hiding this comment.
[P2] Expose the weight used by the headline score
The redesign no longer renders dimension.weight anywhere. The contract uses unequal fixed weights and computes round(sum(score / 10 * weight)); readers cannot reconcile the displayed 0-100 headline with the six /10 values when those inputs are hidden. The previous renderer exposed each weight. Please restore the weights in the rail or ledger and assert all six in the renderer test.
There was a problem hiding this comment.
Fixed in ee73b21. The hero dimension rail now exposes each fixed weight beside its /10 score, and the renderer test asserts the exact name/weight pair for all six dimensions. The headline calculation and atr-1 data remain unchanged.
|
Maintainer-side CLA verification completed:
I cannot inspect CLA Assistant's authenticated signee table from this agent session, but the public configuration is not a correct individual-signing path. The maintainer fix is to link CLA Assistant to an ICLA-only Gist, keep CCLA authorization as a separate verified/manual path, and then have Separately, the exact-head code review found two report-contract regressions and has requested changes. |
bestony
left a comment
There was a problem hiding this comment.
Reviewed exact head ee73b21ca21134a54534cd8a631145a56ea0993e.
Both prior findings are resolved: all six fixed weights are visible alongside their dimension scores, and every dimension blocker remains available with severity, impact, and complete evidence even when it falls outside the Top 3. The same blocker content is present in the print path.
Validation: npm test (52/52), npm run check, and git diff --check passed. A freshly generated five-blocker report retained all five blockers while preserving the deterministic Top 3, rendered without horizontal overflow or console errors at 1440px and 390px, and exposed all five blockers under print media.
Code verdict only: this exact head currently reports the test check as successful, but no license/cla check is present. This approval does not constitute CLA acceptance.
Summary
Preview
Open the checked report example
Validation
npm test— 52/52 passingnpm run checkgit diff --check