Skip to content

Test log isolation, stale references, DCA stop distance, manifest name refusal (#964 #967 #973 #974) - #978

Merged
eaitbrahim merged 6 commits into
mainfrom
fix/964-967-973-974-hygiene
Oct 12, 2026
Merged

eaitbrahim merged 6 commits into
mainfrom
fix/964-967-973-974-hygiene

Conversation

@eaitbrahim

Copy link
Copy Markdown
Contributor

Closes #964
Closes #967
Closes #973
Closes #974

Summary

#964: the test suite no longer leaves logs/keel.log in the checkout

  • Cause: configure_logging opens logging.file (default: the relative path logs/keel.log) against the cwd. Many CLI tests load a config that has no logging: section, so this was not one bad test. A bisect plugin found tests/commands/test_dca_cli.py as the first writer.
  • Fix: a new autouse fixture, tests/conftest.py::_run_in_tmp_path, runs every test from its own tmp_path and sets KEEL_HOME to that same folder.
  • Guard: another autouse fixture, _the_checkouts_logs_are_untouched, records the files under <repo>/logs and <invocation cwd>/logs (size and mtime) before each test. If either changed afterwards, it fails that test by name.
  • The timeline-export tests now set their own absolute logging.file. A probe that put an empty file at that path made 3 tests fail, so the pinned path is the one the export actually reads. The composite-kind test now asserts exact kinds instead of "may or may not appear".
  • Fallout from changing the cwd, now made explicit:
    • The simulate and telemetry tests pass --config <repo>/config.yaml.
    • test_research_record finds its module through __file__.
    • The alpaca .env test unsets KEEL_HOME.

#973: positions reads the stop through status.recorded_stop

  • stop_distance and stop_distance_pct are now None for a DCA tranche that stores initial_stop "0" (or below). PositionRow.initial_stop keeps the stored value, because Holdings' paper reading (shown_stop) names that 0.
  • Corrected the docstrings in positions.py and doctor.py.

#974: scripts/rule_manifest.py apply refuses the params keel rules add refuses

  • The params checks in add_rule_row moved into a new function, rules.validated_row_params. It covers unknown kind, type mismatches, unknown kwargs, the rule's own validation, non-finite values and dropped params, which includes name.
  • apply runs it on every manifest entry. A refused entry prints REFUSED: kind:product: <the same words add_rule_row prints> and is not written; the other entries still apply, and the exit code is 1.
  • The committed deploy/live-rules.json still applies cleanly.

#967: stale references corrected, each pinned by a test that reads the code

  • _common.record_cash_posture_refutation named keel balances and keel brokers list. It now names the real callers that pass no repo; a test resolves every keel ... it cites through click.
  • server._REFRESH_SEC is deleted. Its two citations (events.py, api.py) now point at main.js's POLL_MS. Tests check that every private constant in keel/web/*.py is read somewhere in its module, and that any `server._CONST` cited anywhere in keel/ exists.
  • capabilities.render_lines no longer mentions a browser view. A test requires its docstring to name every module that imports it.
  • gates.py: "the other seven" is now "eight", and the count is derived from CAPABILITIES. "LAST of four checks" is now "seven". That count is taken from the AST of do_POST and _gated_action (their top-level if not guards, plus the phrase), and each guard function must be named in the docstring.

Rulings

  • tests: suite leaves logs/keel.log in CWD; next run fails test_timeline_export header tests #964 mechanism: every test runs in its own tmp_path with KEEL_HOME pinned, and a per-test guard watches logs/. Fixing one test at a time would miss the next CLI test someone writes. Pinning KEEL_HOME keeps a lookup from the bare tmp_path from falling through to the operator's real app-data folder. The guard runs per test so the offending test is named without a bisect.
    • Cost if wrong: a test that reads the checkout through a relative path now fails, and has to switch to __file__. Also, an outside process writing to the checkout's logs/ during a run would fail whichever test was running at that moment.
  • scripts/rule_manifest.py apply stores a "name" param; a non-DCA rule named "dca" would read "no stop by design" #974 hardening skipped: the bracket verdict still keys off positions.rule_name, not rules.kind via rule_id. Changing it means threading rule_id through OpenPositionStatus and PositionRow, plus 4 call sites in payload.py and pulse.py that parallel PRs are editing. Unattributed tranches have no rule_id anyway. The reason is now written in _kind_never_brackets' docstring.
    • Cost if wrong: a non-DCA row named "dca" that was written by hand in SQL would still read "no stop by design".
  • Stale references to retired console surfaces outside keel/web (keel balances, _REFRESH_SEC, capabilities 'browser view') #967 count wording: the phrase check is the LAST of seven checks. The count uses the vocabulary do_POST already uses: four checks on every write, including the /api/ prefix. The unknown-key 404 and the body-shape check are not counted.
    • Cost if wrong: if someone argues for a different count, only the wording changes. The test keeps the number in step with the code. Note that a guard added to do_POST by the parallel halt/serve PR will fail this test until that PR updates the docstring. That is intended.

Verification

  • ruff check keel tests packages: clean. ruff format --check .: clean. mypy (bare): clean, 509 files.
  • Full pytest twice back-to-back, with no logs/ deletion in between: 8319 passed, 3 skipped both times, and no logs/ afterwards. Before the fix, a full run left logs/keel.log behind; with an empty logs/keel.log and the old conftest, the two header tests fail, as tests: suite leaves logs/keel.log in CWD; next run fails test_timeline_export header tests #964 reports.
  • Mutation testing: 11/11 mutants killed. Each mutant's source change was confirmed, and each file was restored from a saved copy; the tree was identical afterwards.
  • CLI check: a scratch copy of the repo's paper config and db, switched to confirm mode, seeded with a DCA tranche (rule_id 19, initial_stop "0").
    • keel status lists that tranche as rule=dca (no stop by design).
    • gather_positions gives stop_distance=None; origin/main gave 64715.98 (the full mark, pct 1).
    • keel positions is only the close group and lists no table.
  • No JS was touched.

🤖 Generated with Claude Code

eaitbrahim and others added 4 commits October 12, 2026 11:56
…the checkout's logs/ (#964)

configure_logging opens the relative logs/keel.log against the cwd, so every CLI test
loading a config without a logging: section left a file in the checkout that the next
run's timeline export read back as an empty engine log. An autouse fixture now chdirs
into tmp_path and pins KEEL_HOME there; a per-test guard fails the test that touches
<repo>/logs by name. The export tests pin their own log path.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…has no distance (#973)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ll (#974)

Extracts rules.validated_row_params from add_rule_row; apply runs it per entry and
skips a refused entry. The rules.kind-via-rule_id verdict hardening is ruled out in
_kind_never_brackets' docstring.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…lly (#967)

keel balances citation, dead server._REFRESH_SEC and its two citations, render_lines'
browser view, gates.py's seven -> eight and LAST of four -> seven checks.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@eaitbrahim eaitbrahim self-assigned this Oct 12, 2026
…ly (#967)

payload._order_payload is payload._order_row_payload; server.needs_database and
server.ROUTES were deleted with the renderer at #540, so the prose now says so
instead of pointing at them. test_a_cited_web_module_attribute_exists resolves
every backticked keel.web module.name in keel/**/*.py by import.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…en import this tree, gates.py counts agree (#978)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@eaitbrahim

Copy link
Copy Markdown
Contributor Author

Gatekeeper: READY at d1f8a0a. Not merged; the owner merges.

Rounds

  • Round 1: 3 findings, all fixed in d1f8a0a.
    • Major: test_rules_seed_unknown_kind_errors passed vacuously under the new tmp_path fixture, because no config was found. It now runs with --config and asserts the refusal text.
    • Minor: python -c children imported keel from the venv install instead of the tree under test. The conftest now prepends the repo root to PYTHONPATH.
    • Minor: the gates.py docstring said "three checks". It now says seven, and both count sentences are pinned to the count derived from the AST.
  • Round 2: zero findings. 9 of 9 mutants killed.
  • Scope addition (6685c63): fixed three dead references, payload._order_payload, server.needs_database and server.ROUTES. They are pinned by test_a_cited_web_module_attribute_exists, which resolves every keel.web citation in keel/ by import.

Verification at d1f8a0a

  • CI is green.
  • Full pytest twice back-to-back: 8321 passed, 3 skipped, both runs. No logs/ was left.
  • ruff, format and bare mypy are clean (510 files).
  • CLI check on a confirm-mode scratch copy with a DCA tranche storing initial_stop "0":
    • keel status shows rule=dca (no stop by design).
    • The positions stop_distance is None, where it was previously the full mark.

Follow-up and merge note

@eaitbrahim
eaitbrahim merged commit 8189ddd into main Oct 12, 2026
4 checks passed
@eaitbrahim
eaitbrahim deleted the fix/964-967-973-974-hygiene branch October 12, 2026 20:37
eaitbrahim added a commit that referenced this pull request Oct 12, 2026
#978's prose test counted do_POST's refusing guards; #977 made do_POST a
drain wrapper around _post, so the walk never reached _gated_action and
main went red. The count is unchanged (four + two + the phrase = seven),
so only the test moves. Mutation-checked: dropping the Sec-Fetch-Site
guard from _post fails it.

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment