/references/tone-guidelines.md

---

Code Review: reports/phase3/scripts/build_top200_v2_h5.py

This diff is a major refactoring of the H5 page builder — from a standalone inline HTML generator (~925 lines) to a template-based approach (~312 lines) that reuses skill-market-h5.html as a shell. The table-based layout is replaced with a card grid, the scoring scale changes from 20 to 5, and the UI switches from select-based filters to chip-based filters.

---

build_top200_v2_h5.py:100 — CLEANUP — Dead return value in extract_h5_shell()

Summary: The function returns a tuple (head_and_body, "") where the second element is always an empty string.

Suggestion: Return just head_and_body as a plain string, and update the caller (shell = extract_h5_shell() instead of shell, _ = extract_h5_shell()).

---

build_top200_v2_h5.py:105–158 — CLEANUP — Fragile template text replacement

Summary: The function performs 10+ sequential .replace() calls against exact HTML markup in skill-market-h5.html. If the template is ever reformatted or tweaked (whitespace changes, attribute reordering), these replacements silently no-op — the old strings remain in the output.

Failure scenario: Someone edits skill-market-h5.html (e.g., adds a newline inside a <div> or changes class="sub" to class="subtitle"). The build script produces output with stale V1 text labels instead of V2 labels, with no warning.

Suggestion: Consider one of:

• Extract translatable strings/labels into a JSON config that both the template and this script reference.
• Use regex or a small HTML parser (e.g., html.parser) to find elements by class or ID and replace text content only.
• At minimum, add a post-build assertion that checks the output for expected V2 strings.

---

build_top200_v2_h5.py:247 — CLEANUP — esc() + inline onclick bypasses HTML escaping in JS context

Summary: The esc() function escapes ' to ', but when interpolated into an inline onclick attribute:

onclick="openDrawer('${esc(r.skill_id)}')"

the HTML parser decodes ' back to ' before the JavaScript engine runs. If a skill_id contained a single quote (e.g., it's_a_test), the decoded JS would be:

openDrawer('it's_a_test')

which is a syntax error — the string terminates at the second '.

Failure scenario: A skill_id containing ' would break the card's click handler. Extremely unlikely given skill ID naming conventions (slugs like namespace__skill-name), but incorrect in principle.

Suggestion: Prefer event delegation — attach one click handler on the grid container and read data-id from the card:

document.getElementById('grid').addEventListener('click', e => {
  const card = e.target.closest('[data-id]');
  if (card) openDrawer(card.dataset.id);
});

Same issue applies to line 268 (onclick="copyText('${esc(r.copied_path || '')}', this)").

---

build_top200_v2_h5.py:176–182 — CLEANUP — securityLabel() fallback ordering is fragile

Summary: The function first checks exact keys in the m map, then falls back to String(s).includes('passed') before String(s).includes('caution'). If a new security value containing both "passed" and "caution" were introduced (e.g., passed_with_caution_but_flagged), it would match includes('passed') first and return '通过' instead of '谨慎通过'.

Failure scenario: A hypothetical new security status passed_with_caution_but_flagged would display as "通过" rather than "谨慎通过". In practice, all known values are explicitly mapped, so this only matters if new values appear at runtime without updating the code.

Suggestion: Check caution before passed in the fallback chain, since "caution" is a more specific qualifier:

if (String(s).includes('caution')) return '谨慎通过';
if (String(s).includes('passed')) return '通过';

---

build_top200_v2_h5.py:233–237 — CLEANUP — Sort options unreachable via UI

Summary: The JS sort handler supports rank, rank-desc, score, score-asc, and name modes. But the template's <select> (after extract_h5_shell() replacements) only has options for score and score-asc. The name sort option and rank-desc are never exposed as user-choosable options. The default state.sort = 'rank' (line 172) works for initial load, but users can't switch to name or reverse-rank sorting.

Suggestion: Either add the missing <option> elements in extract_h5_shell(), or remove the unreachable sort cases from the switch statement.

---

build_top200_v2_h5.py:25–50 — CLEANUP — load_category_zh_map() silently degrades on missing files

Summary: If both CSV files (platform-high-frequency-skills.csv and platform-high-frequency-groups.csv) are absent, the function silently falls back to just the 3-entry SUBGROUP_CATEGORY_ZH dict and whatever the optional markdown filter provides. The output will have very few Chinese category names, but no warning is raised.

Suggestion: Log a warning when neither CSV is found, so the developer knows the Chinese mapping is incomplete.

---

build_top200_v2_h5.py:68–97 — CLEANUP — transform_records() copies fields verbatim without validation

Summary: The function copies fields from raw JSON to output dicts without any type casting or validation (except protected → bool). Fields like weighted_score and selection_score could be strings or numbers from the input, and the JS code on the consumer side has to handle both (e.g., Number(r.weighted_score) || 0). Consider normalizing numeric fields here in Python to reduce JS complexity.

Suggestion: Add type coercion for numeric fields in Python:

"weighted_score": _to_float(row.get("weighted_score")),
"selection_score": _to_float(row.get("selection_score")),
"rank": _to_int(row.get("rank")),

---

Summary

• 0 blocker findings
• 0 correctness bugs
• 6 cleanup/simplification opportunities
• Decision: Ready to merge — no blockers or bugs found. The cleanup items (especially the fragile template replacement and the onclick escaping) are worth tracking but not blocking.