WIP: fix(profiling): reject incomplete or unusable chain captures #682

Draft

NitnekLeBorgne wants to merge 1 commit from NitnekLeBorgne/Longshot-contributions:fix/chain-capture-input-20261115 into main

pull from: NitnekLeBorgne/Longshot-contributions:fix/chain-capture-input-20261115

merge into: MagicBeanDev:main

MagicBeanDev:main

Conversation 1 Commits 1 Files changed 2 +113 -5

NitnekLeBorgne commented 2026-11-15 04:08:20 +00:00

First-time contributor

• Copy link
• Copy Markdown

Type: fix · Risk: low · Tested on: Linux, Python 3.12.3, game-free synthetic captures · Needs: full game-free suite; Windows Python check

Why

An incomplete HOS_CHAIN recording can currently be accepted as if its declared records were complete. A recording with only one present instead prints partial statistics and then raises ZeroDivisionError. Both make the profiling tool harder to trust when a recording is too short or incomplete.

Open work and issues were searched. Nearby HOS_CHAIN users and the broad file-security change were checked by their actual changed-file lists; this change does not alter the runtime recorder or those changes.

How to reproduce

Seen on: Linux, Python 3.12.3; no GPU or game required.
Settings and switches: none.
Steps: Write a synthetic HOSCHAIN1 file declaring two 32-byte records, but truncate the second record; call scripts/chain.py's load(). Separately run main() on a valid recording containing just one PRESENT. The new tests construct these inputs without game data.
Expected: a clear diagnostic before producing a frame analysis.
Actual: main at fad1a964cd silently accepts the shortened declared payload; the one-PRESENT case reaches a division by zero.

What changed

Validate the declared payload length and diagnose incomplete or malformed known header fields. Refuse analysis when the selected presenting thread has fewer than two presents.

• scripts/chain.py: input validity guards in load() and main(); no changes to build(), walk(), Acc or the sample attribution path.
• tests/test_chain_input.py: ten standard-library regression/control methods. The existing tests/test_chain.py and runtime producer remain unchanged.
• Preserve existing unknown-header and trailing-suffix behavior. Do not impose new timestamp/tag restrictions on complete records: the recorder explicitly allows a record still being filled when capture stops.

Measurements

Verdict: the demonstrated parser/short-capture defects fail on the named main baseline and pass on the reviewed candidate. This is a Python tool with synthetic tests, not a performance change; no FPS or hardware performance claim applies.

Flags and settings

None

Testing

Tested:

• Native fresh replay with /usr/bin/python3 -I -B, Python 3.12.3, private HOME/TMP/XDG configured before imports.
• Ten new methods: five already-green control methods on baseline and five affected methods (three assertion-failure events and ten error events, including subtests). Candidate: all ten methods pass, no failures/errors.
• Existing standalone chain-path check passes on baseline and candidate; it is not counted as an eleventh unittest method.
• Seventy-one source-read module checks pass on each side with jobs=1 on complete public-source exports, excluding the protected instructions file. These are a selected partition, NOT the full game-free suite; repetitions are not extra coverage.
• Original author RED/GREEN receipts, a length-check mutant that fails its regression, and complete valid two/three-PRESENT stdout equality retained privately. Independent SPEC and QUALITY reviews passed on the exact patch/source bytes. The fresh replay is separate from historical receipts whose interpreter token was masked; no historical command was reconstructed.

Not tested: historical local export/containment limits and missing OS namespace isolation were recorded; these are not proof that a supported full-runner qualification is impossible. The full tests/run_all.py suite, Windows, real captures, or optional samples/prof integration. Host and renderer builds were not run; no host or renderer code changes.

Must be tested:

