Skip to content

Rewrite the backlog around what is left after the container works - #1241

Merged
Frotty merged 4 commits into
masterfrom
docs/backlog-after-fasthashmap
Aug 16, 2026
Merged

Rewrite the backlog around what is left after the container works#1241
Frotty merged 4 commits into
masterfrom
docs/backlog-after-fasthashmap

Conversation

@Frotty

Copy link
Copy Markdown
Member

Documentation only. The goal these items were organised around is met, so the Todo said things that are no longer true — item 13 read as open when its Jass half landed in #1237, and the ordering still pointed at the feature rather than at what surrounds it.

What changed:

  • Done records what actually landed: the container works on both targets and against the standard library on Jass, the Jass half of subclassing, and byte-oriented strings with the library bump they unblocked. Each notes what was wrong in the earlier analysis, since two items here previously asserted a cause that turned out to be false.
  • Two new items for gaps in what the suite can see. Nothing executes a standard library program on Lua (16), and a bump of the pinned library is only checked for compiling (17). Both were discovered while shipping the container, and the first is how a specialised class allocating no fields at all survived — the paths that would have caught it are compiled and never run.
  • The erasure question is one item (18) instead of the same paragraph repeated under items 6 and 13. Both end there, and Keep the fields a specialised class allocates, and compile FastHashMap against the standard library #1239 is a reason to take it seriously: two class shapes existing at once is what produced the field bugs, and while those are fixed, the shape that produced them is not.
  • The flaky determinism test is item 20, having blocked Bump the pinned standard library, and check its string handling #1240 for a run and passed on re-run of the same commit.
  • Item 9 no longer names WURST_LANGUAGE.md, which is not in this repository.
  • Item 10 gains the note that moveFunctionsOutOfClass copies type variables deliberately and Give a super call the type arguments of the class it reaches #1237 depends on that copy, so what is being made canonical is per scope rather than per program.

