Skip to content

Raise default cache retention from 30 to 90 days; bump deps - #406

Merged
platypii merged 3 commits into
masterfrom
retention-90-and-deps
Jul 27, 2026
Merged

Raise default cache retention from 30 to 90 days; bump deps#406
platypii merged 3 commits into
masterfrom
retention-90-and-deps

Conversation

@platypii

Copy link
Copy Markdown
Contributor

Retention 30 → 90

The 30-day default was narrower than the questions people actually ask of the cache (quarter-scale comparisons, month-boundary rollups), and it forced a round trip to the org server for work the local cache should answer on its own.

The default lived in more places than one constant. Every site that derives the default changed; explicit test/smoke fixtures were left alone.

SiteWhat it is
src/core/cache/retention.jsDEFAULT_RETENTION_DAYS (cache enforcer)
src/core/cli/walkthrough.jsDEFAULT_RETENTION_DAYS (onboarding, team pathway)
src/core/commands/init.js--retention-days flag default
src/core/daemon/status.jsstatus reporter's no-config fallback
hypaware-core/plugins-workspace/claude/src/index.jspreset config writer

The last two were hardcoded 30s duplicating the default, so they now reference DEFAULT_RETENTION_DAYS instead of restating 90 — that's why the diff touches them structurally rather than as a one-character change.

LOCAL_INSTALL_RETENTION_DAYS stays at 120. LLP 0137's rationale (on a local-only install the cache is the only copy of history) still holds, and the ordering 90 < 120 is intact.

Docs

Per the living-docs rule, the doc edits land with the code: LLP 0137 (the #pathway-defaults decision, with a dated revision note recording why the window widened), LLP 0011, LLP 0013, README, CONTEXT.md. All @ref LLP 0137#pathway-defaults annotations still resolve to a live anchor.

Deps

  • @aws-sdk/client-s3 3.1094.0 → 3.1096.0
  • @aws-sdk/credential-provider-ini 3.973.5 → 3.973.7
  • @types/node 26.1.1 → 26.1.2

TypeScript held at 6.0.3. 7.0.2 is a major bump that surfaces 61 type errors across 23 files (mostly TS2339, plus TS2345 from stricter literal-type inference on schema objects like type: 'object'). That's a migration, not a version bump, so it's deliberately not folded in here — and 6.0.3 is already the newest 6.x, so there was no safe intermediate. Worth its own branch.

Verification

  • npm test — 2664 pass, 0 fail
  • npm run typecheck — clean
  • Release smoke battery (13 flows) — all green

Two smokes fail, and both fail identically on master — pre-existing, unrelated to this change:

  • config_load_validate — a sink_pair_incompatible error-kind assertion
  • walkthrough_to_first_queryai_gateway_messages has exactly one row, value=0

One note on that second one: this change did initially break it earlier in the flow, at a golden config fixture. That's fixed here — the golden and two derived status assertions now expect 90 — and it fails at exactly master's failure point. Confirmed by stashing and diffing the failing assertion, not just the exit code; the two failures were indistinguishable at the exit-code level but were not the same failure.

The 30-day default was narrower than the questions people actually ask
of the cache (quarter-scale comparisons, month-boundary rollups) and
forced a round trip to the org server for work the local cache should
answer itself. The local-install default stays 120 days.
Wire the two hardcoded duplicates of the default (the `hyp init`
--retention-days flag default and the status reporter's fallback) to
DEFAULT_RETENTION_DAYS so they cannot drift again.
Update LLP 0137 (the pathway-defaults decision), plus 0011 and 0013.
Deps: @aws-sdk/client-s3 3.1094.0 -> 3.1096.0,
@aws-sdk/credential-provider-ini 3.973.5 -> 3.973.7,
@types/node 26.1.1 -> 26.1.2. TypeScript held at 6.0.3; 7.0.2 is a
major bump that surfaces 61 type errors and needs its own migration.
@platypiiplatypii added the neutral:adopt Foreign PR adopted into neutral's reconcile scope label Jul 27, 2026
README step 3 and docs/PRIVACY.md still described retention as something
onboarding asks the user to pick. LLP 0137 removed that question and made
the window pathway-scoped, so both now state the real behaviour: 90 days
on a team install, 120 on a local-only one, with `hyp init
--retention-days <N>` as the override.
Co-Authored-By: Claude <noreply@anthropic.com>
@philcunliffe

Copy link
Copy Markdown
Contributor

Verdict: approve with doc fixes applied

Reviewed head 254c066. The retention bump itself is correct and complete. Every production call site of the old flat 30 is now sourced from a named constant, both DEFAULT_RETENTION_DAYS definitions moved together, tests and the smoke golden config were updated in step, and LLP 0011 / 0013 / 0137 / CONTEXT.md were amended in the same commit as the living-docs rule requires. npm test, npm run typecheck, and npm run build:types are green (the 8 leave ... failures reproduce identically on master, so they are pre-existing and unrelated to this branch).

Back-compat checks out: composePickerConfig always writes query.cache.retention.default_days into the config it generates (src/core/cli/walkthrough.js:687), so installs created before this change keep their pinned 30-day window. Only new installs, and hand-written configs that omit the block, pick up 90.

Fixed in this PR (pushed as 58b0a47)

1. minor, docs/PRIVACY.md:23: stale 30-day default. The privacy page still told users "Rows age out of the local cache after the retention window you picked at init (default 30 days)." Besides the number, "you picked at init" contradicts LLP 0137, which removed the question entirely. Rewritten to state the pathway-scoped reality and the --retention-days override.

2. minor, README.md:61: the walkthrough does not ask for retention, and the number was wrong for the flow described. The PR updated the number in place but kept the "Pick a retention window" framing. Two problems:

  • The interactive wizard never prompts. src/core/cli/wizard/pick.js:151-156 assigns retentionDays unconditionally with the comment "Retention is not asked either".
  • The section is titled "Quickstart (solo, fully local)". hyp init with no argv on a TTY routes to runInitWizard (src/core/commands/init.js:102), and the local pathway supplies LOCAL_INSTALL_RETENTION_DAYS = 120 (src/core/cli/wizard/index.js:135). So "default 90 days" is the wrong number for the very flow the section documents.

Step 3 now describes the pathway defaults (90 team / 120 local) plus the override, matching LLP 0137 #pathway-defaults.

Findings left open (deliberately out of this PR's scope)

3. minor, README.md:59: step 2 has the same drift. "Pick an export strategy: keep the local query cache only, write Parquet files ... or configure later" is no longer true; the interactive path hardcodes local-parquet (src/core/cli/wizard/pick.js:147-151), with other destinations reachable only via hyp init --export. Same class of staleness as finding 2 but caused by a different change, so it wants its own fix.

4. minor, the widened default also widens the default backfill lookback.resolveRetentionDays (src/core/commands/backfill.js:817) terminates in the same DEFAULT_RETENTION_DAYS, so for a config with no explicit retention block, hyp backfill now imports 90 days of local history rather than 30, and createRetentionEnforcer keeps 3x the rows before purging (src/core/cache/retention.js:513-518). LLP 0137's revision note argues the disk envelope holds, but only for the cache; the backfill-lookback consequence is worth a sentence in the LLP or an acceptance-smoke bound (the installed_daemon_idle_soak bounded-growth candidate is the natural home).

5. nit, llp/0010-config-model.spec.md:37 still shows "retention": { "default_days": 30 } in its example config. It never claims 30 is the default, so it is not contradictory, but a reader may copy it as canonical.

6. nit, duplicated constant.DEFAULT_RETENTION_DAYS exists independently in src/core/cache/retention.js:28 and src/core/cli/walkthrough.js:57. Both were bumped correctly here, but nothing prevents them drifting; a test asserting they are equal would be cheap insurance. Note also that parseInitFlags' default is untested (the function is not exported), so the --retention-days flag default is covered only indirectly.

7. nit, package.jsonexports reformatting (one-line entries expanded to multi-line) is unrelated to the stated purpose of the PR. Verified semantically identical to master, so harmless, just extra diff noise.

8. info, dependency bumps.@aws-sdk/client-s3 3.1094.0 to 3.1096.0, @aws-sdk/credential-provider-ini 3.973.5 to 3.973.7, @types/node 26.1.1 to 26.1.2. All patch-level, and the repo carries no lockfile, so there is nothing to keep in sync. Note that this review environment could not install at the pinned versions, so tests and typecheck exercised older resolved copies; CI installing fresh is the real check.

Style and convention

Clean. No semicolons or em dashes on any added line, no @typedef, no inline import('...') types, no non-root-anchored type-import specifiers. The two @ref LLP 0137#pathway-defaults annotations still resolve (the pathway-defaults anchor is present in the revised doc) and their glosses were updated to match the new number, as the keep-refs-honest rule requires.


Posted by neutral. Findings 3 to 8 are informational and left for the author to take or leave; nothing here blocks the PR. The head has moved to 58b0a47, so neutral will re-review the new head on its next tick.

@philcunliffephilcunliffe added the neutral:adopted Adoption completion record: merged while carrying neutral:adopt (LLP 0031) label Jul 27, 2026
…ault
The canonical v2 config sample in the config-model spec was left at
default_days: 30 while its structural twin in LLP 0013 moved to 90. The
value is incidental to 0010's point (there is no mode flag), but a reader
scaffolding a config from the spec would get a window that contradicts the
shipped default.
@philcunliffe

Copy link
Copy Markdown
Contributor

Review round 1 — 58b0a47

Verdict: findings, no blockers. The retention change is correct and complete; one doc inconsistency was fixed and pushed, two observations need no action.

The 30 → 90 change is complete

Enumerated every site that derives the default; all are updated:

SiteRole
src/core/cache/retention.js:28DEFAULT_RETENTION_DAYS — feeds normalizeRetentionConfig (:512-517) and resolveRetentionDays (commands/backfill.js:817)
src/core/cli/walkthrough.js:57DEFAULT_RETENTION_DAYS — feeds runPickerWalkthrough (:398/:1230/:1250), wizard/pick.js:156,323,343,348
src/core/daemon/status.js:945was a bare 30; now imports the cache constant — the right one, so hyp status reports what the enforcer actually applies
src/core/commands/init.js:169,182two bare 30s, now imported from walkthrough.js
hypaware-core/plugins-workspace/claude/src/index.js:384the only plugin preset that writes default_days (checked codex/gascity/central/otel — none do)

Every remaining 30 in the tree is a genuine fixture, not a derived default: 15 smoke flows passing an explicit retention.default_days: 30 and asserting it back (e.g. status_diagnostics.js:424 → :155/:210-212), smokes passing explicit --retention-days 30 (walkthrough_picker_to_first_query.js:192,403), and test/core/init-configured-entry.test.js:44's synthetic renderer report. No missed site.

LOCAL_INSTALL_RETENTION_DAYS = 120 intact (walkthrough.js:58), consumed at exactly one place — the conditional spread at wizard/index.js:135, resolved by pick.js:156 as opts.retentionDefault ?? DEFAULT_RETENTION_DAYS. There is no clamp or comparison between the two, so 90 < 120 is descriptive rather than load-bearing.

LLP discipline satisfied.<a id="pathway-defaults"></a> is live at llp/0137-onboarding-retention-defaults.decision.md:22; all four @ref LLP 0137#pathway-defaults annotations (walkthrough.js:56, wizard/index.js:134, wizard/pick.js:155, test/core/cli/wizard/pick.test.js:132) resolve, with glosses updated. Doc edits landed in 254c066, same commit as the code, and the revision note is dated rather than rewriting history.

Findings

1. NON-BLOCKING — fixed and pushed (f095c30).llp/0010-config-model.spec.md:37: the canonical v2 config example still read "default_days": 30, while this PR updated its structural twin at llp/0013:37. The value is incidental to 0010's point (illustrating that there is no mode flag), but a reader scaffolding a config from the spec would get a window contradicting the shipped default. Changed to 90.

2. NON-BLOCKING — no action taken.package.json:12-43: the exports block was reflowed to multi-line, producing 40 of the file's 46 changed lines, unrelated to the dep bump. Parsed base-vs-head, exports is byte-identical after parse with unchanged key order — the only semantic deltas are the three intended versions. No behavioral risk; noted only because it inflates the diff and will conflict with any concurrent exports edit. Left as the author wrote it.

3. NON-BLOCKING — no fix needed.test/core/cache-retention-maintenance.test.js:53-54 asserts DEFAULT_RETENTION_DAYS === 90, a pure mirror of the constant. Pre-existing shape (it read 30 before), and the PR's other test updates do pin real behavior: wizard/pick.test.js:143-144 pins the value written to the config file on disk, :156-158 pins the 120 local override, walkthrough-backfill.test.js:254 pins the value out of a cancelled-consent run.

Style: clean — no U+2014, no semicolons added, no TypeScript-isms, no inline import() types, no @typedef. There is no lockfile in this repo, so the three exact-pinned dep bumps have no lock to drift.

Verification run

Ran in an isolated detached worktree at 58b0a47:

  • npm run typecheckpassed, no output.
  • npm test2656 pass / 8 fail / 1 skipped. All 8 failures are in test/core/leave-command.test.js (exit 1 vs 0 in the hyp leave teardown). Rebuilt a second worktree at base b4233a2 and reran: 8/8 fail identically at base — pre-existing, and this PR touches no leave/central code. CI is the authority and is green.
  • npm run smoke -- walkthrough_to_first_queryfails on ai_gateway_messages has exactly one row (value=0); identical failure at base, environmental (the gateway capture leg needs outbound traffic this sandbox blocks). The retention assertions this PR changed sit earlier in that flow and passed: hyp status mentions '90 days', golden config default_days: 90, status.render span retention_days === 90.
  • Cold-start check on status.js's new ../cache/retention.js import (it pulls hyparquet/icebird): cli/core_commands.js:3 already statically imports commands/backfill.js, which already imports cache/retention.js, so the module was in the CLI graph already. Measured dispatch.js cold import at 128ms with and without.

The head has moved to f095c30 with the doc fix, so the next tick re-reviews at that head.

@platypii
platypii merged commit 39bc895 into masterJul 27, 2026
8 checks passed
@platypii
platypii deleted the retention-90-and-deps branch July 27, 2026 21:05
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

neutral:adoptForeign PR adopted into neutral's reconcile scopeneutral:adoptedAdoption completion record: merged while carrying neutral:adopt (LLP 0031)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@platypii@philcunliffe