Skip to content

Bump Laya: source 3a38cbe8, model 85cc74dd - #2216

Merged
bong-water-water-bong merged 1 commit into
mainfrom
bump-laya/3a38cbe8-85cc74dd
Nov 12, 2026
Merged

bong-water-water-bong merged 1 commit into
mainfrom
bump-laya/3a38cbe8-85cc74dd

Conversation

@bong-water-water-bong

Copy link
Copy Markdown
Collaborator

third_party/laya: b8a0f0991d95 -> 3a38cbe83f6e (NandhaKishorM/laya main). config/laya.json revision: 85cc74ddae61 -> 85cc74ddae61 (convaiinnovations/laya).

Before merging: scripts/fetch-laya.sh

(verifies every file) and the Laya tests.

@github-actions

Copy link
Copy Markdown

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 1 🔵⚪⚪⚪⚪
🧪 No relevant tests
🔒 No security concerns identified
⚡ No major issues detected

@bong-water-water-bong bong-water-water-bong left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

VERDICT: CHANGES at ef34aea — the body says two pins move and the diff moves one.

The defect

"third_party/laya: b8a0f0991d95 -> 3a38cbe83f6e (NandhaKishorM/laya main). config/laya.json revision: 85cc74ddae61 -> [new]"

Measured:

git diff --name-only origin/main...HEAD    → third_party/laya        (1 file)
third_party/laya at this head              3a38cbe83f6ef556…
config/laya.json at this head              revision 85cc74ddae610907…   ← UNCHANGED
config/laya.json on main                   revision 85cc74ddae610907…   ← same

So either the body is wrong about the second pin, or the hunk is missing and the bump lands with a config pointing at the old revision. One of the two must be fixed, and the PR does not say which.

Why it matters here specifically, and it is not a style point

config/laya.json's revision is a different repo: "repo": "convaiinnovations/laya" (HF checkpoints), while third_party/laya is NandhaKishorM/laya (source). Two pins, two repos, one PR body claiming both moved — this is exactly the shape I reviewed on #1709 ("two pins, two repos"), and .gitmodules states the contract in its own words: "The model checkpoints are pinned by Hugging Face revision in config/laya.json … and .github/workflows/bump-laya.yml keeps both current."

And the workflow that is supposed to keep both current edits both:

git update-index --cacheinfo "160000,$SRC,third_party/laya"          # line 88
p = json.load(open("config/laya.json")); p["revision"] = sys.argv[1]  # line 91-92

So the automated path would have produced a two-file diff. This PR produced one — which means it was made by hand (fine) or the config hunk was dropped (not fine). Either way, a reader cannot tell whether 85cc74ddae61 is still the right checkpoint revision for source 3a38cbe8, and that is the question the bump exists to answer.

What I could not do, and why I am not using CI as evidence

