mirror of
https://github.com/heygen-com/hyperframes.git
synced 2026-09-10 22:20:14 +00:00
fix(pr-to-video): use display names, not GitHub logins, in credits narration (#2385)
* fix(pr-to-video): use display names, not GitHub logins, in credits narration gh pr view already returns a `name` field for the PR author, commit authors, and mergedBy. ingest.mjs now tracks it in people.json alongside login; fetch-people-avatars.mjs resolves a name for reviewers/commenters/assignees gh doesn't name via the public GitHub user API, best-effort. story-design.md now directs the credits close to speak the person's name (TTS reading a raw handle like @miguAng18947550 aloud is the failure mode), with the handle shown as secondary on-screen text only. * fix(pr-to-video): resolve missing credit names via the agent, not a new script fetch fetch-people-avatars.mjs already runs inside the orchestrating agent's turn, which has gh available — no need for the script to duplicate a name lookup the agent can do itself with `gh api users/<login> --jq .name`. Reverts the script back to avatar-fetching only; SKILL.md/story-design.md now tell the agent to resolve any missing name for the credited people itself before writing the credits close. * fix(pr-to-video): regenerate skills-manifest.json hash Stale hash left over from a rebase conflict I resolved by hand — the generator produces the correct one.
This commit is contained in:
@@ -58,7 +58,7 @@
|
|||||||
"files": 132
|
"files": 132
|
||||||
},
|
},
|
||||||
"pr-to-video": {
|
"pr-to-video": {
|
||||||
"hash": "688b43a31bbc9360",
|
"hash": "734edc2ac48e3d30",
|
||||||
"files": 28
|
"files": 28
|
||||||
},
|
},
|
||||||
"product-launch-video": {
|
"product-launch-video": {
|
||||||
|
|||||||
@@ -98,7 +98,8 @@ PR="<url | owner/repo#N | N>"
|
|||||||
(cd "$PROJECT_DIR" && node <SKILL_DIR>/scripts/fetch-pr.mjs --pr "$PR" --out-dir ./capture)
|
(cd "$PROJECT_DIR" && node <SKILL_DIR>/scripts/fetch-pr.mjs --pr "$PR" --out-dir ./capture)
|
||||||
|
|
||||||
# Offline transform → capture/extracted/{tokens.json (colors:[] → claude palette),
|
# Offline transform → capture/extracted/{tokens.json (colors:[] → claude palette),
|
||||||
# visible-text.txt (the brief), people.json (contributors, bot-filtered, avatarFile=assets/<login>.png)}.
|
# visible-text.txt (the brief), people.json (contributors, bot-filtered, name+login,
|
||||||
|
# avatarFile=assets/<login>.png)}.
|
||||||
(cd "$PROJECT_DIR" && node <SKILL_DIR>/scripts/ingest.mjs \
|
(cd "$PROJECT_DIR" && node <SKILL_DIR>/scripts/ingest.mjs \
|
||||||
--pr-json ./capture/pr.json --diff ./capture/diff.patch --out-dir ./capture/extracted)
|
--pr-json ./capture/pr.json --diff ./capture/diff.patch --out-dir ./capture/extracted)
|
||||||
|
|
||||||
@@ -110,6 +111,8 @@ PR="<url | owner/repo#N | N>"
|
|||||||
|
|
||||||
If `fetch-pr.mjs` exits 1 (gh auth / not found / private), report its stderr and stop — **do not fabricate PR contents**. If `ingest.mjs` exits 1, read its stderr (usually a malformed `pr.json`), fix, and rerun (deterministic). `fetch-people-avatars.mjs` always exits 0; missing avatars just mean no credits close to author.
|
If `fetch-pr.mjs` exits 1 (gh auth / not found / private), report its stderr and stop — **do not fabricate PR contents**. If `ingest.mjs` exits 1, read its stderr (usually a malformed `pr.json`), fix, and rerun (deterministic). `fetch-people-avatars.mjs` always exits 0; missing avatars just mean no credits close to author.
|
||||||
|
|
||||||
|
`people.json` carries a `name` for whichever contributors `gh` already named (the PR author, commit authors, `mergedBy`) — `null` for the rest (reviewers/commenters/assignees, which `gh pr view` only ever gives a bare `login`). Before writing the credits close in Step 3, resolve any `null` name yourself for the 1-6 people who'll actually appear on that frame: `gh api users/<login> --jq .name` (you already have `gh` — no need to script this). If GitHub has no public name for that user either, fall back to the login on-screen and drop that person from the spoken line (see story-design.md's credits section — the voiceover must still say names, never raw handles).
|
||||||
|
|
||||||
**Gate:** `capture/pr.json`, `capture/diff.patch`, `capture/extracted/tokens.json`, `capture/extracted/visible-text.txt`, and `capture/extracted/people.json` exist; you can state the PR's change in one clear sentence. `assets/<login>.png` is best-effort — its absence is not a failure.
|
**Gate:** `capture/pr.json`, `capture/diff.patch`, `capture/extracted/tokens.json`, `capture/extracted/visible-text.txt`, and `capture/extracted/people.json` exist; you can state the PR's change in one clear sentence. `assets/<login>.png` is best-effort — its absence is not a failure.
|
||||||
|
|
||||||
---
|
---
|
||||||
|
|||||||
@@ -159,6 +159,8 @@ A PR is shipped by people, and every PR video closes with a `credits` frame nami
|
|||||||
|
|
||||||
The `credits` frame is an avatar row with names + roles + an "approved" check. On that frame only, set `asset_candidates` to 1–6 entries of `assets/<login>.png — <login>, <role>` (commit authors by `commitCount` first, then reviewers; only `avatarFetched: true` logins). The body stays code-only — avatars appear **only** on this close, never decorating a diff frame. The frame sits in the Step 3 proposal like any other, so the user can cut it there; skip it yourself only when no avatar was fetched.
|
The `credits` frame is an avatar row with names + roles + an "approved" check. On that frame only, set `asset_candidates` to 1–6 entries of `assets/<login>.png — <login>, <role>` (commit authors by `commitCount` first, then reviewers; only `avatarFetched: true` logins). The body stays code-only — avatars appear **only** on this close, never decorating a diff frame. The frame sits in the Step 3 proposal like any other, so the user can cut it there; skip it yourself only when no avatar was fetched.
|
||||||
|
|
||||||
|
> **Narrate the name, not the handle.** `people.json` carries a `name` field (GitHub display name, e.g. "Miguel Angel Simon Sierra") next to `login` for whichever contributors `gh` already named (author, commit authors, `mergedBy`); it's `null` for reviewers/commenters/assignees, which `gh pr view` only ever gives a bare login. Before writing this frame, resolve any `null` name yourself for the 1-6 people going on the close: `gh api users/<login> --jq .name`. Voiceover always says the **name** (first name is enough — "Shipped by Miguel, reviewed by Wenbo") and **never** reads a raw `@login` aloud (`@miguAng18947550` spoken by TTS is the failure mode this exists to avoid). On-screen text under each avatar can show both, name first, handle small and secondary: `Miguel Angel Simon Sierra` / `@miguel-heygen`. When a name still doesn't resolve (GitHub has no public name for that user either), fall back to the login on-screen and skip that person from the spoken line rather than reading the handle.
|
||||||
|
|
||||||
Every other frame has **no** `asset_candidates` (the visuals are invented downstream from `scene` + the diff).
|
Every other frame has **no** `asset_candidates` (the visuals are invented downstream from `scene` + the diff).
|
||||||
|
|
||||||
### Versions on the end card (cta / changelog)
|
### Versions on the end card (cta / changelog)
|
||||||
|
|||||||
@@ -157,11 +157,14 @@ const isBot = (login) => {
|
|||||||
// (often differs from the opener — a teammate force-pushes the branch, or commits
|
// (often differs from the opener — a teammate force-pushes the branch, or commits
|
||||||
// are co-authored). Commit authors are first-class contributors for a credits close.
|
// are co-authored). Commit authors are first-class contributors for a credits close.
|
||||||
const ROLE_ORDER = ["author", "committer", "reviewer", "commenter", "assignee"];
|
const ROLE_ORDER = ["author", "committer", "reviewer", "commenter", "assignee"];
|
||||||
const peopleMap = new Map(); // login -> { login, roles:Set, reviewState, association, commitCount }
|
const peopleMap = new Map(); // login -> { login, name, roles:Set, reviewState, association, commitCount }
|
||||||
const botsFiltered = new Set();
|
const botsFiltered = new Set();
|
||||||
// Returns the person record for a real (non-bot) login, creating it on first
|
// Returns the person record for a real (non-bot) login, creating it on first
|
||||||
// touch; records and drops bots. null means "skip this login".
|
// touch; records and drops bots. null means "skip this login". `name` is the
|
||||||
function consider(login) {
|
// GitHub display name (e.g. "Miguel Angel Simon Sierra") — gh only hands this
|
||||||
|
// over for author/commits/mergedBy, not reviewers/commenters/assignees, so it's
|
||||||
|
// filled in opportunistically and the first non-empty value wins.
|
||||||
|
function consider(login, name) {
|
||||||
if (!login) return null;
|
if (!login) return null;
|
||||||
if (isBot(login)) {
|
if (isBot(login)) {
|
||||||
botsFiltered.add(login);
|
botsFiltered.add(login);
|
||||||
@@ -170,27 +173,30 @@ function consider(login) {
|
|||||||
if (!peopleMap.has(login))
|
if (!peopleMap.has(login))
|
||||||
peopleMap.set(login, {
|
peopleMap.set(login, {
|
||||||
login,
|
login,
|
||||||
|
name: null,
|
||||||
roles: new Set(),
|
roles: new Set(),
|
||||||
reviewState: null,
|
reviewState: null,
|
||||||
association: null,
|
association: null,
|
||||||
commitCount: 0,
|
commitCount: 0,
|
||||||
});
|
});
|
||||||
return peopleMap.get(login);
|
const p = peopleMap.get(login);
|
||||||
|
if (!p.name && name) p.name = name;
|
||||||
|
return p;
|
||||||
}
|
}
|
||||||
|
|
||||||
const authorLogin = pr.author?.login || null;
|
const authorLogin = pr.author?.login || null;
|
||||||
{
|
{
|
||||||
const p = consider(authorLogin);
|
const p = consider(authorLogin, pr.author?.name);
|
||||||
if (p) p.roles.add("author");
|
if (p) p.roles.add("author");
|
||||||
}
|
}
|
||||||
|
|
||||||
// Commit authors — the people who actually wrote the code. pr.commits[].authors[]
|
// Commit authors — the people who actually wrote the code. pr.commits[].authors[]
|
||||||
// carries login/name/email; co-authored commits list several. Counts drive ordering
|
// carries login/name/email; co-authored commits list several. Counts drive ordering
|
||||||
// and the brief ("@login (N commits)"). Authors with no GitHub login (email-only)
|
// and the brief ("Name (@login, N commits)"). Authors with no GitHub login
|
||||||
// can't be avatar'd, so they're skipped here.
|
// (email-only) can't be avatar'd, so they're skipped here.
|
||||||
for (const c of Array.isArray(pr.commits) ? pr.commits : []) {
|
for (const c of Array.isArray(pr.commits) ? pr.commits : []) {
|
||||||
for (const a of Array.isArray(c?.authors) ? c.authors : []) {
|
for (const a of Array.isArray(c?.authors) ? c.authors : []) {
|
||||||
const p = consider(a?.login);
|
const p = consider(a?.login, a?.name);
|
||||||
if (!p) continue;
|
if (!p) continue;
|
||||||
p.roles.add("committer");
|
p.roles.add("committer");
|
||||||
p.commitCount += 1;
|
p.commitCount += 1;
|
||||||
@@ -209,7 +215,7 @@ if (!reviewSource.length && Array.isArray(pr.reviews)) {
|
|||||||
reviewSource = [...lastByAuthor.values()];
|
reviewSource = [...lastByAuthor.values()];
|
||||||
}
|
}
|
||||||
for (const r of reviewSource) {
|
for (const r of reviewSource) {
|
||||||
const p = consider(r?.author?.login);
|
const p = consider(r?.author?.login, r?.author?.name);
|
||||||
if (!p) continue;
|
if (!p) continue;
|
||||||
p.roles.add("reviewer");
|
p.roles.add("reviewer");
|
||||||
if (r.state) p.reviewState = r.state;
|
if (r.state) p.reviewState = r.state;
|
||||||
@@ -217,11 +223,11 @@ for (const r of reviewSource) {
|
|||||||
}
|
}
|
||||||
|
|
||||||
for (const c of Array.isArray(pr.comments) ? pr.comments : []) {
|
for (const c of Array.isArray(pr.comments) ? pr.comments : []) {
|
||||||
const p = consider(c?.author?.login);
|
const p = consider(c?.author?.login, c?.author?.name);
|
||||||
if (p) p.roles.add("commenter");
|
if (p) p.roles.add("commenter");
|
||||||
}
|
}
|
||||||
for (const a of Array.isArray(pr.assignees) ? pr.assignees : []) {
|
for (const a of Array.isArray(pr.assignees) ? pr.assignees : []) {
|
||||||
const p = consider(a?.login);
|
const p = consider(a?.login, a?.name);
|
||||||
if (p) p.roles.add("assignee");
|
if (p) p.roles.add("assignee");
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -239,6 +245,11 @@ const primaryRoleRank = (roles) => {
|
|||||||
const people = [...peopleMap.values()]
|
const people = [...peopleMap.values()]
|
||||||
.map((p) => ({
|
.map((p) => ({
|
||||||
login: p.login,
|
login: p.login,
|
||||||
|
// Display name for narration/on-screen credits — GitHub logins read aloud
|
||||||
|
// badly ("@miguAng18947550"). null when GitHub has no public name for this
|
||||||
|
// user and fetch-people-avatars.mjs couldn't resolve one either; the credits
|
||||||
|
// frame falls back to the login in that case.
|
||||||
|
name: p.name || null,
|
||||||
roles: ROLE_ORDER.filter((r) => p.roles.has(r)),
|
roles: ROLE_ORDER.filter((r) => p.roles.has(r)),
|
||||||
commitCount: p.commitCount || 0,
|
commitCount: p.commitCount || 0,
|
||||||
reviewState: p.reviewState || null,
|
reviewState: p.reviewState || null,
|
||||||
@@ -418,11 +429,14 @@ if (shippedVersion)
|
|||||||
lines.push("");
|
lines.push("");
|
||||||
|
|
||||||
// People & reviews — human context for an optional credits / shipped-by close.
|
// People & reviews — human context for an optional credits / shipped-by close.
|
||||||
// Avatars land in assets/<login>.png (downloaded by the orchestrator).
|
// Avatars land in assets/<login>.png (downloaded by the orchestrator). Each
|
||||||
|
// person is labeled "Name (@login)" — the credits close speaks the name, the
|
||||||
|
// handle is display-only (never read aloud; see story-design.md).
|
||||||
|
const label = (p) => (p.name ? `${p.name} (@${p.login})` : `@${p.login}`);
|
||||||
if (people.length) {
|
if (people.length) {
|
||||||
lines.push("## People & reviews");
|
lines.push("## People & reviews");
|
||||||
const authorPerson = people.find((p) => p.roles.includes("author"));
|
const authorPerson = people.find((p) => p.roles.includes("author"));
|
||||||
if (authorPerson) lines.push(`Author (opened PR): @${authorPerson.login}`);
|
if (authorPerson) lines.push(`Author (opened PR): ${label(authorPerson)}`);
|
||||||
const committers = people.filter((p) => p.roles.includes("committer"));
|
const committers = people.filter((p) => p.roles.includes("committer"));
|
||||||
if (committers.length) {
|
if (committers.length) {
|
||||||
const parts = committers
|
const parts = committers
|
||||||
@@ -430,7 +444,7 @@ if (people.length) {
|
|||||||
.sort((a, b) => b.commitCount - a.commitCount)
|
.sort((a, b) => b.commitCount - a.commitCount)
|
||||||
.map(
|
.map(
|
||||||
(p) =>
|
(p) =>
|
||||||
`@${p.login}${p.commitCount ? ` (${p.commitCount} commit${p.commitCount === 1 ? "" : "s"})` : ""}`,
|
`${label(p)}${p.commitCount ? ` (${p.commitCount} commit${p.commitCount === 1 ? "" : "s"})` : ""}`,
|
||||||
);
|
);
|
||||||
lines.push(`Commit authors: ${parts.join(", ")}`);
|
lines.push(`Commit authors: ${parts.join(", ")}`);
|
||||||
}
|
}
|
||||||
@@ -438,7 +452,7 @@ if (people.length) {
|
|||||||
if (reviewers.length) {
|
if (reviewers.length) {
|
||||||
const parts = reviewers.map(
|
const parts = reviewers.map(
|
||||||
(p) =>
|
(p) =>
|
||||||
`@${p.login}${p.reviewState ? ` (${REVIEW_STATE_LABEL[p.reviewState] || p.reviewState.toLowerCase()})` : ""}`,
|
`${label(p)}${p.reviewState ? ` (${REVIEW_STATE_LABEL[p.reviewState] || p.reviewState.toLowerCase()})` : ""}`,
|
||||||
);
|
);
|
||||||
lines.push(`Reviewers: ${parts.join(", ")}`);
|
lines.push(`Reviewers: ${parts.join(", ")}`);
|
||||||
}
|
}
|
||||||
@@ -446,8 +460,7 @@ if (people.length) {
|
|||||||
(p) =>
|
(p) =>
|
||||||
p.roles.includes("commenter") && !p.roles.includes("author") && !p.roles.includes("reviewer"),
|
p.roles.includes("commenter") && !p.roles.includes("author") && !p.roles.includes("reviewer"),
|
||||||
);
|
);
|
||||||
if (commentersOnly.length)
|
if (commentersOnly.length) lines.push(`Commenters: ${commentersOnly.map(label).join(", ")}`);
|
||||||
lines.push(`Commenters: ${commentersOnly.map((p) => `@${p.login}`).join(", ")}`);
|
|
||||||
if (reviewDecision) lines.push(`Review decision: ${reviewDecision}`);
|
if (reviewDecision) lines.push(`Review decision: ${reviewDecision}`);
|
||||||
if (mergedByLogin) lines.push(`Merged by: @${mergedByLogin}`);
|
if (mergedByLogin) lines.push(`Merged by: @${mergedByLogin}`);
|
||||||
lines.push(`Avatars: assets/<login>.png (${people.length} contributor(s) — see people.json)`);
|
lines.push(`Avatars: assets/<login>.png (${people.length} contributor(s) — see people.json)`);
|
||||||
|
|||||||
Reference in New Issue
Block a user