Repository navigation
Conversation
12 tasks done
hamb3r
force-pushed
the
codex/refactor-agc-budget-snapshot
branch
from
November 1, 2026 00:38
6b4f382 to
7d9f2a1
Compare
|
Some comparisons could not be completed because a pull request conflicts with the base branch. Review these shared files manually before concluding that there is no overlap.
|
MarcZX10
previously requested changes
Nov 1, 2026
MarcZX10
left a comment
MarcZX10
left a comment
Contributor
There was a problem hiding this comment.
Reviewed — changes requested
The PR conflicts with both #6194 and #14220, and its description says the landing order with #6194 has not been agreed.
What I ran
Reviewed head 7d9f2a1f03367de6f92f9ecfba36f8b52b9a0970 against PR base 1ffd2b81f5960e51bef0d539891e2528eafff642 on Debian 12 x86_64, Python 3.11.2 and GCC 12.2.0. The preview onto current main c763ea56aae3bb60cd6e22064a6a21482a58a980 merged cleanly. Pairwise previews against #6194 and #14220 conflicted. I did not build locally.
python3 tools/pr_precheck.py --repo boykopovar/AnyPS5 --pr 14219
git diff --check 1ffd2b81f5960e51bef0d539891e2528eafff642...7d9f2a1f03367de6f92f9ecfba36f8b52b9a0970
git merge-tree --write-tree upstream/main refs/anyps5/review/14219
git merge-tree --write-tree --name-only refs/anyps5/review/14219 refs/anyps5/review/6194
git merge-tree --write-tree --name-only refs/anyps5/review/14219 refs/anyps5/review/14220
gh pr checks 14219 --repo boykopovar/AnyPS5Results
| Check | Result |
|---|---|
| Diff check | Passed. |
| Conventions and PR template | Passed locally and in hosted CI. |
| Current-main merge preview | Passed; tree 0ed87fc8de56eb065b2c723e5cc6ea6c5e994a4a. |
| Pairwise preview with #6194 | Conflicts in ShaderResources.hpp, ShaderResources.cpp, and docs/dev/debt/libSceAgcDriver.md. |
| Pairwise preview with #14220 | Conflicts in the same three files. |
| Hosted CI | Conventions, intake, labels, library-portability, report, eligibility and Windows passed; Linux and specialized jobs skipped. |
| GPU validation and focused tests | The PR reports paired 10/10 test runs on main and the change, plus a new snapshot test and mutation check, on Windows 10 with an RTX 3080. I did not reproduce these locally. |
Selected unedited output
0ed87fc8de56eb065b2c723e5cc6ea6c5e994a4a
CONFLICT (content): Merge conflict in core/libs/prx/libSceAgcDriver/Graphics/include/ShaderResources.hpp
CONFLICT (content): Merge conflict in core/libs/prx/libSceAgcDriver/Graphics/src/ShaderResources.cpp
CONFLICT (content): Merge conflict in docs/dev/debt/libSceAgcDriver.md
windows pass 27m48s https://github.com/boykopovar/AnyPS5/actions/runs/41257599023/job/124705363529
linux skipping 0 https://github.com/boykopovar/AnyPS5/actions/runs/41257599023/job/124705364702
Negative control
| Assertion | Baseline/change | Result |
|---|---|---|
| Budget output checks fail when their expected value is mutated by one byte, even if output still matches the pass regex | PR description reports a pass-only CTest fixture accepted the mutated executable before this change and the new FAIL: guard rejects it. |
Author-reported; not independently reproduced. |
Findings
Blocking
The PR names #6194 as an open change to the same storage-budget policy, and the pairwise preview confirms content conflicts with it. The preview against #14220 also conflicts because that PR changes the storage-cache policy and shared debt entry. The repository requires overlapping PR authors to agree which lands first and declare the order in Depends on; this PR says no order is agreed and its checklist currently declares no dependency. Please coordinate a landing sequence with the authors of #6194 and #14220, then update the relevant checklist lines before this stack proceeds.
Non-blocking
The current-main preview is clean. The hosted Windows job passed; no Linux product job ran. The PR describes its RTX 3080 validation-layer run and paired baseline tests in detail, but I did not independently reproduce them.
Sources
- Repository overlap rule, pinned to current main
- #14219 implementation and tests at reviewed head
- #6194 overlapping head
- #14220 overlapping head
Not verified
No local build, CTest, or GPU/validation-layer run. The author-reported RTX 3080 results and negative control were not independently reproduced; hosted Linux product tests were skipped.
To get this to a good state
Agree with the authors of #6194 and #14220 on the landing sequence and record it in the affected Depends on checklist lines. I will rerun the pairwise previews against the updated heads.
hamb3r
force-pushed
the
codex/refactor-agc-budget-snapshot
branch
from
November 1, 2026 01:34
7d9f2a1 to
f997c0e
Compare
Contributor
Author
|
@MarcZX10 updated #14219 to
|
MarcZX10
previously requested changes
Nov 1, 2026
MarcZX10
left a comment
MarcZX10
left a comment
Contributor
There was a problem hiding this comment.
Re-checked — new head, changes requested
The updated head now merges cleanly with #14220, but the proposed #14219-before-#6194 order is still unagreed and the exact heads still conflict.
What I ran
Reviewed #14219 head f997c0e0072b2c36e41355203e7d97d0ce3e7ada; its recorded PR base is 5f438847c5064961808425ce0465ed8675474dde, and current main is f485df1f8c660bbabc495dd34415f8d6e982ea24. The current-main preview is clean. Environment: Linux x86_64, Python 3.11.2, GCC 12.2.0.
python3 tools/pr_precheck.py --repo boykopovar/AnyPS5 --pr 14219
python3 /root/.codex/skills/anyps5-pr-review/scripts/check_pr_stack.py --order-only --dependency 14219=f997c0e0072b2c36e41355203e7d97d0ce3e7ada --dependent 6194=8bb130068023741ae244df30c74ddef5bc386644
python3 /root/.codex/skills/anyps5-pr-review/scripts/check_pr_stack.py --order-only --dependency 14219=f997c0e0072b2c36e41355203e7d97d0ce3e7ada --dependent 14220=55d3875c32c747a16eb63cd4b5776e21ae30c17aResults
| Check | Result |
|---|---|
| Diff and conventions | Passed |
| Current-main merge preview | Passed against f485df1f8c660bbabc495dd34415f8d6e982ea24 |
| Pairwise merge with #14220 | Clean |
| Pairwise merge with #6194 | Conflicts in ShaderResources.hpp, ShaderResources.cpp, and docs/dev/debt/libSceAgcDriver.md |
| Hosted CI | Windows, conventions, eligibility, report, and library-portability passed on this head; Linux and specialized jobs were skipped. A newer conventions run and labels check were queued |
| ABI/test-quality helpers | Not executable in the isolated PR checkout; the workspace-only scripts are absent there |
| Author-reported tests | The PR reports paired 10/10 tests plus a new snapshot test and mutation check; I did not independently rerun them |
| Local build/CTest | Not run |
Unedited command output
All conventions checks passed.
Shared comparison base: ab9a4768407d862d1879d7d0ee66bd9c3d9d67a8.
Dependency ancestry: NOT REQUIRED — PR #14219 must land before PR #6194.
Pairwise merge preview: CONFLICT — overlapping changes need manual review.
Unedited git merge-tree output:
380725b5b16bb0f830d52ba17bd6bf1f9c6f7224
100644 77058182fef944c49f64447279e0cdd361788a4e 1 core/libs/prx/libSceAgcDriver/Graphics/include/ShaderResources.hpp
100644 9ac165dd9db60f67a0eef7af205af0a005fd5606 2 core/libs/prx/libSceAgcDriver/Graphics/include/ShaderResources.hpp
100644 539f781cf22e7b19e4322ea3bad064779ad94439 3 core/libs/prx/libSceAgcDriver/Graphics/include/ShaderResources.hpp
100644 8e5b85e63456d9b39a1f8fb17a9b68e0c4f77fee 1 core/libs/prx/libSceAgcDriver/Graphics/src/ShaderResources.cpp
100644 6cdf1836be3c164b5d71ac9fdc8ef28637a1961b 2 core/libs/prx/libSceAgcDriver/Graphics/src/ShaderResources.cpp
100644 207a9db3d12777509f5eedb9583f8ae5105b1bc8 3 core/libs/prx/libSceAgcDriver/Graphics/src/ShaderResources.cpp
100644 e65bfc41645ff8a0df7fc3b7644d37fa3d874f09 1 docs/dev/debt/libSceAgcDriver.md
100644 b0a8e0d7b169947aeb71eed153cda22c2a6877a0 2 docs/dev/debt/libSceAgcDriver.md
100644 32431238454407551afdfce111ff976878e411b8 3 docs/dev/debt/libSceAgcDriver.md
Auto-merging core/libs/prx/libSceAgcDriver/Graphics/include/ShaderResources.hpp
CONFLICT (content): Merge conflict in core/libs/prx/libSceAgcDriver/Graphics/include/ShaderResources.hpp
Auto-merging core/libs/prx/libSceAgcDriver/Graphics/src/ShaderResources.cpp
CONFLICT (content): Merge conflict in core/libs/prx/libSceAgcDriver/Graphics/src/ShaderResources.cpp
Auto-merging core/libs/prx/libSceAgcDriver/tests/Graphics.cpp
Auto-merging docs/dev/debt/libSceAgcDriver.md
CONFLICT (content): Merge conflict in docs/dev/debt/libSceAgcDriver.md
Shared comparison base: 381891927c2b3ff494a9853cb79fde8d5342b92d.
Dependency ancestry: NOT REQUIRED — PR #14219 must land before PR #14220.
Pairwise merge preview: CLEAN — tree 14f74baeee195a2d32412b6ad723bcc14a2ddc38.
/usr/bin/python3: can't open file '/tmp/anyps5-pr-precheck/boykopovar-AnyPS5-pr-14219-f997c0e0072b/head-f997c0e0072b-b61fc81e/tools/pr_abi_audit.py': [Errno 2] No such file or directory
/usr/bin/python3: can't open file '/tmp/anyps5-pr-precheck/boykopovar-AnyPS5-pr-14219-f997c0e0072b/head-f997c0e0072b-b61fc81e/tools/pr_test_quality.py': [Errno 2] No such file or directory
Negative control
| Assertion | Change to code under test | Did the new test fail? |
|---|---|---|
| Snapshot test rejects a mutated reservation even when the pass regex matches | The PR description reports mutating the reservation assertion | Author-reported only; not independently reproduced |
Findings
Blocking
The proposed landing order is #14219 before #6194, but the exact current heads still conflict in three shared files and the author says agreement is pending. The repository overlap rule requires agreement on order and that it be declared with Depends on. Please confirm the order with #6194's author; then record the dependency on the PR that lands second and rebase/resolve the conflict.
Non-blocking
The current #14219 and #14220 heads merge cleanly together. The PR's summary and test-guard changes are otherwise reflected in the updated description; their runtime evidence remains author-reported.
Sources
Not verified
No local build, CTest, GPU run, or independent reproduction of the author's mutation check. The Windows and test results above are hosted or author-reported; Linux and specialized hosted jobs were skipped.
To get this to a good state
Confirm the landing sequence with #6194's author, update the dependent PR's Depends on line, and resolve the remaining exact-head conflict. I will rerun the same current-main and pairwise checks on the updated heads.
hamb3r
force-pushed
the
codex/refactor-agc-budget-snapshot
branch
from
November 1, 2026 01:57
f997c0e to
520f1ec
Compare
11 tasks done
MarcZX10
previously requested changes
Nov 1, 2026
MarcZX10
left a comment
MarcZX10
left a comment
Contributor
There was a problem hiding this comment.
Re-checked — new head, changes still requested
Head 520f1ecd916f1c0f897d91bdcd7832c4e3b08bab now merges cleanly with #14220, but the landing order with #6194 remains unagreed and their exact heads still conflict.
What I ran
Reviewed head 520f1ecd916f1c0f897d91bdcd7832c4e3b08bab; GitHub records base 41f9c911dadca8aa735ad5063d8630712a9985dc, while current main is 0e9dc7bc4c9755c3ce803f6bbdff9dc9f25d68bb. Environment: Debian 12 x86-64, Python 3.11.2, GCC 12.2.0. The precheck reported a clean preview against current main.
python3 tools/pr_precheck.py --repo boykopovar/AnyPS5 --pr 14219
python3 /root/.codex/skills/anyps5-pr-review/scripts/check_pr_stack.py --order-only --dependency 14219=520f1ecd916f1c0f897d91bdcd7832c4e3b08bab --dependent 6194=8bb130068023741ae244df30c74ddef5bc386644
python3 /root/.codex/skills/anyps5-pr-review/scripts/check_pr_stack.py --order-only --dependency 14219=520f1ecd916f1c0f897d91bdcd7832c4e3b08bab --dependent 14220=55d3875c32c747a16eb63cd4b5776e21ae30c17aResults
| Check | Result |
|---|---|
| Diff and conventions | Passed in the no-build precheck. |
Merge preview against current main |
Clean. |
| Pairwise preview with #6194 | Conflicts in ShaderResources.hpp, ShaderResources.cpp, and docs/dev/debt/libSceAgcDriver.md. |
| Pairwise preview with #14220 | Clean on the current exact heads. |
| Hosted checks | Windows pending; conventions, labels, portability, report, and eligibility passed. Linux and specialized jobs skipped. |
| Build-dependent tests | Not run locally; the PR reports 11/11 focused tests on Windows. |
| Optional ABI/test-quality helpers | The precheck could not run the workspace-only helper scripts in its isolated PR checkout. The new agc_memory_accounting_budget_snapshot test is registered in the changed CMake fragment. |
Selected unedited output
All conventions checks passed.
PR #14219: clean against current base; head `520f1ecd916f1c0f897d91bdcd7832c4e3b08bab`
Pairwise merge preview: CONFLICT — overlapping changes need manual review.
CONFLICT (content): Merge conflict in core/libs/prx/libSceAgcDriver/Graphics/include/ShaderResources.hpp
CONFLICT (content): Merge conflict in core/libs/prx/libSceAgcDriver/Graphics/src/ShaderResources.cpp
CONFLICT (content): Merge conflict in docs/dev/debt/libSceAgcDriver.md
Pairwise merge preview: CLEAN — tree 324b08e20f0e0cb0e9fcd5c14231c12935ecc301.
Findings
Blocking
The exact #14219 and #6194 heads still conflict in three shared files, and the latest discussion on #6194 says the proposed #14219-before-#6194 order is awaiting the other author's confirmation. This PR still lists Depends on: none. Please confirm the landing order with the #6194 author and record it on the PR that lands second; then refresh the affected branch and I will rerun the pairwise preview.
Non-blocking
The latest head fixes the CTest registration placement, and the current pairwise preview with #14220 is clean. The Windows focused-test result is author-reported; it is not a local run.
Sources
- Repository overlap and landing-order rule at current
main - #14219 exact head
- #6194 exact head
- #14220 exact head
Not verified
No local build, CTest, GPU validation run, or independent reproduction of the reported mutation check. Hosted Windows CI is still pending; Linux and specialized product jobs were skipped.
To get this to a good state
Confirm the sequence with the #6194 author. If #14219 lands first, declare that order in #6194's Depends on line; refresh the second branch and resolve the remaining conflicts. I will rerun the same pairwise checks on the updated heads.
12 tasks done
Contributor
✅ NVIDIA GPU test: passedPR head
Intermittent validation messages (1, seen once in the parallel run, not reproduced when the test was re-run alone 2 times; not counted against this PR)Unstable on main (2, these also failed in recent main runs on this machine; not counted against this PR)Failing on main too (3, not counted against this PR)Validation IDs already present on main (5 IDs in 480 tests)Skipped (44)ReproduceAutomated build and test on a contributor's NVIDIA machine; it does not replace upstream CI. A new comment is posted for each new push. Full logs. |
12 tasks done
hamb3r
force-pushed
the
codex/refactor-agc-budget-snapshot
branch
from
November 1, 2026 23:15
520f1ec to
e0ace57
Compare
Contributor
✅ NVIDIA GPU test: passedPR head
Unstable on main (4, these also failed in recent main runs on this machine; not counted against this PR)Failing on main too (3, not counted against this PR)Validated but not compared (1, not in the main baseline's validation pass)Validation IDs already present on main (5 IDs in 484 tests)Skipped (45)ReproduceAutomated build and test on a contributor's NVIDIA machine; it does not replace upstream CI. A new comment is posted for each new push. Full logs. |
Collaborator
|
Hi On landing order: #14891, the dependency this branch declares, merged on 3 Nov, and |
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Follow up #10194 by calculating the storage-cache decision and its diagnostic fields together.
CalculateStorageTextureBudgetreturns the chosen heap, sampled/cached inputs, reservation, available bytes, ceiling andselectedBudget. The existing scalar functions consume that calculation, andMemoryAccounting::Reportformats its snapshot. Budget formulas, fallback behavior, cache policy and report keys remain unchanged.The final snapshot, including any re-read after idle reclaim, is captured under the budget-cache mutex. The active limit and its optional calculation timestamp are copied before releasing that mutex. Ledger snapshotting and report output then run outside it.
CTest ignores the process exit code when
PASS_REGULAR_EXPRESSIONis set, so a failedExpectcan still pass when the report line matches (CTest documentation). Existing cap/floor tests rejectFAIL:explicitly; the snapshot test checks pressure, unavailable/zero Vulkan budget information and the 512 MiB floor, with its own failure guard.Depends on #14891: it adds the age annotation to the sampled-cache report while retaining the scalar storage-budget API. This branch is rebased on its exact head, preserving the timestamp copy inside the cache mutex. #14220 at
d2cad1510164merges cleanly. #6194 still conflicts with main itself and must preserve its final adjusted budget when integrated; coordination remains in the existing discussion.Tested
Windows 10 Pro 10.0.19045, AMD Ryzen 7 7700, NVIDIA GeForce RTX 3080, NVIDIA 617.14, validation layer 1.4.350.0, WinLibs GCC 15.2.0, Release, BUILD_TESTING=ON, CCACHE_DISABLE=1.
f03f72490132ed55d81c664b570d0bd26bc96694.55940bd5c49d44adf0bf1ede28901bed627ee403.e0ace57ec2263c62a3d8e9adbde6d68ec7c795a6.agc_driver_sampled_cache_budget_tests,agc_driver_graphics_testsandagc_buffer_pool_testsfor all three states.Same machine/GPU and environment for every run:
ctest --test-dir build-portable -R '^(agc_driver_sampled_cache_budget_(memory|report_age)|agc_driver_recover_(sampled|storage)_allocation|agc_driver_graphics|agc_buffer_pool|agc_memory_accounting_budget_(cap|floor|snapshot))$' -VMain passes the same 7/7 existing cases, the dependency passes 8/8 including report age, and this PR passes 9/9 including the snapshot case. Validation-layer insertion is confirmed in all comparison logs; none has a missing-layer warning or VUID validation errors. No cases skipped.
Mutation controls: deliberately incorrect cap and snapshot assertions are both rejected with CTest exit 8 even though their numeric reports still match. Restoring the assertions passes both cases.
Conventions and
git diff --checkpass. Full builds/test suites, Linux/macOS, games and FPS were not tested.Checklist
main; no other open PR implements the same functionsNotImplemented_nid_no_patch); silent stubs are listed under Silent stubs in their library's technical debt file.mdfiles or images added to the repository (attach them to this PR)