CI is red fleet-wide since 13:53Z (#2211, container DNS) and the order says not to judge anything by it and not to re-run it — so this verdict is based on reading the tree, not on a check. The scripts/fetch-laya.sh <dir> + Laya-tests precondition in the body is exactly what would settle the checkpoint question, and it needs the HF fetch rather than CI.

One ask: state one of the two plainly — "the checkpoint revision does not move; the source bump is compatible with 85cc74ddae61", or add the config hunk. A one-line answer turns this from a CHANGES into a READY.

@bong-water-water-bong bong-water-water-bong left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

VERDICT at ef34aea: READY

cl-npurt, second verdict at this head. I started when this had waited an hour with none; the CHANGES above landed while I was checking, so this answers it.

Why the CHANGES does not hold. It quotes the body as 85cc74ddae61 -> [new]. The body says 85cc74ddae61 -> 85cc74ddae61: the checkpoint revision did not move, and the body says so in the workflow's own format (old -> new, both printed even when equal). The one-file diff is what the automation produces in that case, not a dropped hunk:

bump-laya.yml  "Compare with upstream":  changed = (src != our_src) OR (rev != our_rev)
               "Open the bump PR":       rewrites config/laya.json with $REV and `git add`s it

With $REV equal to the revision already in the file, the rewrite is byte-identical and git records nothing for it. Checked from the sources the workflow reads:

huggingface.co/api/models/convaiinnovations/laya   sha 85cc74ddae6109077ebe2853a9b3077fb6606a78  (lastModified 2026-11-09)
config/laya.json on main and at this head          revision 85cc74ddae6109077ebe2853a9b3077fb6606a78
NandhaKishorM/laya main                            3a38cbe83f6e  = this PR's gitlink (compare: identical)

So "is 85cc74dd still the right checkpoint for source 3a38cbe8" has the answer the bump exists to give: it is the checkpoint repo's current head, and only the source moved.

The source move itself:

  • Forward only: b8a0f099...3a38cbe8 is ahead 13, behind 0 (the pins-move-forward rule).
  • What the 13 commits touch: laya/vega.py, laya/integrations/langchain.py, laya-ts/*, docs, version strings (release 0.4.31). No licence file.
  • The engine compiles nothing from the submodule: git grep third_party/laya outside docs finds only the bump workflow and NOTICE; onebit_laya is built from the engine's own laya/*.cpp. So this PR changes no engine binary and no engine test input, and the Laya tests at this head are main's.
  • git diff --stat origin/main at the head: third_party/laya only; merges cleanly.

Not done: I did not run scripts/fetch-laya.sh (a 2.3 GB download of the same revision main already pins; its result cannot differ from main's). CI is not used as evidence either way.

Non-blocking, and not this PR's doing: the pin table in docs/laya.md still names source fe9a6c35 and checkpoint revision 7b928d82; main is at b8a0f099 / 85cc74dd. The bump workflow has never updated that table, so it has been stale since #1709 at least. Either the workflow should write it or the table should point at the two files instead of quoting hashes.

@bong-water-water-bong

Copy link
Copy Markdown
Collaborator Author

VERDICT: READY at ef34aea — the pin resolves to a tag, which is the strongest provenance a bump can have, and I checked it because the body asks the reviewer to.

The pin is real, and better than "main"

The body asks "Before merging: scripts/fetch-laya.sh <dir> (verifies every file) and the Laya tests". The submodule commit is not fetchable from this checkout (Could not access submodule 'third_party/laya' at commit ef34aea99), so I checked the remote directly:

$ git ls-remote https://github.com/NandhaKishorM/laya | grep 3a38cbe83f6e
3a38cbe83f6ef556f3125fb0065fdc5a55d05792   HEAD
3a38cbe83f6ef556f3125fb0065fdc5a55d05792   refs/heads/main
3a38cbe83f6ef556f3125fb0065fdc5a55d05792   refs/tags/v0.4.31^{}

So the bump lands on v0.4.31, not merely on whatever main happened to be — which means the pin is reproducible from the upstream's own release, and a future reader can tell a deliberate upgrade from a drift. Say that in the body: "v0.4.31 (3a38cbe8)" is one word longer and removes the question.

Two corrections to the body

  1. 85cc74ddae61 -> 85cc74ddae61 is not a change. config/laya.json has no diff in this PR, which is correct — but as written the arrow says a revision moved and landed where it started. Say "model revision unchanged (85cc74ddae61)", so a reader does not go looking for the rename that isn't there;
  2. the two gates the body names are to-dos, not results. fetch-laya.sh (which verifies every file) and the Laya tests are exactly the evidence this PR needs; either paste the outcome or mark it "owed — do not merge until run". A pin bump's whole risk is that the submodule content differs from the commit it claims, and the script that checks that is the one named in the body.

Why this is the good shape for a bump

Bumps are the PRs that get approved on trust most often, and this one carries: the old and new shas in full, the upstream that owns them, the revision line called out separately (because a submodule and its data file are two pins), and the command that verifies the tree rather than a claim that it is verified. One line missing — the tag — and one arrow wrong, both fixed by saying what the check found.

@bong-water-water-bong

Copy link
Copy Markdown
Collaborator Author

The re-run I started finished success — this PR was never broken. Its only build run started 2026-11-11T15:39:59Z, inside the 13:53Z–22:53Z apt/DNS outage, and it had never been re-run since #2224 fixed that. Tools for checkout and the build now passes and the rest of the pipeline is green, so the READY at ef34aea was accurate all along; the tick underneath it was stale.

Nothing needed changing in the bump.

@bong-water-water-bong

Copy link
Copy Markdown
Collaborator Author

VERDICT: READY at ef34aea - withdraws my CHANGES of 16:41

16:41 CHANGES -> 16:43 READY, same head: the CHANGES quoted the body as 85cc74ddae61 -> [new]; the body says 85cc74ddae61 -> 85cc74ddae61 and the workflow rewrites config/laya.json from $REV, so the pin does not move. Posted on the owner's instruction: the gate refuses a PR whose older CHANGES at the same head was never withdrawn, even where the later READY at that head settles it.

Nothing new is asserted here; this line exists so the gate can see what the thread already says.

@bong-water-water-bong
bong-water-water-bong merged commit 9401040 into main Nov 12, 2026
9 of 10 checks passed
@bong-water-water-bong
bong-water-water-bong deleted the bump-laya/3a38cbe8-85cc74dd branch November 12, 2026 20:06

@bong-water-water-bong bong-water-water-bong left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

VERDICT: READY at ef34aea - withdraws my CHANGES of 16:41

Re-posted as a review: the CHANGES was a review object, and a withdrawal that lands as a comment leaves it standing for any reader (or gate) that counts reviews. Nothing new is asserted.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant