Skip to content

[l10n/automation] Manifest contract and catalog diff engine#940

Open
Chaitanya-Keyal wants to merge 1 commit into
SeedSigner:devfrom
Chaitanya-Keyal:l10n/diff-engine
Open

[l10n/automation] Manifest contract and catalog diff engine#940
Chaitanya-Keyal wants to merge 1 commit into
SeedSigner:devfrom
Chaitanya-Keyal:l10n/diff-engine

Conversation

@Chaitanya-Keyal

@Chaitanya-Keyal Chaitanya-Keyal commented Jun 22, 2026

Copy link
Copy Markdown
Contributor

Note

One of the l10n source-string automation PRs (#940, #941, #942, #943, #944). This PR has no dependencies on the others.

Description

Problem or Issue being addressed

To tell reviewers and translators what a PR changes, we need
(a) a stable, machine-checkable contract for that data and
(b) a diff between two gettext catalogs that reports only translator-meaningful changes, ignoring source-location moves and header churn.

Solution

Adds the foundation for the source-string automation:

  • schema/review-manifest.schema.json -- the "review manifest" contract (JSON Schema draft 2020-12) for the data passed from the untrusted PR diff job to the trusted comment job. It is strict by design: additionalProperties: false, a const version, bounded array/string lengths, and SHA/owner/name patterns on the trust-anchor fields.
  • pot_diff.py -- keys catalog entries on (msgctxt, msgid) and reports added/removed/changed strings, then assembles a manifest conforming to the schema. Counts are always exact; the per-bucket example lists are capped (and flagged truncated) so a huge diff cannot produce an unbounded manifest. The committed messages.pot is never read -- the catalog is extracted fresh from base and head source -- so the result is correct regardless of whether the committed catalog is current.

This pull request is categorized as a:

  • Other (l10n automation)

Checklist

I ran pytest locally

  • All tests passed before submitting the PR

(python -m pytest l10n/automation/tests -- these tests live outside the project's default testpaths and are run explicitly.)


I included screenshots of any new or modified screens

  • N/A

I added or updated tests

  • Yes

I tested this PR hands-on on the following platform(s):

  • Github Actions

I have reviewed these notes:

  • Keep your changes limited in scope.
  • If you uncover other issues or improvements along the way, ideally submit those as a separate PR.
  • The more complicated the PR, the harder it is to review, test, and merge.
  • We appreciate your efforts, but we're a small team of volunteers so PR review can be a very slow process.
  • Please only "@" mention a contributor if their input is truly needed to enable further progress.
  • I understand

Adds the review-manifest JSON Schema -- the contract for the data passed from
the untrusted PR diff job to the trusted comment job -- and pot_diff.py, which
computes a translator-meaningful diff between two gettext catalogs (keyed on
msgctxt + msgid + plural, ignoring source-location and header churn) and
assembles a manifest conforming to that schema. Includes tests.
@@ -0,0 +1,119 @@
{
"$schema": "https://json-schema.org/draft/2020-12/schema",
"$id": "https://seedsigner.com/l10n/schema/review-manifest.schema.json",

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

What was the reasoning behind using https://seedsigner.com/l10n/schema/review-manifest.schema.json as the $id? The URL isn't reachable right now. Do you plan to host the schema there eventually?

Comment on lines +197 to +202
payload = json.dumps(manifest, indent=2, ensure_ascii=False)
if args.out == "-":
print(payload)
else:
with open(args.out, "w", encoding="utf-8") as fh:
fh.write(payload + "\n")

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Since ensure_ascii=False is used here, payload can contain raw non-ASCII characters. If the runner's stdout happens to be bound to ASCII, print(payload) would crash with a UnicodeEncodeError.

The file-output path below already handles this correctly with encoding="utf-8".
Should the stdout path do something similar (e.g. writing to sys.stdout.buffer with explicit UTF-8 encoding)?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Summer of Bitcoin

Development

Successfully merging this pull request may close these issues.

2 participants