Repository navigation
refactor(components): remove offloaders that delete by heuristic - #474
OsherElhadad wants to merge 7 commits into
Conversation
A component may now remove only content that is provably redundant (duplicates, superseded copies, formatting, boilerplate) or that it parks behind an expand handle chosen from the content itself. The following deleted by age, size, run order or prediction and are removed: failed_run, collapse, mask, linecap, smartcrush, readlifecycle, coref, agentdiet and skeleton (with the tree-sitter dependency). Presets, docs, the plugin component list, stats fields and benchmark arms no longer name them. A stored config that still does loads, logs a warning and runs without the component (the agentdiet preset likewise), so an old document cannot stop the proxy at boot. Refs #471 Signed-off-by: Osher-Elhadad <Osher.Elhadad@ibm.com>
Signed-off-by: Osher-Elhadad <Osher.Elhadad@ibm.com>
|
CI is green. Task-success check: the default |
|
Review of 20057e6 + c6b6242. Verified in a fresh worktree: go build, go vet clean; go test ./... passes except dash (SQLITE_BUSY, load-dependent) and one proxy test (database is locked); both fail the same way on origin/main under the current host load (load avg ~40), neither touches code this PR changes. TestRemovedComponentsAreIgnoredNotFatal and TestEveryPresetBuilds pass. Blocking
Non-blocking Safety: house pipeline unchanged; no defaults changed; go.mod drops the tree-sitter deps only. Privacy: nothing found in the diff. Attribution: no AI/Co-Authored-By lines in the two commits. Not labelled ready-to-merge until item 1 is fixed. |
Signed-off-by: Osher-Elhadad <Osher.Elhadad@ibm.com>
…e on /compact too Signed-off-by: Osher-Elhadad <Osher.Elhadad@ibm.com>
|
Re-review of 03b92fe: blocker 1 is resolved (the PR body carries no measurements). Item 3 is fixed and covered by TestRemovedPresetResolvesToEmptyPipeline. Build and vet are clean; config, cmd and components tests pass. The one failure, TestExtractLLMDoesNotBookASingleFlightFollowersSaving, is a flake that fails the same way on origin/main (29 of 40 runs on main, 31 of 40 here) and is unrelated. Remaining nit, not blocking: comments in components/offload/state.go (lines 59, 258, 287, 378, 429) and config/form.go (65, 509) still name mask, failed_run and linecap. Historical wording, fine to leave or tidy later. Approving; labelled ready-to-merge. |
Signed-off-by: Osher-Elhadad <Osher.Elhadad@ibm.com> # Conflicts: # context-guru-plugin/skills/insights-components/SKILL.md
…sk is removed Signed-off-by: Osher-Elhadad <Osher.Elhadad@ibm.com>
OsherElhadad
left a comment
There was a problem hiding this comment.
Review: APPROVE at f0a0237. (Posted as a comment: GitHub does not let the author account approve.)
- Merge 83be4b8. Its only conflict was
context-guru-plugin/skills/insights-components/SKILL.md, and it was resolved to main's version: the file has no diff against main and names none of the removed components. - Removed components.
failed_run,collapse,mask,linecap,smartcrush,readlifecycle,coref,agentdietandskeletonare no longer referenced by any preset, shipped config, plugin script or doc except in historical notes. A stored config that still names one keeps loading:dropRemoveddrops it with a warning, andpreset: agentdietwarns and runs the pipeline as written. - Fixtures. f0a0237 drives the thread-cache fixtures with
extract, and the test still fails ifextractacts on nothing ("the comparison proves little"), so the fixtures still test something. - Checks.
gofmtandgo vet ./...are clean.go testpasses for./config,./components/...,./apply,./context-guru-plugin,./proxyand./dash. CI is green. All new commits, including the merge, are signed off with no AI attribution. It merges cleanly with #475 and #500.
Non-blocking:
- The removed
TestKeptVerbatimNotRecompactedwas the only end-to-end pipeline test of the kept-verbatim guarantee, and it drovemask. The sharedskipReducepath still enforces it, but porting the test toextractwould keep it pinned. - Some
deploy/harbor/cfg-iter02*.yamlexperiment configs still listcollapseandcoref. They load with a warning; delete or annotate them as historical.
Signed-off-by: Osher-Elhadad <Osher.Elhadad@ibm.com> # Conflicts: # components/offload/skeleton_measure_test.go # components/offload/testdata/read_lifecycle.json # docs/components/offload-reducers.md # docs/components/skeleton.md
OsherElhadad
left a comment
There was a problem hiding this comment.
Re-review: APPROVE at 4e22725 (main merged; last approved f0a0237).
- Conflicts.
git show --remerge-diff 4e22725cshows four, all on files this PR deletes:skeleton_measure_test.go,testdata/read_lifecycle.jsonanddocs/components/skeleton.mdwere modify/delete conflicts. Main's only edits to them were path placeholders from #529, so dropping those edits is correct.- In
docs/components/offload-reducers.md, the resolution keeps the intro and the dedup/extract sections. It removes only the blocks for removed components: failed_run, linecap, smartcrush, mask, collapse and read_lifecycle. - Nothing else from main is lost.
- Leftovers. No build-time or config reference to the removed components remains. What does remain is historical comments, a CI comment about the old preset breakage,
internal/coref(still used by extract_llm_sweep), and test stubs that use the names only as strings.mkdocs build --strictpasses. - Tests. On a local merge with today's main,
gofmtandgo vet ./...are clean../config,./components/...,./apply,./context-guru-plugin,./proxyand./dashall pass. CI is green, and the merge commit is signed off.
The earlier non-blocking note (port the end-to-end "kept verbatim" test to extract) still stands. This approval covers 4e22725.
Part of #471 (compaction cleanup).
What changes
A component may now remove only content that is provably redundant (duplicates, superseded copies, formatting, boilerplate), or content it parks behind an expand handle where the choice of what to park comes from the content itself. Components that deleted by age, size, run order, or a prediction about the future are removed:
failed_run,collapse,mask,linecap,smartcrush,readlifecycle,coref,agentdiet,skeleton(and the tree-sitter dependency that onlyskeletonused).Removed from: code, presets (
balanced,aggressive,coding,mcp,agent,general,codesmart,codesafe, and theagentdietpreset), stats fields, docs and nav, the plugin component list, and the benchmark sweep arms.internal/corefstays:extract_llm_sweeppasses its index to the model as evidence.Kept after review:
format,textclean,searchfold,toon,dedup,extract,cmdfilter,extract_llm,extract_llm_sweep,summarize,cache_aware_summarizer,cachesplit,cacheinject,toolschema,toolfilter. The defaulthousepipeline never contained any of the removed components, so its behaviour is unchanged.Compatibility
A stored config that still names a removed component, or
preset: agentdiet, loads, logs a warning and runs without it. It does not stop the proxy.config.LoadBytesandConfig.Buildboth drop the names, so library callers are covered. Test:TestRemovedComponentsAreIgnoredNotFatal.TestEveryPresetBuildsnow fails if a preset names a removed or unregistered component.Evidence
The per-component audit and the replay figures are kept in the private research notes. Summary: each removed component failed the pre-declared check that the agent does not need removed text, or had no supporting measurement. The default
housepipeline is byte-identical in replay, with no warm-cache busts, format failures or reverts for any preset. Presets that carried a removed component lose the savings those components made; none of them is the service default.Tests
go test ./...passes (pure-Go build). Tests that usedmask,collapseorlinecapas a generic offloader fixture were moved toextractorcmdfilter; tests whose subject was a removed component were deleted.mkdocs build --strictpasses.Not in this PR:
marker_mode: summary|off(no expand handle, in no preset) is left as is and flagged in #471.