feat: Deferred skills + skill search: load only a few top skills at start, update the system prompt to look up skills (scale t#394
Conversation
aa3fb4b to
a14d3ce
Compare
Rebase + Blacksmith testbox verificationRebased onto Blacksmith testbox — run 30145401580 ·
Two exclusions, both justified:
Full PR CI on the rebased head: 38 pass / 2 skipping / 2 pending (the multi-hour Windows Follow-up for the reviewer (not fixed here, to keep the diff scoped)Wiring |
a14d3ce to
c5f00c7
Compare
|
Round 2 — design defects addressed. Head BLOCKER 1 — cap fires when not needed. BLOCKER 2 — escape hatch cannot escape. BLOCKER 3 — wrong copy de-quadratified. DEFECT 4 — observability. New DEFECT 5 — duplicated preamble. The per-turn ranked fragment renders a new compact DEFECT 6 — latent trap. DEFECT 7 — recall. Added light Porter-style step-1a/1b stemming (plurals + Also: (a) Gate — Blacksmith Testbox run 30148059491, head
|
…tart, update the system prompt to look up skills (scale t
…tent index - codex-rs/ext/skills declared `test = false` on its lib target while the new `ranking` module adds `#[cfg(test)]` unit tests, which made cargo emit a warning and `cargo shear --deny-warnings` fail. Sibling ext crates that carry in-source tests (memories, web-search, workflows, image-generation) omit the flag, so drop it here too. - The skills extension is now installed in the app-server registry, so a `<skills_instructions>` contextual fragment can precede the user text in the request payload. Assert the combined user text is present among the user content segments instead of pinning it to index 0.
The skills extension is now installed in the app-server registry, so a <skills_instructions> contextual fragment is appended as an additional user input item after the user message. Search every user content segment for the combined text instead of pinning it to the last item.
…em skills The starter cap took the first `MAX_STARTER_SKILLS` entries of the scope-then-name ordering. Every session also loads the five bundled `System` skills, which sort first, so the starter list was always 100% built-ins and a workspace's own Repo/User/Admin (and plugin) skills never reached the model-visible list at all. Deal the starter slots round-robin across the scopes that are present, then render the winners in the usual deterministic order. Adds a unit test for the starvation case. Also retunes the core alias budget-pressure test: only five of its twelve skills now reach the developer message, so the context window has to be tighter for those starter entries to still exceed the 2% skills budget and exercise the alias rendering path.
…achable Round-2 review fixes for the deferred-skills change. The cap now only fires when it buys something, and everything it hides is genuinely reachable. - The `MAX_STARTER_SKILLS` cap is no longer unconditional. It applies only when the whole prompt-visible catalog cannot be rendered inside the skills context budget (checked under both the absolute-path and aliased-path layouts) *and* the thread exposes a `skills.list` catalog-search tool. A workspace with a dozen skills on a 200k window renders all of them again, and the budget machinery (aliasing, description truncation, omission warnings) is back on the live path instead of being near-dead code. `build_available_skills_with_starter_cap` exposes the ceiling for embedders. - Tie the cap to the tool it depends on. `codex-core-skills` now owns the canonical `skills`/`list` tool names, the extension uses them, and core derives `SkillCatalogSearch` from the thread's actual extension tools. An embedder built with `empty_extension_registry()` can no longer be shown a truncated list it has no way to search past. - `skills.list` can widen and page: `limit` default 5 / max 50, new `offset` (max 1000), and a `next_offset` cursor in the response. The old default == max made `limit` vestigial and left matches 6..N unreachable. - De-quadratify the real hot path. `catalog_from_outcome` runs on every turn and de-duplicated with a linear scan per insert; because every host entry shares one authority, each comparison degraded into a full string compare of long common-prefix paths. Adds a 20k-entry regression test. - Surface deferral in telemetry: new `codex.thread.skills.deferred_total`, and `codex.thread.skills.truncated` now also flags deferral, so a 2105 -> 5 drop can no longer be reported alongside `truncated = 0`. - Stop duplicating the ~2.5k-char `How to use skills` preamble. The per-turn ranked fragment renders a compact `## Task-relevant skills` body and defers to the developer-message block for the shared guidance. - Improve and pin down retrieval recall: light Porter-style plural/verb-form stemming so `charts` finds a `chart` skill, plus a test documenting that synonym paraphrases still return nothing. - Restore the `skills_use_aliases_in_developer_message_under_budget_pressure` 12k context window and assert all 12 skills render; add a deterministic core-skills mirror of it. Restore follow-up-turn catalog coverage in `installed_extension_injects_available_catalog_and_selected_entrypoint`. - Update CHANGELOG.
c5f00c7 to
a5bd59c
Compare
|
All contributors have signed the CLA ✍️ ✅ |
73dd117 to
55edb99
Compare
…ng about it Round-3 review fixes on top of the deferred-skills work. - Remove the offset ceiling. `skills.list` walked at most rank 1049 (MAX_SKILL_MATCH_OFFSET 1000 + MAX_SKILL_MATCH_LIMIT 50), so on a 2000-skill catalog 47.5% of the entries were unreachable by any offset/limit combination. Ranking is now bounded by the catalog only. - Never emit a cursor the tool rejects. Following `next_offset` verbatim used to hit `offset must be at most 1000`; deep offsets are now answered with a well-formed page, `next_offset` strictly advances, and new `total_matches`/`has_more` fields separate exhaustion from a partial page. - Add an enumeration escape hatch. `query` is optional: omit it (or pass an empty string) to page the catalog alphabetically. Lexical ranking cannot reach a skill whose wording the model has to guess, so enumeration is the only complete path. - Prefer truncating descriptions over hiding skills. Deferral now fires only when the catalog cannot be listed even without descriptions; a realistic twelve-skill workspace with 1536-char descriptions rendered five and silently dropped seven. Deferral also always warns. - Carry the rules when there is no `## Skills` block. Core builds that block from the host outcome while the extension ranks the merged catalog, so a remote/executor-only catalog left the model with bare skill lines and a pointer to a section that was never written. - Make the intro conditional. A complete list no longer claims to be a subset, and `skills.list` is not advertised on threads that do not install it. - Stop diluting the description-truncation metric with the full catalog size, and assert the new deferred_total metric. - Delete the callerless O(n)-per-insert `SkillCatalog::push_entry`.
55edb99 to
895feea
Compare
1 participant
Objective
Deferred skills + skill search: load only a few top skills at start, update the system prompt to look up skills (scale to thousands)
Do not load all skills. Load only a few top skills initially and make most skills deferred (not all injected). Add a skill-search mechanism and update the system prompt to always look for the relevant skills (consider deterministic user-prompt skill retrieval as a fallback). Research the proper approach — there may eventually be thousands of skills or more. The agent MUST start a goal plan for this task and drive it node by node (this is required). Investigate the current codewith implementation first and cite file:line. Build/test on remote Blacksmith only (never compile locally); keep cargo fmt (--config imports_granularity=Item), clippy --tests --all-targets -D warnings, and prettier clean; land a PR into hasna/codewith main.
Verification
Run
run_32621b0ae5f3· backendcodewith· task3e8a1d71-3890-4d38-b08e-02cddc43242d🏭 Generated by @hasnaxyz/factory