• Linux: run the complete game-free suite on a supported checkout/toolchain before merge; the selected source-read partition is not a full suite result, expecting the full suite to pass or each shared main failure to be named (by hand: an equipped contributor/tester inspects diagnostics and the stated outcomes before merge; not executed by this documentation edit).
• Windows Python: run tests/test_chain_input.py and tests/test_chain.py, expecting the new diagnostics, ten passing new methods, and unchanged valid-path check (by hand, no game needed). (by hand: an equipped contributor/tester inspects diagnostics and the stated outcomes before merge; not executed by this documentation edit).
• Linux / branch provenance: assess the then-current main and revalidate before ready; the reviewed base remains fad1a964cd, with source/mode identity and unoccupied new test path checked historically but no rebase claimed, expecting current-main provenance and the relevant checks to be revalidated before ready; no rebase or force-push is authorized (by hand: an equipped contributor/tester inspects diagnostics and the stated outcomes before merge; not executed by this documentation edit).

Risks

What could break: callers relying on silently incomplete recordings now receive SystemExit. Unknown headers and extra bytes after the declared record boundary retain their previous behavior. This is not general hardening against arbitrarily huge or hostile files.
Turning it off: revert this commit.
Compatibility: no saves, options, caches, Vulkan ABI, runtime recorder layout or Builder behavior changes. Public code and self-authored synthetic fixtures only.

Checklist

Everyone

• One topic.
• The title is a Conventional Commit (type(scope): description) of the status line's type.
• Why gives the reason plainly and links the issues that fit (the tracker searched).
• Measurements has a verdict, or one line saying why there is nothing to measure.
• Testing names what was tested, what wasn't, and what must still be tested, where and how.
• Rebased on current main.
• python -I -B tests/run_all.py passes (any failure that also fails on main is named).
• No game files, dumps, build output or generated files.
• third_party/ untouched (a new folder with a PROVENANCE.md instead).
• The license is OK: GPL-3.0-or-later, nothing incompatible and nothing decompiled.

When it applies

• AI assistance recorded with Assisted-by: / Co-Authored-By: trailers.
• A game-free test added for the fix, failing on main and passing here.
• A check under tests/ that reads changed source updated in this PR, not weakened.

Readiness provenance and checklist notes

Current main in the frozen public readback is aa95d35f3d021b27328b6d68109666a46c340aee; this PR head is 1a2f4e2372e801610c8ffd744fe8c39d93e097db. The current-main box is unchecked: historical publication/base checks are not proof of current ancestry. Current-main updating and revalidation are needed before ready, but rebase/force-push/merge are not authorized; no such operation is promised or performed. The full runner box remains unchecked; no full-suite run or Windows/host qualification is claimed by this description edit. The normal project pr ready remains blocked and must not be forced. Manual pre-merge plans are future work, not completed evidence.

Historical checklist explanations (not current-main qualification):

• Rebased on current main. Reviewed base is pinned above; upstream advanced. Kept WIP, no rebase claimed.
• python -I -B tests/run_all.py passes (any failure that also fails on main is named). Only the explicit source-read partition and targeted checks ran; full suite remains required.
• A check under tests/ that reads changed source updated in this PR, not weakened. New tests added; the existing path check remains byte-identical.

