fix(hooks): a new session is shown the newest memories; the default briefing level is now minimal - #413
Merged
Conversation
…t, when scores tie The SessionStart hook ranks a project's memories by confidence, use and recency and keeps the top few. The daily decay multiplies the confidence of never-accessed memories by 0.9, so the memories captured since its last run carry one confidence value and, never accessed, score exactly alike (as do old ones that have sunk to the decay floor), and SQLite returns equal scores in ascending id order (measured): the cut kept the oldest of them. The lesson query, whose pool is claimed first, had no ORDER BY at all and kept the five oldest lessons. On a real graph, in the hours after a decay run, a new session was given the same two old commit lines every time and never the decision made an hour earlier, while `memesh briefing` on the same data (a newest-first window, stable-sorted) showed the right memories. Add `id DESC` as the last ORDER BY key of both scored query forms (the schema without scoring columns already ordered by it), which also covers the global and recent pools that `full` adds, and order the lesson query newest first. "Newest" is creation order, the key the briefing sorts by too. A higher score still beats a newer memory. Tests: a parity test in which every memory ties, lessons included (the hook byte-equal to the briefing, for both ranking forms); hook tests for a limit of 3 and for the lesson section; and a control showing that a higher score still wins, for both forms. Removing the scored tie-breaks turns five of the new tests red, removing the lesson ordering turns three red, and the two controls pass either way. docs/ARCHITECTURE.md and the CHANGELOG say what changed. Closes #401
The maintainer decided that a new session should get only what belongs to the project it is in: its decisions, lessons, known facts and recent activity, with the live repository state in front of them. The fresh task state and the capped index of durable memories now need `memesh config set briefing standard` (or MEMESH_BRIEFING=standard). Only the default moved: the levels, the env > config > default precedence and an explicit setting are unchanged, and an unset or invalid value now resolves to `minimal`. The constant in src/core/briefing-level.ts is the one owner of the default (the hook reads its generated copy). Everything else stated the old default and moved with it: source comments, the `briefing` and `task_state` MCP tool descriptions and the OpenAI export copy, AGENTS.md, the three READMEs, the skill text, ARCHITECTURE and API_REFERENCE, the dashboard's Project tab (all 11 locales) and the doc-parity lock. The CHANGELOG entry says what a new session no longer gets and how to get it back. Tests that leaned on the old default now say which level they need (pinned `standard` where the index or the task state is the subject); new tests cover the default on the hook, the built CLI and the MCP tool at once. The live journeys run at `standard`, the level they exercise. The suite's isolation helpers now drop an ambient MEMESH_BRIEFING, since the tests assume the default. Setting the constant back to `standard` turns 36 tests red across four files. A stale or unknown-age task state still collapses to a one-line flag at every level, the default included: the `briefing` tool description says so (the doc-parity lock is re-attested) and the age-unknown flag is now tested at all three levels.
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.
Closes #401.
Two commits, each reviewed on its own:
fix(hooks): a new session is shown the newest memories, not the oldest, when scores tiefeat(briefing): the default briefing level is now minimalWhat changes for a user
id DESCas the last ORDER BY key of both scored forms, which also serves the global and recent pools thatfulladds;ORDER BY e.id DESCon the lesson query). A higher score still beats a newer memory.minimal(wasstandard; requested by the maintainer). A new session, andmemesh briefing/ thebriefingtool, gets this project's decisions, lessons, known facts and recent activity, with the live repository state in front of them; the fresh task state and the durable-memory index come back withmemesh config set briefing standardorMEMESH_BRIEFING=standard. An explicit setting is honoured exactly as before (env > config > default); an unset or invalid value now resolves tominimal. Only the constant moved; every statement of the old default (source comments, both MCP tool descriptions, AGENTS.md, three READMEs, the skill text, ARCHITECTURE, API_REFERENCE, the dashboard's Project tab in 11 locales, the doc-parity lock) was updated with it, and the tests that assumed the old default were made to say which level they need.Evidence
memesh briefingdisagreed at every level; the hook returned the oldest of 43 tied memories and the newest decision was number 43 of 43.npm run verifyGREEN on the tree of each commit (commit 1: tree 66acd87a53a3; commit 2: tree 1ef0207d138f).standardturns 36 tests in 4 files red.Simplification pass
A read-only review of the whole diff looked for anything that could be cut without changing behaviour. Nothing was cut. It found two test-only candidates (about 11 lines) and no source or documentation candidate:
tests/core/briefing.test.ts: the tie-parity test restoresMEMESH_BRIEFINGby hand, although the file'safterEachalready callsvi.unstubAllEnvs();vi.stubEnvwould take two lines instead of nine.tests/hooks/session-start.test.ts: a five-line comment above the new describe block repeats the comment on the query it tests.Neither changes behaviour, and applying them would restart the whole 14-check CI matrix (the Windows legs alone take about half an hour) for a cosmetic gain, so they are left. Kept on purpose, with the reason: the 12-line comment above the query in
scripts/hooks/session-start.js(it records a measured SQLite behaviour that nothing else proves); the legacy-form leg of the parity test (nothing else compares the legacy hook's rendered text with the briefing's); thedocs/ARCHITECTURE.mdbullet (the only reader-facing statement of the tie rule); the twodelete isolatedEnv.MEMESH_BRIEFINGlines (they follow the existing per-helper pattern, and merging them would refactor code this PR does not own).Not in this PR (tracked)
minimalis the default. The proposed fix (select per layer in the shared leaf) changes whatsessionLimitmeans and needs the owner's decision.