Thumbnail logo detection ignores logos already recorded in manifest.shared

Open Beginner friendly
  • Is something wrong?
#246 0 comments 0 reactions 0 assignees View on GitHub

Maintainers usually reply within 1 day

Nobody has claimed this yet.

Assessment

Difficulty
2/5
Estimated time
1-3 hours
Newbie friendliness
84/100
Issue type
Bug
Clarity
Clearly specified
Activity status
Active
Tech stack
javascript, node.js
Domain
backend, data

Research direction

Read the shared-logo drop loop in site/thumbs.mjs:311-318 and sharedImageIds in site/lib/thumbs.mjs:162-172, then inspect the existing sharedImageIds test in site/lib/lib.test.mjs. Add a test for one new record whose hash already exists in shared, and run npm test. Done means the new record is moved to shared and the test passes.

Written by the indexing model from the issue text.

Description

bug good first issue priority: medium

Summary

The three-or-more shared-hash rule that keeps publisher logos out of thumbnails only counts records fetched in the current run, so a logo that was already dropped into manifest.shared is accepted again when a single new entry from the same site is fetched.

Where

  • site/thumbs.mjs:311-318 (logo drop loop)
  • site/thumbs.mjs:263-266 (shared skip)
  • site/lib/thumbs.mjs:162-172 (sharedImageIds)

Details

for (const id of sharedImageIds(thumbs)) {   // counts hashes in `thumbs` only
  shared[id] = thumbs[id];
  delete thumbs[id];
  pending.delete(id);
}
for (const id of Object.keys(thumbs)) delete shared[id];

Records dropped in an earlier run live in shared, not thumbs, so they never count toward the limit again. The existing test (site/lib/lib.test.mjs, the sharedImageIds case) only covers three records sitting in thumbs together.

Impact

A publisher's site-wide logo or social card is published as an asset preview with a "Preview image shown under " credit, which is the case the shared-hash rule (#132) exists to block.

Steps to reproduce

  1. Entries A, B and C on one site share an og:image; a run drops them into manifest.shared.
  2. Add entry D from the same site (or change A's url).
  3. Run node site/thumbs.mjs: D's image hash appears once in thumbs, below the limit, so D keeps the logo. Pure check: sharedImageIds({ D: { hash: "h" } }) is empty even when shared holds three records with hash h.

Suggested fix

Count hashes across { ...shared, ...thumbs } and drop only the ids that are in thumbs; or keep a set of known logo hashes and reject any fetched image whose hash is in it.

Acceptance criteria

  • A newly fetched record whose hash matches a hash already in shared is moved to shared
  • Unit test: one new record plus an already recorded shared hash

Getting started

  • Relevant files: site/thumbs.mjs, site/lib/thumbs.mjs, site/lib/lib.test.mjs
  • Run the tests: npm test; the full local gate is npm run check (Node 22+, no install step)
  • Related docs: CONTRIBUTING.md, site/README.md

Related: #232, #132

Filed by repo-audit/full-audit at 4adb944. Severity: medium. Effort: S. audit-key: bug|site/thumbs.mjs|sharedImageIds

Dominant language
JavaScript
Stars
58
Forks
15
Avg merge
1d 15h
Merged PRs (30d)
12

Getting set up

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from TMHSDigital/Free-Game-Dev-Assets

All issues in TMHSDigital/Free-Game-Dev-Assets

Similar issues

More JavaScript issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.