**Type:** fix · **Risk:** low · **Tested on:** Linux, Python 3.12.3, game-free synthetic captures · **Needs:** full game-free suite; Windows Python check ## Why An incomplete HOS_CHAIN recording can currently be accepted as if its declared records were complete. A recording with only one present instead prints partial statistics and then raises ZeroDivisionError. Both make the profiling tool harder to trust when a recording is too short or incomplete. Open work and issues were searched. Nearby HOS_CHAIN users and the broad file-security change were checked by their actual changed-file lists; this change does not alter the runtime recorder or those changes. ## How to reproduce **Seen on:** Linux, Python 3.12.3; no GPU or game required. **Settings and switches:** none. **Steps:** Write a synthetic HOSCHAIN1 file declaring two 32-byte records, but truncate the second record; call scripts/chain.py's load(). Separately run main() on a valid recording containing just one PRESENT. The new tests construct these inputs without game data. **Expected:** a clear diagnostic before producing a frame analysis. **Actual:** main at fad1a964cdd17772cd44e3a8972b667f01873d36 silently accepts the shortened declared payload; the one-PRESENT case reaches a division by zero. ## What changed Validate the declared payload length and diagnose incomplete or malformed known header fields. Refuse analysis when the selected presenting thread has fewer than two presents. - scripts/chain.py: input validity guards in load() and main(); no changes to build(), walk(), Acc or the sample attribution path. - tests/test_chain_input.py: ten standard-library regression/control methods. The existing tests/test_chain.py and runtime producer remain unchanged. - Preserve existing unknown-header and trailing-suffix behavior. Do not impose new timestamp/tag restrictions on complete records: the recorder explicitly allows a record still being filled when capture stops. ## Measurements **Verdict:** the demonstrated parser/short-capture defects fail on the named main baseline and pass on the reviewed candidate. This is a Python tool with synthetic tests, not a performance change; no FPS or hardware performance claim applies. ### Flags and settings None ## Testing **Tested:** - Native fresh replay with /usr/bin/python3 -I -B, Python 3.12.3, private HOME/TMP/XDG configured before imports. - Ten new methods: five already-green control methods on baseline and five affected methods (three assertion-failure events and ten error events, including subtests). Candidate: all ten methods pass, no failures/errors. - Existing standalone chain-path check passes on baseline and candidate; it is not counted as an eleventh unittest method. - Seventy-one source-read module checks pass on each side with jobs=1 on complete public-source exports, excluding the protected instructions file. These are a selected partition, NOT the full game-free suite; repetitions are not extra coverage. - Original author RED/GREEN receipts, a length-check mutant that fails its regression, and complete valid two/three-PRESENT stdout equality retained privately. Independent SPEC and QUALITY reviews passed on the exact patch/source bytes. The fresh replay is separate from historical receipts whose interpreter token was masked; no historical command was reconstructed. **Not tested:** historical local export/containment limits and missing OS namespace isolation were recorded; these are not proof that a supported full-runner qualification is impossible. The full tests/run_all.py suite, Windows, real captures, or optional samples/prof integration. Host and renderer builds were not run; no host or renderer code changes. **Must be tested:** - Linux: run the complete game-free suite on a supported checkout/toolchain before merge; the selected source-read partition is not a full suite result, expecting the full suite to pass or each shared main failure to be named (by hand: an equipped contributor/tester inspects diagnostics and the stated outcomes before merge; not executed by this documentation edit). - Windows Python: run tests/test_chain_input.py and tests/test_chain.py, expecting the new diagnostics, ten passing new methods, and unchanged valid-path check (by hand, no game needed). (by hand: an equipped contributor/tester inspects diagnostics and the stated outcomes before merge; not executed by this documentation edit). - Linux / branch provenance: assess the then-current main and revalidate before ready; the reviewed base remains fad1a964cdd17772cd44e3a8972b667f01873d36, with source/mode identity and unoccupied new test path checked historically but no rebase claimed, expecting current-main provenance and the relevant checks to be revalidated before ready; no rebase or force-push is authorized (by hand: an equipped contributor/tester inspects diagnostics and the stated outcomes before merge; not executed by this documentation edit). ## Risks **What could break:** callers relying on silently incomplete recordings now receive SystemExit. Unknown headers and extra bytes after the declared record boundary retain their previous behavior. This is not general hardening against arbitrarily huge or hostile files. **Turning it off:** revert this commit. **Compatibility:** no saves, options, caches, Vulkan ABI, runtime recorder layout or Builder behavior changes. Public code and self-authored synthetic fixtures only. ## Checklist ### Everyone - [x] One topic. - [x] The title is a Conventional Commit (`type(scope): description`) of the status line's type. - [x] Why gives the reason plainly and links the issues that fit (the tracker searched). - [x] Measurements has a verdict, or one line saying why there is nothing to measure. - [x] Testing names what was tested, what wasn't, and what must still be tested, where and how. - [ ] Rebased on current `main`. - [ ] `python -I -B tests/run_all.py` passes (any failure that also fails on `main` is named). - [x] No game files, dumps, build output or generated files. - [x] `third_party/` untouched (a new folder with a `PROVENANCE.md` instead). - [x] The license is OK: GPL-3.0-or-later, nothing incompatible and nothing decompiled. ### When it applies - [x] AI assistance recorded with `Assisted-by:` / `Co-Authored-By:` trailers. - [x] A game-free test added for the fix, failing on `main` and passing here. - [x] A check under `tests/` that reads changed source updated in this PR, not weakened. ## Readiness provenance and checklist notes Current main in the frozen public readback is `aa95d35f3d021b27328b6d68109666a46c340aee`; this PR head is `1a2f4e2372e801610c8ffd744fe8c39d93e097db`. The current-main box is unchecked: historical publication/base checks are not proof of current ancestry. Current-main updating and revalidation are needed before ready, but rebase/force-push/merge are not authorized; no such operation is promised or performed. The full runner box remains unchecked; no full-suite run or Windows/host qualification is claimed by this description edit. The normal project pr ready remains blocked and must not be forced. Manual pre-merge plans are future work, not completed evidence. Historical checklist explanations (not current-main qualification): - Rebased on current `main`. Reviewed base is pinned above; upstream advanced. Kept WIP, no rebase claimed. - `python -I -B tests/run_all.py` passes (any failure that also fails on `main` is named). Only the explicit source-read partition and targeted checks ran; full suite remains required. - A check under `tests/` that reads changed source updated in this PR, not weakened. New tests added; the existing path check remains byte-identical.

