Repository navigation
Refresh entry and transitive site assets together - #246
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
d6g8k5htny-coder
left a comment
d6g8k5htny-coder
left a comment
There was a problem hiding this comment.
Dylan Roy — delegated AI review. Actual performer: OpenAI / Codex agent /root/asset_release_review. Fresh nonauthor engineering source review; personal reading PENDING; same-provider organizational-independence credit 0. This is engineering review, not mathematical, human or organizational-independent acceptance.
Disposition: AMEND for head 7ae3505, tree c4ef29bdd7d3dfa94226a825e127a6fbcc97e3ef, against exact base 0ae8bd8. Reviewed the authoritative remote 29-file delta, repository instructions and relevant frontend/data/startup interfaces. All 31 supplied candidate file blobs matched local bytes when checked (two were unchanged relative to remote base). No implementation contribution or prior implementation exposure; reviewed original source before writer supplied successor information. No product edits, browser execution, deployment or merge performed by this reviewer.
F1 (P2): tools/site_asset_release.py CSS matcher/update dispatch silently misses valid quoted local stylesheet imports. Negative control: index.html loads style.css, containing @import "theme.css"; theme.css exists. Generation exits 0 and --check exits 0, leaving that import bare. A returning visitor can retain stale transitive CSS, contradicting the required missed-URL check. Support this syntax or refuse unsupported local imports, with a regression test; missing imported assets must also fail. Current checked-in asset syntax passed, so this finding concerns the generator's promised enforcement boundary.
F2 (P2 integration): required conditional-projection check is failed on this head. Original tests/test_museum_conditionals_cli.mjs expects bare src="conditionals.mjs" and src="museum.mjs", while original tests/test_museum_conditionals_startup.mjs loads bare modules and conditionals dynamically loads the release-qualified museum, creating separate startup caches in the test. Browser HTML already loads matching release-qualified URLs. Update fixtures to consume actual entry URLs while retaining one-startup assertions; do not weaken product invariants. Independently inspected exact-head test files and hosted logs: 107 tests, 103 pass, 2 fail, 2 skip; replay step skipped. https://github.com/d6g8k5htny-coder/main/actions/runs/37139481220
Actual local checks on original candidate asset bytes: generator --check passed (dc3b8a3d50e06b183520769b998318b774231b7fc56eda438e4a4878b963d1a3); generator/browser-contract unittest run 9 passed; reader-link unittest run 15 passed; focused frontend run 164 tests, 162 passed, 2 skipped, 0 failed. The CSS negative control independently reproduced F1. The 29-file remote delta contains no JSON or proof-byte modifications; local fresh data requests retain hash and byte-count verification.
Hosted browser evidence was retrieved and inspected, not rerun by this reviewer: browser-smoke job 111250761031, run 37139481247, checked merge commit 529b23b, PR head 7ae3505 and matching tree c4ef29bdd7d3dfa94226a825e127a6fbcc97e3ef. All 16 reported cases passed. Warm regression genuinely uses Chromium HTTP cache: no routing interception, immutable predecessor HTTP response, two visits showing 30 results and exactly one server hit for the bare entry module, then current versioned entry/transitive/style requests and 39 results in the same context. https://github.com/d6g8k5htny-coder/main/actions/runs/37139481247
This original-head AMEND remains historical evidence. A successor needs bounded review of fixes and applicable current-head checks before engineering ACCEPT; no successor acceptance is asserted here.
d6g8k5htny-coder
left a comment
d6g8k5htny-coder
left a comment
There was a problem hiding this comment.
Dylan Roy — delegated AI review. Actual performer: OpenAI / Codex agent /root/asset_release_review. Fresh nonauthor engineering review plus bounded successor revalidation; Dylan's personal reading PENDING; same-provider organizational-independence credit 0. No implementation edits by this reviewer. No claim of mathematical, human or organizational-independent acceptance.
Disposition: engineering source ACCEPT for head 655a22d, tree 159fd351cf444050aa326b119d2b65eec08a2ccb, base 0ae8bd8. This covers the entire final 31-file PR scope: the original 29-file source review and actual remote four-file successor delta, including two newly changed conditional fixtures. Original-head AMEND #246 (review) is retained as historical evidence, not rewritten.
F1 closed: the generator applies both CSS url() and quoted @import passes, with local-target validation shared by each. The permanent fifth generator test verifies quoted-import rewriting and missing-target refusal. My independent temporary reproduction now verifies token addition, clean --check, --check failure after imported CSS changes, a changed token after regeneration, byte-identical idempotence and missing-target refusal. The original reproduction failed before the fix. Generator --check passed with unchanged public release key dc3b8a3d50e06b183520769b998318b774231b7fc56eda438e4a4878b963d1a3; all 5 generator tests passed.
F2 closed: CLI checks now require release-qualified module entry URLs, and the startup fixture resolves actual museum.html script URLs. The page and conditional route consume the identical museum module instance. Single-request/cache-sharing assertions, rejection retry, stale-manifest refusal and source-substitution checks remain intact. Independently ran the two changed Node suites: 15 tests passed, no skips/failures. All four tested successor file blobs matched GitHub exact-head SHA identities. Remote comparison confirms no successor public presentation/config/source/proof byte changes; the complete final PR has no JSON or proof-byte changes.
Earlier verification reused only for unchanged source: focused frontend 164 tests / 162 pass / 2 skip; generator/browser-contract unittest 9 pass before successor; reader links 15 pass. No broad duplicate replay performed where bytes and consumed interfaces were unchanged. Generator checks establish the supported asset syntax/current site, not arbitrary JavaScript/CSS parsing completeness.
Current-head hosted evidence independently retrieved: public-shop run 37139647783 succeeded including browser-smoke. Browser report binds checked merge commit 4e9a60e, PR head 655a22d and matching tree 159fd351cf444050aa326b119d2b65eec08a2ccb; all 16 reported cases passed, including genuine HTTP-cache warm reuse. The controlled predecessor uses cacheable HTTP responses without Playwright route interception, checks one server hit over two predecessor visits, and then verifies release-qualified entry/transitive/style requests and 39 current results in the same context. https://github.com/d6g8k5htny-coder/main/actions/runs/37139647783
Current-head conditional-projection run 37139647781 succeeded: 107 tests / 105 pass / 2 skip / 0 fail, and the actual committed source replay step succeeded. https://github.com/d6g8k5htny-coder/main/actions/runs/37139647781
Exposure: dispatched after initial implementation, no prior implementation contribution; subsequent writer messages described proposed fixes, which I verified against actual remote bytes and independently tested. Hosted browser execution belongs to GitHub Actions, not this reviewer; I inspected code and fetched logs, without executing local browser automation. No remaining source-review blocker found. Integration owner must still refresh current head/base/ownership and require every applicable current-head check; this source ACCEPT does not override a pending/red gate or authorize a claim that all gates passed.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7ae3505288
ℹ️ 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".
| catalog = site.parent/'public-math' | ||
| if catalog.is_dir(): roots.append(catalog) | ||
| for root in roots: | ||
| for path in sorted(p for p in root.rglob('*') if p.is_file()): |
There was a problem hiding this comment.
When this generator is run on Windows, Path sorting is case-insensitive, whereas the Linux CI runner sorts paths case-sensitively. Because the hashed tree contains both uppercase names such as AGENTS.md and lowercase names such as app.js, identical bytes produce different release tokens, so a release generated on Windows will fail the new --check step in CI. Sort by an explicit repository-relative POSIX path key to make the advertised release digest reproducible across hosts.
Useful? React with 👍 / 👎.
| return digest.hexdigest() | ||
|
|
||
| def version(value, path, site, token): | ||
| clean = TOKEN.sub('', value) |
There was a problem hiding this comment.
If an asset gains a query parameter after the generated token, such as style.css?site-release=<old>&media=screen, this substitution leaves style.css&media=screen. urlsplit() then treats the entire value as a path whose suffix is not .css, so version() returns the original stale URL and even --check reports success after the asset changes. Remove site-release by parsing and rebuilding the query parameters so additional parameters cannot disable cache invalidation.
Useful? React with 👍 / 👎.
| catalog = site.parent/'public-math' | ||
| if catalog.is_dir(): roots.append(catalog) | ||
| for root in roots: | ||
| for path in sorted(p for p in root.rglob('*') if p.is_file()): |
There was a problem hiding this comment.
If a contributor has an untracked or ignored file anywhere under docs/site or docs/public-math—for example .DS_Store, an editor backup, or a __pycache__ artifact—this traversal includes it in the release digest even though it will not be committed or deployed. The documented generation command then writes a token derived from local-only state, and the clean Linux CI checkout computes another token and fails --check. Build the inventory from tracked/deployed files or explicitly exclude ignored and temporary artifacts.
Useful? React with 👍 / 👎.
| HTML = re.compile(r'(<(?:script|link)\b[^>]*?\b(?:src|href)\s*=\s*)([\"\'])([^\"\']+)(\2)', re.I) | ||
| IMPORT = re.compile(r'(\bfrom\s*|\bimport\s*\(\s*|\bimport\s*)([\"\'])([^\"\']+)(\2)') | ||
| CSS = re.compile(r'(url\(\s*)([\"\']?)([^\"\'\s)]+)(\2\s*\))') |
There was a problem hiding this comment.
Because these expressions scan raw source text rather than syntax, a JavaScript comment or user-facing string containing text such as import './plugin.js' or from './plugin.js' is treated as a real module specifier whenever that file exists, and the generator silently inserts the release query into the comment or displayed value. The analogous HTML and CSS expressions also process commented-out tags and url() examples. Use syntax-aware parsing, or at least lexical handling that excludes comments and string contents outside actual import specifiers and asset declarations.
Useful? React with 👍 / 👎.
|
Scoped visitor asset-refresh handoff / RELEASE — PR246 merged as 52252bc, tree159fd351cf444050aa326b119d2b65eec08a2ccb, identical to reviewed head655a22dc60b81eb6ff6aa44bf1d110a7ec86da2c. Full nonauthor engineering review5401819671 ACCEPT; original5401815629 AMEND and both repaired findings remain preserved. All eight landed checks succeeded, Pages deployment37139813500 succeeded, landed browser37139813997 binds the actual merge/tree: 16/16 cases,48 screenshots,0 page errors. Downloaded artifact11279349453 is7478799 bytes SHA256cbaa1e83971285e72bba2ec40f95bd18786cba417350732d7540dad12f9ae872. Local final verification207 Node tests (205 pass,2 optional fixture skips),74 Python tests pass; generator --check and public_shop_check succeeded. Browser tests include genuine HTTP cache reuse of the controlled predecessor and new entry/transitive/style release keys. Actual deployed stable URL https://d6g8k5htny-coder.github.io/main/site/dependencies.html was reloaded in the existing returning browser; search math now shows39 entries, with entry/style key dc3b8a3d50e06b183520769b998318b774231b7fc56eda438e4a4878b963d1a3. Dated research snapshot/source boundaries remain intact. Query keys choose fresh caches; they do not make historical hosting immutable. Future site changes run python -B tools/site_asset_release.py before commit; hosted --check rejects stale supported asset references. This releases only the bounded asset-refresh scope of pickup5971400668. Existing Library/equation redesign work remains separate and may use this merged base. No R17 mutation, scientific promotion, proof/source JSON rewrite or private-author metadata publication. Actual executor OpenAI/Codex /root; reviewer /root/asset_release_review, same-provider independence0, personal readingPENDING. |
|
Pickup: implement Dylan Roy's 2026-10-03 UI/UX review as a source-derived evidence/reading layer on the existing dependency viewer. Actual executor OpenAI/Codex /root, base52252bc8ab32df5f7d96620c9965a7e85343447c. Scope docs/site/dependency-model.mjs, dependencies.mjs/html/css, relevant tests/browser coverage, generated asset release keys, and a review-to-implementation roadmap. No Library inventory/equation redesign takeover, graph/status/proof byte changes, or new scientific register. Native isolated branch codex/evidence-reader-20261003. Bounded lease through2026-10-03T19:45:00Z; no R17 mutation. Current open PR read shows only unrelated228; previous246 scope remains released. Review facts are checked against frozen source, not copied as current acceptance. |
Returning visitors could receive cached predecessor scripts after a new Pages deployment. This gives local HTML assets, static/dynamic module imports and CSS images a reproducible site-release query key. Local config/data requests bypass cached responses; existing hash validation still refuses mismatches. Generator --check catches stale supported URLs without rewriting proof/source JSON bytes.
Final head 655a22d; tree 159fd351cf444050aa326b119d2b65eec08a2ccb; base 0ae8bd8. Full 31-file nonauthor engineering review 5401819671 ACCEPT closes F1 (quoted CSS import) and F2 (conditional fixture URLs). Original review 5401815629 AMEND and its failed conditional run remain recorded. Actual reviewer: OpenAI/Codex /root/asset_release_review; same-provider independence 0, personal reading PENDING.
Verification: 207 Node tests (205 pass, 2 optional fixture skips), 74 Python tests pass; generator --check clean; public_shop_check succeeded. All eight final-head hosted checks successful. Browser run 37139647783 artifact 11279279279 downloaded 7478491 bytes SHA256 68b62c17e227f9cd71e8d7f696278fd8d7efaee90eb906750b9fc8012709d916: checked merge 4e9a60e has the exact final tree; Chrome154, 16/16 cases, 48 screenshots, 0 page errors. Controlled predecessor uses actual Chromium HTTP cache without routing interception, confirms two visits/one module request, then requires new entry/transitive/style keys and 39 entries. This is controlled browser evidence; deployed readback remains a separate step.
Owner: OpenAI/Codex /root under Dylan Roy’s direct authorization; pickup #245 comment5971400668 through19:15UTC. No existing owner branch mutation or Library/equation content takeover. Scientific effect NONE. Release keys select fresh caches, not immutable historic hosting. Future site edits run python -B tools/site_asset_release.py before committing. Merged as 52252bc with identical tree 159fd351cf444050aa326b119d2b65eec08a2ccb. All eight landed checks and Pages deployment 37139813500 succeeded. Landed browser run 37139813997 artifact11279349453: 7478799 bytes SHA256 cbaa1e83971285e72bba2ec40f95bd18786cba417350732d7540dad12f9ae872, exact landed commit/tree, 16/16 cases, 48 screenshots, 0 page errors. The deployed stable dependencies.html URL was reloaded in the existing returning browser, showing all39 math results with the current release key in entry and stylesheet URLs. Bounded owner working scope RELEASED after this handoff; no R17 mutation.