fix(mention-context): adopt hasna-mention-warm.py and record the co-location requirement (OPE15-00061) - #24
Merged
Conversation
…half (OPE15-00061) PR #23 adopted hasna-mention-context.py and shipped half a co-located pair. This commit adopts the other half, byte-identical to the live file, with no behaviour change. Any change to the warmer belongs in a separate commit. The pair is coupled in BOTH directions, and only the softer direction was visible in the repository before this commit: hook -> warmer WARM_BIN = <own dir>/hasna-mention-warm.py, guarded by os.path.isfile (src/hasna-mention-context.py:132, :1011). Absent warmer degrades SILENTLY: request_warm() returns early, so the cold-cache self-heal at :1200 never fires. warmer -> hook HOOK_PATH = <own dir>/hasna-mention-context.py, imported at MODULE SCOPE via H = load_hook() with no guard at all. Absent hook is an immediate FileNotFoundError. The warmer imports the hook deliberately, in its own words, "so cache paths, shapes and the sanitizer have ONE definition" — it derives CACHE_DIR, LOCK_PATH and LOG_PATH from the hook module. That is a shared-contract dependency, not an optional helper. The consequence that documentation alone could not reach: before this commit the repository held a file with an out-of-tree consumer that no reviewer could see. A change to CACHE_DIR, to the cache entry shape, or to the sanitizer would pass review and CI and break the warmer on every box, because the code binding to those names was not in the tree. Verified rather than assumed, two-sided so the check can both pass and fail: paired (this tree) IMPORT: OK, hook module bound: True isolated (warmer alone) IMPORT: FAILED FileNotFoundError RESULT paired=True isolated=False -> DISCRIMINATES Byte-identity against the live file at ~/.hasna/hooks/bin/: cmp rc=0 sha256 559ac9242ae6103523d7d4c4f889e68b01f0b7308c8aa8c90944668b7a0fe8f4 (identical both sides) size 19964 bytes both sides Mode 100755 matches its sibling hasna-mention-context.py. No live file was modified. Installation remains a separate, separately verified step, per the README. Refs: OPE15-00061, follow-up A from the adversarial review of #23 Agent: Silvanus
…eaks without it (OPE15-00061)
The README covered installation and never said the two files must sit together.
An installer following it copied the hook alone and lost the cold-cache
self-heal with no crash, no log line and nothing to notice.
Adds a "The two files are a pair" section stating the actual failure mode in
each direction, rather than that things may break:
warmer missing request_warm() returns at the os.path.isfile guard. A repo
mentioned for the first time is degraded once and STAYS
degraded on every later prompt instead of being clean
thereafter. It cannot recover on its own: by the hook's own
recorded measurement the live GitHub probe needs 1.02-1.18 s
against a 0.900 s deadline, so it is killed before it can
write the cache. The warmer is the only thing that fills it.
hook missing the warmer imports the hook at module scope with no guard;
FileNotFoundError, it does not start.
Also records the consequence for reviewers, which is the part an install note
alone does not reach: because the warmer imports the hook, the hook's CACHE_DIR,
cache entry shape and sanitizer are a contract with a second program in this
directory, not private details.
Two precisions that were measured rather than assumed:
- the cron entry invokes the warmer by ABSOLUTE path, so scheduled warming
does not depend on co-location. The hook's self-heal path is what does.
- installing the hook alone is supported and still works; the README says so,
so this reads as a decision with a stated cost rather than a prohibition.
Documentation only. No code, no behaviour change.
Verified: python3 test_run_capture.py -v -> Ran 6 tests, OK, rc=0;
bun test hooks/mention-context -> 3 pass, 0 fail, rc=0.
Refs: OPE15-00061
Agent: Silvanus
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.
Follow-up A from the adversarial review of #23. Todos
OPE15-00061.#23 adopted
hasna-mention-context.pyand shipped half of a co-located pair. Thisadopts the other half and records the requirement.
I chose to adopt rather than only document, and the reason is not tidiness
The row describes a one-way dependency: the hook resolves the warmer and the lookup is
guarded, so the loss is silent. That is accurate. Measuring it turned up a second
direction that was not in the row, and it is the harder one.
WARM_BIN = <own dir>/hasna-mention-warm.py(:132)os.path.isfile(:1011)request_warm()returns, self-heal never firesHOOK_PATH = <own dir>/hasna-mention-context.py(:57)FileNotFoundErrorat module scope (H = load_hook())The warmer imports the hook on purpose — in its own words, "so cache paths, shapes and
the sanitizer have ONE definition" — and derives
CACHE_DIR,LOCK_PATHandLOG_PATHfrom the hook module.
That is what documentation could not have reached. Before this PR the repository held
a file with an out-of-tree consumer that no reviewer could see. A change to
CACHE_DIR,to the cache entry shape, or to the sanitizer would pass review and pass CI and break the
warmer on every box, because the code binding to those names was not in the tree. A README
line is read by installers; it is not read by someone changing a constant. Adoption puts
the dependent code where review can see it, which is the failure mode with the longest
tail.
Two secondary arguments, both measured:
package.jsonshipshooks/, so the publishedpackage now carries both halves; and a repo-only checkout is now self-consistent, where
before it was not.
Commits
693020a— adoption, byte-identical, no behaviour change.1fa3087— README only.Verified rather than assumed
Byte identity of the committed blob (not merely the working tree) against the live
file at
~/.hasna/hooks/bin/hasna-mention-warm.py:The co-location coupling, two-sided so the probe can both pass and fail — the same
warmer bytes imported from a directory with the hook beside it, and from one without:
Arm A also proves the two files in this tree are contract-compatible:
H.CACHE_DIRandthe paths derived from it resolve at import with no
AttributeError.Tests, both paths:
Staged secrets scan, run before each commit, with a positive control on the same
pattern set so a zero is not an unvalidated zero:
The temp-path asymmetry (follow-up B): left alone deliberately, with a note
The warmer builds its own temp path with no uniqueness component, so the
one-file-per-
run_capture-call invariant that #23 established holds for the hook file andnot for the warmer.
Not changed here, and it should not be. It is not a live defect — the warmer is
single-threaded, has no thread pool and no concurrent submission, and dedupes its tokens.
Changing it would mean a behaviour change to a cron-driven program inside a PR whose
adoption commit is required to be byte-identical, with no failing test to justify it. It
is recorded on the todos row as follow-up B, which is the right place for it.
It is not mentioned in the README either: a README is not where a non-defect belongs,
and writing it up as a caveat there would misrepresent a sound program as a fragile one.
Not checked
~/.hasna/hooks/bin/are untouched;the hook there fires on every prompt on this machine and installation is a separate,
separately verified step per the README.
main(), acquiredthe lock, or hit the network.
PR makes no claim about its correctness — only about where it lives.
One inconsistency observed, not fixed
Both files carry a comment describing the warmer as running on a 5-minute cron
(
hasna-mention-context.py:105, and theAGE_GHnote in the warmer). The actual crontabon station01 runs it at minutes 1,11,21,31,41,51 — every 10 minutes. The substantive
sizing note at
:116-118is correct and does say 1,11,21,...; only the two parentheticalsare stale. Pre-existing in both files, cosmetic, and out of scope for a byte-identical
adoption — flagged rather than silently corrected.
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.