NitnekLeBorgne added 1 commit 2026-11-15 04:08:20 +00:00

fix(profiling): reject incomplete or unusable chain captures ... 1a2f4e2372

Check declared record payload before analysing HOSCHAIN1 input.
Diagnose incomplete headers and captures without a complete frame.
Preserve valid path analysis and existing recorder compatibility.

Assisted-by: Hermes Agent (OpenAI GPT-6.1-sol)
Co-Authored-By: Claudia (OpenAI GPT-6.1-sol)

SocialWare referenced this pull request 2026-11-15 13:27:34 +00:00

Game: Crash in Hyrule field after opening and closing map #809

mooki added the

kind

bug

platform/linux

platform/windows

priority

high

labels 2026-11-15 13:58:07 +00:00

HumanBean force-pushed fix/chain-capture-input-20261115 from 1a2f4e2372 to 898fbcb634 2026-11-15 18:42:48 +00:00 Compare

HumanBean commented 2026-11-15 18:45:31 +00:00

Owner

• Copy link
• Copy Markdown

Rebased onto main @ bbe6fdf: no conflicts. No tests were run here; the test boxes stay yours.

Rebased onto `main` @ bbe6fdf: no conflicts. No tests were run here; the test boxes stay yours.

Some checks are pending

Hide checks

PR description / Description follows the template (pull_request_target) Waiting to run

Details

CI / Conventional Commit title (pull_request) Waiting to run

Details

CI / Python source checks (ubuntu-24.04, part 1/2) (pull_request) Has been skipped

Details

CI / Python source checks (ubuntu-24.04, part 2/2) (pull_request) Has been skipped

Details

CI / SPIR-V fixtures (global narrow load/store, direct surface atomics, F2I immediate) (pull_request) Has been skipped

Details

CI / Workflow lint (actionlint) (pull_request) Waiting to run

Details

Guide screenshots / Guide images and local UI source review (pull_request) Waiting to run

Details

Third-party parts / As their notes say (pull_request) Waiting to run

Details

Third-party parts / The same as upstream (pull_request) Has been skipped

Details

Website / Website on Cloudflare (pull_request) Has been skipped

Details

This pull request is marked as a work in progress.

This branch is out-of-date with the base branch

View command line instructions

Checkout

From your project repository, check out a new branch and test the changes.

git fetch -u 'fix/chain-capture-input-20261115:NitnekLeBorgne-fix/chain-capture-input-20261115'

git switch 'NitnekLeBorgne-fix/chain-capture-input-20261115'

Sign in to join this conversation.

Reviewers

No reviewers

Labels

Clear labels   

area/accessibility

Barriers for people with disabilities

  

area/audio

Sound and music

  

area/input

Controllers, keyboard, mouse

  

area/install

Installing, updating, building from your game

  

area/menu

In-game menu and settings

  

area/renderer

Vulkan renderer, shaders, effects

  

area/saves

Save files

  

area/upscaling

DLSS, FSR, frame generation

  