The goal these items were organised around is met, so the Todo said things
that are no longer true - item 13 read as open when its Jass half landed, and
the ordering still pointed at the feature rather than at what surrounds it.
What is left is generality (items 6, 7, 13's Lua half, 15) and two gaps in
what the suite can see: no standard library program is ever executed on Lua,
and a bump of the pinned library is only checked for compiling. Both are
recorded as items rather than as asides, because the first is how the
empty-allocation bug survived. Items 6 and 13 both end at the same erasure
decision, which is now item 18 rather than a paragraph repeated in each.

@chatgpt-codex-connectorchatgpt-codex-connectorBot left a comment

Copy link
Copy Markdown

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:bb2f32d5d8

ℹ️ 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 threadBACKLOG.md Outdated
Comment on lines +126 to +130
20. **`luaOutputIsDeterministicForGenericOverrideSlots` is itself nondeterministic.** It failed once
on Windows CI and passed on a re-run of the same commit, having blocked an unrelated pull
request in between. A test asserting the compiler is deterministic which is not itself
deterministic is worse than none: every failure has to be investigated before it can be
dismissed, and the next one will be too. Either find what differs between the two compiles or

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Keep the required compile-twice determinism check

Do not replace this assertion merely because it fails intermittently: the test compiles the same repro twice and compares byte-identical output, so an intermittent mismatch is evidence of the Lua emission nondeterminism it is specifically required to detect unless harness interference is first proven. Keep the compile-twice equality check and diagnose the differing output or unstable compiler iteration instead of weakening the assertion.

AGENTS.md reference: AGENTS.md:L277-L280

Useful? React with 👍 / 👎.

Comment threadBACKLOG.md Outdated
Jass, and compiles on Lua (#1239). What is left is generality around it rather than the feature
itself, and one gap in what the suite can see.

16. **Nothing executes a standard library program on Lua.** Every test which puts the library on

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Assign unused IDs to the new backlog items

The new Todo entries reuse 16, 17, and 18 even though completed items with those IDs remain under Done at lines 213–225. This defeats the stated stable-numbering scheme and makes references such as “item 18” ambiguous between the new erasure decision and the completed unresolved-default fix; allocate fresh numbers instead.

Useful? React with 👍 / 👎.

Comment threadBACKLOG.md Outdated
Comment on lines +83 to +86
9. **Keep `CHANGELOG.md` current** as items land — a standing practice rather than a task to
finish. `WURST_LANGUAGE.md`, which this item used to name, is not in this repository: the
language documentation lives in the docs repo, so the changelog is the only dev-facing surface
reachable from here. It covers type class bounds with a worked example.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Keep the tracked language digest in the update list

The repository does contain de.peeeq.wurstscript/src/main/resources/agent-docs/WURST_LANGUAGE.md, and it is a tracked compiler resource that already documents type-class bounds and closure behavior. Removing it from this standing item based on the false claim that it is external will let that shipped language reference drift as the remaining behavior lands.

Useful? React with 👍 / 👎.

…eference
Three corrections to the rewrite.
The new items reused 16, 17 and 18, which Done already holds, so a reference
to "item 18" was ambiguous between the erasure decision and a finished one.
They are 21, 22 and 23 now, and the determinism item is 24.
That item said the assertion could be replaced if diagnosing it proved hard.
It compiles one repro twice and compares byte for byte, so an intermittent
mismatch is evidence of intermittent nondeterminism in Lua emission, which is
what it exists to catch. Calling it flaky on the strength of a passing re-run
was too quick, and the item says so and says to diagnose instead.
WURST_LANGUAGE.md is in this repository, under the compiler's resources rather
than at the root, which is where I looked. Item 9 names it again, and since
the standing practice is to fold documentation into whichever item changes the
behaviour, the reference now covers subclassing a bounded generic (and that it
does not work on Lua) and strings being bytes.
@Frotty

Copy link
Copy Markdown
MemberAuthor

@codex review

@chatgpt-codex-connectorchatgpt-codex-connectorBot left a comment

Copy link
Copy Markdown

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:81159183e2

ℹ️ 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 threadBACKLOG.md
which asserts the program compiles, runs, and never reaches `testSuccess` rather than leaving
the difference between the targets to be discovered. `transformGenericNewOnly` runs neither
`simplifyClasses` nor `addMemberTypeArguments`, so the type variables are never lifted there and
a super call has nothing to carry. Closing it is item 23.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P3 Badge Update the test's erasure-item cross-reference

After centralizing the Lua erasure work under item 23, TypeClassTests.java:351 still says that the remaining subclassing issue is tracked by item 5. Item 5 is now listed under Done, so anyone following that test's JavaDoc is directed to completed closure work rather than this decision item; update the JavaDoc as part of the renumbering.

Useful? React with 👍 / 👎.

Its JavaDoc named item 5, which is finished, so a reader following it landed
on closure work rather than the decision the test is waiting on. The erasure
question is item 23 now that it is one item instead of a paragraph repeated
under two.
@Frotty

Copy link
Copy Markdown
MemberAuthor

All three addressed, and two of them were errors of mine rather than differences of opinion.

Keep the compile-twice determinism check — you are right, and I was too quick to call it flaky. It compiles one repro twice and compares byte for byte, so an intermittent mismatch is evidence of intermittent nondeterminism in Lua emission, which is the thing it exists to catch; a re-run passing says the nondeterminism is intermittent, not that the test is at fault. The item now says so in those terms, says not to weaken the assertion, and says to diagnose instead — capture both outputs on a failing run and diff them, and prove harness interference rather than assume it. One thing already ruled out: WurstScriptTest clears the global caches around every test method, so both compiles do start from the same cache state.

Assign unused IDs — correct, Done already holds 16, 17, 18 and 19. The new items are 21, 22, 23 and 24, and the cross references in items 6, 13 and 22 follow.

Keep the tracked language digest — also correct, and my claim that it was external was simply wrong: I searched two levels from the repository root and it sits under src/main/resources/agent-docs. Item 9 names it again.

Since the standing practice is to fold documentation into whichever item changes the behaviour, and two items landed without doing so, the digest now also covers what shipped: subclassing a bounded generic class and that it does not work on Lua, and strings being byte sequences on both targets, including that a compiletime expression cannot return half a character.

Also caught by the follow-up: TypeClassTests.subclassOfBoundedGenericIsStillBrokenOnLua pointed at item 5, which is finished. It points at item 23 now, and that is the only backlog reference in code.

@codex review

@chatgpt-codex-connectorchatgpt-codex-connectorBot left a comment

Copy link
Copy Markdown

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:06b2626582

ℹ️ 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 threadBACKLOG.md Outdated
Comment on lines +138 to +139
interference rather than assuming it. `WurstScriptTest` clears the global caches around every
test method, so the two compiles do start from the same cache state; whatever differs is

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P3 Badge Point to the cache clear between the two compiles

@BeforeMethod and @AfterMethod only clear caches before and after the entire test method, so they do not by themselves give the two internal compilations the same starting state. That is actually guaranteed by the explicit GlobalCaches.clearAll() between the compilations in LuaTranslationTests.luaOutputIsDeterministicForGenericOverrideSlots; naming that reset matters when diagnosing this failure, because removing or bypassing it would invalidate the stated conclusion about cache interference.

Useful? React with 👍 / 👎.

The item said clearing the caches around every test method is what gives the
two compilations the same starting state. That is half of it: the before-method
reset covers the first compile, and the explicit clearAll between them covers
the second. Naming the wrong one matters when diagnosing, because removing the
in-test reset would invalidate the conclusion rather than explain the failure.
@Frotty

Copy link
Copy Markdown
MemberAuthor

Right, and the distinction matters for exactly the reason given: the conclusion depends on which reset is doing the work.

The item now names both. @BeforeMethod clears before the first compilation, and the explicit GlobalCaches.clearAll() between the two compilations inside the test clears before the second; both are load bearing, and removing either stops the comparison being between equal starting states, which would invalidate the conclusion rather than explain the failure.

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Chef's kiss.

Reviewed commit:b3bd719407

ℹ️ 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".

@Frotty
Frotty merged commit 16f8f23 into masterAug 16, 2026
6 checks passed
@Frotty
Frotty deleted the docs/backlog-after-fasthashmap branch August 16, 2026 15:00
Sign up for freeto 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.

1 participant

@Frotty