Repository navigation
fix(repo): arm the conflict-marker gate and untrack the file it missed (#17297) - #17407
Merged
Merged
Conversation
#17297) docs/.obsidian/workspace.json has carried a committed conflict block on main since 2026-04-03 (24d5193). The file is not valid JSON, so anything that parses it fails. The interesting half is why the gate did not catch it. .pre-commit-config.yaml has run check-merge-conflict since 2025-08 -- before the conflict landed -- and reported "Passed" on every commit since. Its first statement is: if not is_in_merge() and not args.assume_in_merge: return 0 is_in_merge() requires MERGE_MSG AND one of MERGE_HEAD / rebase-apply / rebase-merge. The markers read "Updated upstream" / "Stashed changes" -- a git stash pop conflict, which creates none of those. So the hook exited before reading a single line of the file it was pointed at. It was not misconfigured and did not miss a pattern: it was structurally blind to this entire conflict class, and it answered "clean" while the markers sat in the tree. Armed with --assume-in-merge. The stash detail is not incidental. The stash stack is shared across every worktree in this repo, which is exactly why a blind pop is hazardous here -- and the conflict class the hook cannot see is the one that hazard produces. AC2 needed no decision: .gitignore:509 ALREADY lists docs/.obsidian/workspace.json. The repo decided this file should not be tracked and recorded it in the one place that has no effect on an already-tracked file. `git rm --cached` completes a decision already made rather than making a new one. docs/.obsidian/graph.json is kept -- graph settings are shareable; workspace.json is per-user pane and cursor state and conflicts by nature. AC4 sweep, stated as a result rather than as silence: ONE tracked file on origin/main (6f5d119) carried markers, the known one, all three of them. Enabling the gate breaks no other tracked file. New repo_tests guard, which is not redundant with the hook: a pre-commit hook is skippable with --no-verify and only sees the files a commit touches; this runs in CI over all 10810 tracked files, so a marker arriving by a bypassed hook or a wrongly-resolved merge is still caught. It also pins the --assume-in-merge flag, because a disarmed hook still prints "Passed" and nothing in the output distinguishes it from a clean tree. The guard uses check-merge-conflict's own patterns, including the bare "=======". This issue's own acceptance criteria proposed "^======= " with a trailing space, which never matches -- git writes the separator alone on its line. A gate built to that spelling would have found the outer two markers and silently missed the middle one. The reach floor was first written as 2000 against a live population of 10810 and verify_floor refused it. Recorded at the declaration: that refusal is the mechanism working, not an obstacle to it. Closes #17297
Contributor
|
Warning Review limit reachedNext included review available in 6 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Repository: mrveiss/AutoBot-AI/.coderabbit.yaml Review profile: ASSERTIVE Plan: Advanced Run ID: 📒 Files selected for processing (4)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
✅ SSOT Configuration Compliance: Passing🎉 No new hardcoded values of either class — Known backlog in |
Owner
Author
Review at
|
| claim | measurement |
|---|---|
the marker exists on main and this PR removes the only one |
git grep -n "^<<<<<<< " on origin/main → exactly one hit, docs/.obsidian/workspace.json:182, the file this PR untracks |
untracking makes .gitignore effective, so it cannot return |
git check-ignore -v --no-index → .gitignore:509. The rule was already there and inert, because .gitignore does not apply to a tracked path |
| the deleted file is not code | Obsidian editor workspace state, 240 lines of pane geometry. Untracking is the fix, not a deletion of work |
| the separator predicate has no false positive on the tree today | git grep -x "=\{7\}" → 0 files. A setext heading underlined with exactly seven = would match line.rstrip() == _SEPARATOR; none exists |
| the sweep cannot pass by not looking | SCANNED_FILES floor 10000 / growth 1000 against a 10810 population — 190 files of headroom under the ceiling |
Constructing _OUTER_PREFIXES as "<" * 7 + " " rather than writing the literal, so the guard does not flag its own source, is the right call and worth keeping.
The one fix: test_the_detector_recognises_each_marker_shape cannot fail
Its docstring says the assertion goes through the real predicate:
Asserted through
_conflict_sitesown predicate rather than re-implemented here: a contrast written inline would pass against a detector that cannot see the bare=======
The assertion is:
line = marker if marker == _SEPARATOR else f"{marker}some-branch"
assert line.startswith(_OUTER_PREFIXES) or line.rstrip() == _SEPARATORThat is the re-implementation the docstring disclaims. It never calls _conflict_sites, and tmp_path is accepted and unused. Both sides of the assertion are the module's own constants compared against a copy of the module's own condition, so drop or line.rstrip() == _SEPARATOR from _conflict_sites and all three parametrised cases still pass — including the separator case the docstring names as the easy miss.
The fix is what the unused tmp_path was already reaching for: write a file containing the marker into tmp_path, git init it, and assert _conflict_sites(tmp_path) returns the site. That routes the assertion through the predicate under test, and the separator case then genuinely pins the clause it is named for.
Same shape as the two controls repaired in #17388 today: a positive control that asserts against local literals reads as a live guard from its name and its position, and is satisfied without the code under test participating. Not blocking the marker fix, which is real and verified — but this test is the only thing pinning the detector, and it pins nothing.
Review found that test_the_detector_recognises_each_marker_shape could not fail. Verified by mutation rather than by reading: dropping `or line.rstrip() == _SEPARATOR` from _conflict_sites -- so the detector can no longer see a bare `=======` at all -- gave 6 passed. All three parametrised cases survived, including the separator, which the docstring names as the easy miss. The docstring claimed it asserted "through `_conflict_sites`' own predicate rather than re-implemented here", and then re-implemented it inline. It never called _conflict_sites. It accepted tmp_path and never used it, which was the fix reaching for itself. A test that re-states a predicate agrees with itself by construction, and no care taken in writing the restatement changes that. The only way to pin a detector is to run it over something it must detect. It now git-inits a throwaway repo under tmp_path, writes a file carrying the marker, stages it so `git ls-files` reports it, and asserts _conflict_sites returns that exact site. #15246: the git calls scrub GIT_DIR, or they would address the real repository instead of the temporary one. Mutation-checked, each caught by its own case: drop the bare `=======` predicate -> 1 failed [=======] drop the outer-prefix predicate -> 2 failed [<<<<<<< ] [>>>>>>> ] Refs #17297
…ace.json (#17297) python-suite shard 10/12 red on this branch only -- six other open PRs pass the same shard, which rules out a flaky shard, a bad runner and an upstream condition. FAILED repo_tests/stranded_recorder_entries_test.py:: test_the_stranded_census_is_pinned_and_may_only_shrink AssertionError: Recorder entr(ies) name a path the tree no longer holds. A stranded entry is a failure to SHRINK: remove it, do not extend the census. {'.secrets.baseline': ('docs/.obsidian/workspace.json',)} Untracking docs/.obsidian/workspace.json left 11 entries in .secrets.baseline naming a path the tree no longer holds. All 11 were Hex High Entropy String with is_secret false -- Obsidian pane ids, never secrets. Removed; 301 files remain. The diff is 79 deletions and 0 insertions, so nothing else in the baseline was reformatted. This is a consequence of the deletion I did not think through, and a guard catching it is the guard working: a baseline entry for a file that no longer exists is an audit record pointing at nothing, and the census can only shrink. Swept the rest of the class rather than fixing the one the failure named. repo_tests/secrets_baseline_legacy_keys.json also names the path and is deliberately NOT touched: #17130 records that it is a census of already-audited (filename, type, hashed_secret) triples, kept so an excluded file cannot quietly gain a real secret. It records what WAS audited, not what currently exists, so entries for removed files are the point. The stranded guard passes with them in place. Log retrieved through the jobs API. `gh run view --job --log-failed` refused with "run is still in progress; logs will be available when it is complete" even though the shard had concluded FAILURE -- #17351, and the jobs endpoint serves it immediately. Refs #17297
4 tasks
4 tasks
mrveiss
added a commit
that referenced
this pull request
Sep 25, 2026
…elf (#17432) Review found the guard had no negative case. The parametrised test iterates the real tree, the tree now declares greenlet everywhere, so it passed trivially -- and would have passed identically if the detector were broken. A local mutation showed the detector worked on the day it was written; nothing in the suite would catch it breaking later. Correct finding, and it is the third instance of this shape in this repo today, each time caught by someone other than the author: a glob of requirements*.txt blind to requirements-ci/storage.txt an exclusion keyed on __file__.name that stopped covering a moved record an async-usage check that passed a file it never examined A detector with no contrast pair is the same family as no detector at all: it reports clean either way. Two synthetic fixtures under tmp_path -- one async service declaring SQLAlchemy without greenlet that must be flagged, one with greenlet that must not be. Both go through _violates(), the same single predicate the real-tree test uses, so the fixtures prove the detector rather than a re-statement of its rule. A contrast test that re-implements the logic agrees with itself and catches nothing, which is #17407's failure in another guard on the same day. Mutation-checked in both directions, which is what a pair is for: detector blind, greenlet always 'declared' -> negative fixture fails detector over-eager, nothing counts -> positive fixture + real tree fail async-usage check always False -> negative fixture fails 8 passed on the real tree. Refs #17432
mrveiss
added a commit
that referenced
this pull request
Sep 25, 2026
…#17432) (#17433) * fix(deps): declare greenlet where the async SQLAlchemy engine is used (#17432) Every docker-smoke-test run since 2026-09-24 ~20:10 failed, on every branch including main: Container autobot-slm Error ModuleNotFoundError: No module named 'greenlet' greenlet appeared in no requirements file in the tree. SQLAlchemy needs it for AsyncSession / create_async_engine, and it had been arriving transitively: SQLAlchemy declares it under an `[asyncio]` extra AND, in some releases, under a `platform_machine == "x86_64" or ...` marker. Verified against the installed metadata -- 2.0.49 carries both forms. So whether it arrives depends on which release a `>=2.0.54` floor resolves to and on the machine resolving it. Neither is a property this repo controls, and both can change with no commit here. That is exactly what happened. Declared explicitly in both backends rather than via the extra. An extra is still an inheritance, and the point is to stop inheriting a runtime requirement this code makes directly. Scoped by evidence rather than blanket. Four requirements files declare SQLAlchemy: autobot-backend/requirements.txt 278 async-engine files -> declared autobot-slm-backend/requirements.txt 83 async-engine files -> declared ai-stack/requirements-ai.txt 0 async-engine files -> untouched requirements-ci/storage.txt no source tree -> recorded autobot-backend had the identical latent gap and only the SLM container happened to fail first. The ai-stack image never opens an async engine, so adding greenlet there would be noise. requirements-ci/storage.txt declares sqlalchemy and aiosqlite and would need it if async-engine tests run against it, but nothing has been observed failing there and this is the PR that has to unblock a red base -- so it is an explicit, reasoned exemption in the new guard, not an omission. New guard: a requirements file that declares SQLAlchemy, whose own source tree uses AsyncSession or create_async_engine, must declare greenlet with a floor. Mutation-checked -- removing the line reddens it naming autobot-slm-backend/requirements.txt. Two defects found in my own scan while writing it, both by its own tests: the glob was `requirements*.txt`, which cannot see `requirements-ci/storage.txt` because that file is named `storage.txt`; the exemption-staleness test caught it, reading as "the file is gone" rather than "the scan is narrow". And the exemption list is checked against the tree so an entry cannot outlive the file it excuses. NOT established, and deliberately not closed on: whether `ERROR: Migration execution failed: relation "agents" does not exist` clears with this. The reading is that without greenlet the async migration machinery cannot run, so the table is never created -- but that ordering is unproven, and if it survives this fix it is a second defect that gets its own issue rather than widening this one. This PR's own smoke-test run is the measurement. Refs #17432 * fix(deps): mirror greenlet into requirements-ci instead of exempting it (#17432) code-quality red on the requirements-ci drift audit, one package: 1 package(s) declared in production but missing from requirements-ci.txt / requirements-ci/*.txt, and not recorded in repo_tests/requirements_ci_drift_baseline.txt: greenlet (greenlet>=3.1.0) Two things were wrong, and the second is the one worth recording. The mechanical error: I recorded the exemption in my own guard's list with my reasoning attached, and this gate keeps a different list -- repo_tests/requirements_ci_drift_baseline.txt -- which does not read mine. The judgement was written down somewhere that could not enforce it. The substantive error: the judgement itself was weak. I wrote "nothing has been observed failing there" as grounds for not declaring greenlet in requirements-ci/storage.txt. That is precisely the reasoning this PR exists to reject. A test that skips, or never opens an async engine, observes nothing either way -- and that invisibility is what hid the missing dependency for a day across every branch. "Not observed failing" and "does not need it" are different claims, and only the second justifies an omission. That file declares sqlalchemy AND aiosqlite, so a test opening an async engine against it fails exactly as the SLM container did. Mirrored: greenlet==3.3.2, matching the exact-pin convention of that file, with the reasoning inline. Drift audit now clean over 106 production packages. Also removes the now-obsolete entry from the new guard's exemption list. The mechanism stays -- empty dict plus the staleness test -- so a future exemption must carry a reason and cannot outlive its file. And replaces it with a STATED limitation rather than a silent one: a requirements file with no sibling Python tree reaches the async-usage check and finds nothing, so it passes without being examined. That is a pass by absence of evidence. Closing it means teaching the guard which suites run against which requirements set, which nothing in the tree records today. Said out loud because the rule this guard enforces is that exact distinction. Refs #17432 * fix(deps): cap SQLAlchemy below 2.1 -- 2.1.0 breaks the SLM backend (#17445) The greenlet fix unmasked a second defect rather than causing one. On main those tests died at collection (17 errors, no greenlet); with greenlet present collection succeeded and 123 ran, 27 failing inside SQLAlchemy's own cache-key machinery: TypeError: 'InternalTraversal' object is not callable sqlalchemy/sql/_cache_key_cy.py:196 in BaseHasCacheKey._gen_cache_key Traced rather than inferred. Two separate venvs, each internally consistent: 1055 $VENV/bin/python -m pip install -r requirements-ci.txt -> 2.0.54 1991 rm -rf "$SLM_VENV"; python -m venv "$SLM_VENV" 2002 $SLM_VENV/bin/python -m pip install -r autobot-slm-backend/requirements.txt -> 2.1.0 frames: .venv-python-suite-slm/lib/python3.14/site-packages/sqlalchemy/... So there is no mixed install and no cross-file conflict: the failing tests run in the SLM venv, which took 2.1.0 because `sqlalchemy>=2.0.54` had no upper bound. `requirements-ci/storage.txt`'s `==2.0.54` is a different environment and is healthy on the same commit -- which is what makes 2.0.54 known-good rather than merely previous. The traceback also confirms the greenlet fix works: it passes straight through `concurrency.py:199 greenlet_spawn`, so greenlet is present and the failure is downstream inside 2.1.0. Capped at <2.1 in all three service requirements files. The break is proven only in the SLM venv, but autobot-backend carries the identical unbounded floor over 278 async-engine files and is protected today only by CI installing it from requirements-ci's pin -- its own Docker build would take 2.1.0. Same reasoning as the greenlet declaration: fix the class, not the site that happened to fail first. This is a DEFERRAL and the comment says so. It restores the version CI already pins rather than choosing a new state, and converts an uncontrolled minor bump into a recorded one. Migrating to 2.1.x is separate work and is emphatically not a base-unblocking change. Not capping was not an option worth weighing against migrating: it is a choice to leave the base red with six PRs behind it. That is why this is on #17433 rather than posted as a decision. repo_tests/requirements_pin_parity_test.py and the new greenlet guard: 19 passed. Drift audit clean over 106 production packages. Refs #17445 * test(deps): give the greenlet guard a contrast pair, so it proves itself (#17432) Review found the guard had no negative case. The parametrised test iterates the real tree, the tree now declares greenlet everywhere, so it passed trivially -- and would have passed identically if the detector were broken. A local mutation showed the detector worked on the day it was written; nothing in the suite would catch it breaking later. Correct finding, and it is the third instance of this shape in this repo today, each time caught by someone other than the author: a glob of requirements*.txt blind to requirements-ci/storage.txt an exclusion keyed on __file__.name that stopped covering a moved record an async-usage check that passed a file it never examined A detector with no contrast pair is the same family as no detector at all: it reports clean either way. Two synthetic fixtures under tmp_path -- one async service declaring SQLAlchemy without greenlet that must be flagged, one with greenlet that must not be. Both go through _violates(), the same single predicate the real-tree test uses, so the fixtures prove the detector rather than a re-statement of its rule. A contrast test that re-implements the logic agrees with itself and catches nothing, which is #17407's failure in another guard on the same day. Mutation-checked in both directions, which is what a pair is for: detector blind, greenlet always 'declared' -> negative fixture fails detector over-eager, nothing counts -> positive fixture + real tree fail async-usage check always False -> negative fixture fails 8 passed on the real tree. Refs #17432 * fix(guards): close three false answers in the greenlet guard (#17432) Three review findings, all verified against the code before fixing, all real. (1) FALSE NEGATIVE. _ASYNC_MARKERS omitted async_engine_from_config, which is the async-engine entry point autobot-backend/migrations/env.py:18,99 uses -- and it uses no other marker in the set. A service reaching the async engine only that way passed this guard without declaring greenlet. Added, along with async_sessionmaker, AsyncEngine and AsyncConnection. (2) FALSE NEGATIVE, and the one that would have let the original outage through. The floor regex accepted `greenlet>=3.1.0; python_version < "3.0"` -- a declaration that can never install on the 3.14 runtime, counted as satisfying the rule. Verified against the old pattern before replacing it. The rule is now unconditional declaration: a floor and NO environment marker. Conditionality is the defect -- greenlet went missing because SQLAlchemy declared it behind an extra and a platform_machine marker, so a marker here rebuilds exactly that. Judging a marker needs a PEP 508 evaluation against the target environment, which this guard does not know, so it refuses markers rather than guessing which are benign. (3) FALSE POSITIVE. `any(marker in text ...)` over the raw file meant a comment or docstring saying "AsyncSession" made a purely synchronous service require greenlet. Now identifier-based: ast alias, Name and Attribute nodes, so prose produces no match. Substring pre-filter first, because parsing every module is what took a sibling guard past 120 seconds. An unparseable file falls back to the substring answer rather than reporting "no async usage", which would be a pass by failure to read. Over-strict is not the safe direction: a guard that cries wolf gets narrowed by whoever it inconveniences, and the narrowing is where real coverage is lost. A fixture per finding, and one of them exposed a fourth problem in my own test design. test_every_async_marker_is_detected is parametrised over _ASYNC_MARKERS, so dropping an entry drops its own test case -- the mutation left 15 tests passing. A set that supplies its own test cases cannot notice becoming smaller. Fixed with an anchor OUTSIDE the set: migrations/env.py references exactly one async name and no other marker, so it binds the entry the review found missing. Remove it and that test fails where the parametrised one cannot. Mutation-checked, 3 of 3 now caught, each by its own test: drop async_engine_from_config -> test_the_marker_set_still_detects_the_real_migrations_module accept a marker-carrying line -> test_a_greenlet_declaration_behind_an_environment_marker_is_refused back to raw substring -> test_naming_the_async_engine_in_prose_is_not_using_it 17 passed. Refs #17432 * docs(guards): cite the symbol, not the line, in the greenlet marker note (#15877) python-suite shard 7/12: 1 failed, 2901 passed. test_no_new_comment_cites_a_line_number async_sqlalchemy_declares_greenlet_17432_test.py cites autobot-backend/migrations/env.py:18,99 The comment I added for the async_engine_from_config finding cited line numbers. Replaced with the symbol: `run_async_migrations` in `autobot-backend/migrations/env.py` reaches the async engine through async_engine_from_config and through no other name in the marker set. Checked the rest of the branch rather than only the line the guard named -- `git diff origin/main...HEAD` shows exactly one such citation and this was it. The coincidence is worth recording because it is the same defect twice in one PR, at two levels. #15877 exists because a line-number citation fails SILENTLY into whatever later occupies the line -- the reference drifts and nothing says so. That is the identical shape as the guard this PR is about: greenlet arrived through a conditional upstream declaration, the condition changed, and nothing said so. I fixed the second while introducing the first, in the same file, in the comment explaining the fix. #15877 caught it loudly, which is what separates it from the class it guards against. 22 passed across both guards. Refs #15877, #17432
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
1 participant
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.
Thinking Path
docs/.obsidian/workspace.jsonhas carried a committed conflict block onmainsince 2026-04-03 (24d5193235). The file is not valid JSON, so anything parsing it fails.The interesting half is why the gate did not catch it, and it is not the answer I expected.
.pre-commit-config.yamlhas runcheck-merge-conflictsince 2025-08 — before the conflict landed — and reportedPassedon every commit since. Its first statement:is_in_merge()requiresMERGE_MSGand one ofMERGE_HEAD/rebase-apply/rebase-merge. These markers readUpdated upstream/Stashed changes— agit stash popconflict, which creates none of those. The hook exited before reading a single line of the file it was pointed at.So it was not misconfigured and it did not miss a pattern. It was structurally blind to this entire conflict class, and it answered "clean" while the markers sat in the tree. The fix is one flag,
--assume-in-merge, which skips the short-circuit and always scans.The stash detail is not incidental: the stash stack is shared across every worktree in this repo, which is exactly why a blind
popis hazardous here — and the conflict class the hook cannot see is precisely the one that hazard produces.AC2 turned out to need no decision at all. It asked whether this file belongs in version control.
.gitignore:509already listsdocs/.obsidian/workspace.json. The repo decided this years ago and recorded it in the one place that has no effect on an already-tracked file.git rm --cachedcompletes a decision already made rather than making a new one — worth stating, because "an agent removed a tracked file" and "an agent finished what.gitignorealready said" are different acts.docs/.obsidian/graph.jsonis kept: graph settings are shareable team config,workspace.jsonis per-user pane and cursor state and conflicts by nature.What Changed
.pre-commit-config.yaml—check-merge-conflictgainsargs: [--assume-in-merge], with the reason recorded inline.docs/.obsidian/workspace.json— untracked and removed. Already.gitignored.repo_tests/no_conflict_markers_in_tracked_files_17297_test.py— new CI guard.The guard is not redundant with the hook. A pre-commit hook is skippable with
--no-verifyand only sees the files a commit touches. This runs in CI over all 10,810 tracked files, so a marker arriving by a bypassed hook, a direct push or a wrongly-resolved merge is still caught. It also pins the--assume-in-mergeflag — because a disarmed hook still printsPassed, and nothing in the output distinguishes it from a clean tree.Verification
AC4 sweep, stated as a result rather than as silence — one file on
origin/main(6f5d119ef3), the known one, all three markers:Two things this issue's own acceptance criteria got wrong, both load-bearing
The proposed regex cannot match. AC3 specified
^=======with a trailing space. Git writes the separator alone on its line, so that pattern never matches it — a gate built to that spelling would find the outer two markers and silently miss the middle one. My first sweep used it and returned 2 of the 3 markers while looking complete. The guard usescheck-merge-conflict's own patterns instead, which handle the bare=======correctly; reusing them beat writing a third spelling of the same thing.The reach floor was wrong and the mechanism caught it. I first wrote
floor=2000against a live population of 10,810, andverify_floorrefused it:Ratcheted to 10,000 with growth 1,000, and the refusal is recorded at the declaration — a floor that far below the population passes while most of the tree stops being reached, which is the exact failure a floor exists to prevent. That is the mechanism working, not an obstacle to it.
Not verified here
The hook change is verified by the CI guard and by the sweep, not by observing a real
git stash popconflict being rejected at commit time. Arming the flag is a config change whose effect is exercised on the next conflicted commit, and I did not manufacture one to watch it fail.Model Used
Claude Opus 5 (1M context)
Closes #17297