beanstalk: approved @ 1697603

Set by Beanstalk for show; the farm's database is what counts

  

beanstalk: approved @ 2c5a62e

Set by Beanstalk for show; the farm's database is what counts

  

beanstalk: approved @ 4fd82b7

Set by Beanstalk for show; the farm's database is what counts

  

beanstalk: approved @ 64d6d09

Set by Beanstalk for show; the farm's database is what counts

  

beanstalk: approved @ 6f91fae

Set by Beanstalk for show; the farm's database is what counts

  

beanstalk: approved @ 70a78f9

Set by Beanstalk for show; the farm's database is what counts

  

beanstalk: approved @ 8d361dc

Set by Beanstalk for show; the farm's database is what counts

  

beanstalk: approved @ 9d8356e

Set by Beanstalk for show; the farm's database is what counts

  

beanstalk: approved @ abf2227

Set by Beanstalk for show; the farm's database is what counts

  

beanstalk: approved @ d82f202

Set by Beanstalk for show; the farm's database is what counts

  

beanstalk: approved @ e5f1e40

Set by Beanstalk for show; the farm's database is what counts

  

beanstalk: approved @ ee8ab5f

Set by Beanstalk for show; the farm's database is what counts

  

ci: approved

A maintainer approved CircleCI for this fork PR (sticky; removed after 14 quiet days)

  

good first issue

A good start for a new contributor

  

gpu/amd

AMD GPUs

  

gpu/igpu

Integrated GPUs

  

gpu/intel

Intel GPUs

  

gpu/nvidia

NVIDIA GPUs

  

help wanted

Extra hands or knowledge wanted

  

kind

bug

Something doesn't work as it should

  

kind

chore

Build, CI, dependencies, tidying

  

kind

docs

Documentation

  

kind

feature

A new feature or an improvement

  

kind

performance

Frame rate, frame time, loading, memory

  

kind

question

A question rather than a change

  

needs measurements

Waiting for before/after measurements

  

platform/linux

Affects Linux (a change for every system carries both Windows and Linux)

  

platform/macos

macOS (Apple silicon or Intel Macs)

  

platform/steam-deck

Steam Deck

  

platform/windows

Affects Windows (a change for every system carries both Windows and Linux)

  

preview

Maintainers: deploy a preview of this PR's website (read its changes first). Removed after the run.

  

priority

critical

Crashes, data loss, or blocks most players

  

priority

high

Important: next in line

  

priority

low

Nice to have

  

regression

Worked in an earlier build, broken in a newer one: highest priority, above other bugs

  

status

blocked

Waits on something else first

  

status

confirmed

Reproduced or agreed on

  

status

duplicate

Already reported or proposed

  

status

invalid

Not a problem in this project

  

status

needs-info

Waiting for more information from the reporter

  

status

wontfix

Will not be worked on

  

testing wanted

Wanted: a test on other hardware (posts a test request in #test-requests)

  

ci: approved

No labels

area/accessibility

area/audio

area/input

area/install

area/menu

area/renderer

area/saves

area/upscaling

beanstalk: approved @ 1697603

beanstalk: approved @ 2c5a62e

beanstalk: approved @ 4fd82b7

beanstalk: approved @ 64d6d09

beanstalk: approved @ 6f91fae

beanstalk: approved @ 70a78f9

beanstalk: approved @ 8d361dc

beanstalk: approved @ 9d8356e

beanstalk: approved @ abf2227

beanstalk: approved @ d82f202

beanstalk: approved @ e5f1e40

beanstalk: approved @ ee8ab5f

ci: approved

good first issue

gpu/amd

gpu/igpu

gpu/intel

gpu/nvidia

help wanted

kind

bug

kind

chore

kind

docs

kind

feature

kind

performance

kind

question

needs measurements

platform/linux

platform/macos

platform/steam-deck

platform/windows

preview

priority

critical

priority

high

priority

low

regression

status

blocked

status

confirmed

status

duplicate

status

invalid

status

needs-info

status

wontfix

testing wanted

ci: approved

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

2 participants

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference

MagicBeanDev/Longshot!682

No description provided.