Skip to content

Add NPC portrait bridge generation#137

Merged
mfoltz merged 6 commits into
mainfrom
codex/npc-portrait-bridge-clean
May 7, 2026
Merged

Add NPC portrait bridge generation#137
mfoltz merged 6 commits into
mainfrom
codex/npc-portrait-bridge-clean

Conversation

@mfoltz
Copy link
Copy Markdown
Owner

@mfoltz mfoltz commented May 6, 2026

Summary

  • Add a Blood Hunts localization GUID bridge so blood-hunts-map.json records the Unity localization GUID derived from each Blood Hunts name key and verifies it against English localization text.
  • Add NPC portrait candidate and promoted portrait snapshots generated from source-backed and approved/user-attested joins.
  • Wire the new portrait snapshots into npm run refresh:db-assets, generated-data validation, coverage reporting, and npm test.

Interface / data shape

  • data/enrichment/blood-hunts-map.json entries now include nameLocalizationGuid and localized-text provenance.
  • New generated artifacts:
    • data/enrichment/npc-portrait-candidates.json
    • data/enrichment/npc-portrait-map.json
  • npm test now runs both scripts/blood-hunts.test.ts and scripts/npc-portraits.test.ts.

Verification

  • npx tsx scripts/blood-hunts.test.ts
  • npx tsx scripts/npc-portraits.test.ts
  • $env:VRISING_ASSET_DUMP_DIR='C:\Users\mitch\Local\Assets'; npm run refresh:db-assets
  • npm run generate:db
  • npm run validate:data
  • npx tsc --noEmit
  • npm run verify
  • npm test

Notes:

  • tsx and Vite/esbuild commands hit sandbox spawn EPERM when run inside the restricted shell; reruns outside the sandbox passed.
  • validate:data and npm run verify pass while reporting existing enrichment threshold warnings plus the new npc-portrait-map warning: coverage 15.94% is below the 50.00% target and 182 low-signal rows are excluded from matched coverage.
  • This PR is opened as draft because the generated portrait candidate set is large and intended to receive provenance/approval review before merge close-out.

@mfoltz mfoltz marked this pull request as ready for review May 6, 2026 18:46
@mfoltz
Copy link
Copy Markdown
Owner Author

mfoltz commented May 6, 2026

@codex[agent] Please review PR and provide feedback, as applicable.

@Codex
Copy link
Copy Markdown

Codex AI commented May 6, 2026

@codex[agent] Please review PR and provide feedback, as applicable.

Reviewed the PR and ran npm run verify locally; it passes (with the expected enrichment threshold warnings, including npc-portrait-map at 15.94% vs 50% target, and broad-control validation skipped because VRising.DataExtractor isn’t present in this workspace).

Feedback:

  • The Blood Hunts GUID bridge looks solid: scripts/blood-hunts.ts derives nameLocalizationGuid from the Unity Name.Key chunks and scripts/validate-generated-data.ts enforces it.
  • The portrait pipeline separation is nice: scripts/npc-portraits.ts produces a verbose candidate snapshot and only promotes source-backed + explicitly approved user-attested into npc-portrait-map.json, with validations to prevent unsafe prefabs and unapproved promotion.
  • The new tests (scripts/blood-hunts.test.ts, scripts/npc-portraits.test.ts) are self-contained and exercise the intended guardrails.

Nits / follow-ups to consider:

  • unsafePrefabPattern is duplicated (generator vs validation); exporting a shared constant would prevent drift.
  • enrichment-coverage.json currently uses lowSignalExcluded for npc-portrait-map as a count of circumstantial candidate assets (182), which makes the generic warning text (“row(s)”) a bit misleading; consider switching that metric to a prefab-level count or adjusting the warning wording for this domain.
  • npm ci reports 5 npm audit vulnerabilities (pre-existing from lockfile); not blocking for this PR, but worth tracking separately.

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: f133422961

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread scripts/validate-generated-data.ts Outdated
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 046361155f

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread scripts/refresh-db-assets.ts
@mfoltz mfoltz merged commit 001afc8 into main May 7, 2026
1 check passed
@mfoltz mfoltz deleted the codex/npc-portrait-bridge-clean branch May 7, 2026 01:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants