Skip to content

Integrating helper agents(TitleAgent / PurposeAgent / DescriptionAgent) into a common interface#50

Merged
grf53 merged 4 commits into
refactoring-appliedfrom
refactor-helpers
May 6, 2026
Merged

Integrating helper agents(TitleAgent / PurposeAgent / DescriptionAgent) into a common interface#50
grf53 merged 4 commits into
refactoring-appliedfrom
refactor-helpers

Conversation

@grf53
Copy link
Copy Markdown
Contributor

@grf53 grf53 commented May 3, 2026

Ticket

#49

Summary

  • Centralize provider configuration on ailoy's process-global default — main.rs populates it once at boot; helpers stop reading env directly.
  • Lift the duplicated dispatch shell of TitleAgent / PurposeAgent / DescriptionAgent into a single HelperAgent trait with a fixed {"result": "<string>"} JSON contract.
  • Pick the first registered model from a preference list per helper; fall back gracefully when no provider is registered.
  • Scope all helper internals to module-private — Store is the sole public entry point for LLM-backed metadata.

Why

Helpers each called dotenvy::dotenv() + OPENAI_API_KEY and copy-pasted ~25 lines of agent setup / streaming / parsing. Without OPENAI_API_KEY — even with Anthropic or Gemini keys — Store::ingest and Store::describe died with "No provider found for model …".

What changes

  • main.rs populates ailoy::agent::default_provider_mut once at boot; build_agent reads via default_provider().
  • HelperAgent trait owns dispatch, streaming, JSON-field extraction, response-empty fallback, and model selection. Helpers declare only INSTRUCTION, build_query, fallback.
  • MODELS defaults to ["openai/gpt-5.4-mini", "anthropic/claude-haiku-4-5-20251001", "google/gemini-2.5-flash"] (override per helper if a different preference order is needed) and is checked against ailoy's registered glob patterns (openai/*, anthropic/claude-*, google/gemini-*); first match wins. Nothing registered → log::warn! + fallback without an LLM call.
  • Response contract is {"result": "<string>"}; the trait extracts and substitutes fallback (logging the agent via std::any::type_name) on empty / malformed responses.

Adding a new helper

use ailoy::message::{Message, Part, Role};
use super::helper::HelperAgent;

struct SummaryAgent;

impl HelperAgent for SummaryAgent {
    type Input<'a> = &'a str;
    type Output = String;
    // const MODELS: &'static [&'static str] = &[...];  // override if needed
    const INSTRUCTION: &'static str = concat!(
        "Summarize the document in 3 sentences. ",
        "Return ONLY a JSON object: {\"result\": \"<string>\"}.",
    );
    fn build_query(&content: &&str) -> Message {
        Message::new(Role::User).with_contents([Part::text(content.to_string())])
    }
    fn fallback(_: &&str) -> String { String::new() }
}

// SummaryAgent::generate(content).await

MODELS, process, generate, agent identifier, model-availability check, and fallback path all come from trait defaults.

Test plan

  • cargo build --workspace
  • cargo test -p speedwagon --lib
  • LLM round-trip: OPENAI_API_KEY=… cargo test -p speedwagon --lib -- --ignored describe_round_trips_against_pre_populated_index
  • Manual: only ANTHROPIC_API_KEY (or only GEMINI_API_KEY) set → ingest succeeds via that provider.

@grf53 grf53 requested a review from nuri-yoo May 3, 2026 17:59
@grf53 grf53 linked an issue May 4, 2026 that may be closed by this pull request
@grf53 grf53 force-pushed the refactor-helpers branch from 53407f3 to 3f0391c Compare May 4, 2026 11:33
@grf53 grf53 requested a review from jhlee525 May 4, 2026 11:35
@grf53 grf53 marked this pull request as ready for review May 4, 2026 11:35
Copy link
Copy Markdown
Collaborator

@nuri-yoo nuri-yoo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested locally, clean. LGTM.

Base automatically changed from feat/agentcard-description to refactoring-applied May 6, 2026 04:36
grf53 and others added 4 commits May 6, 2026 15:46
Stop reading OPENAI_API_KEY directly from inside helper wrappers
(`get_title`, `get_purpose`, `get_description`). All LLM helpers now
delegate to ailoy's process-global default provider via `Agent::try_new`,
which the application populates once at boot via `default_provider_mut`.

- main.rs: populate ailoy default at boot, drop local AgentProvider
  threading; build_agent reads from default_provider().
- parser.rs / description.rs: drop dotenvy + env-read; helper structs
  shrink to a single `spec` field with `new()` and `Default`.
- description tests: ignored round-trip test populates the global default
  itself before opening the store.

Verified: cargo test -p speedwagon --lib (56 passed, 2 ignored).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The three helper agents (Title, Purpose, Description) shared an identical
dispatch shell: build query, construct an `Agent`, drain a stream, join
text, parse. Lift that into a `HelperAgent` trait with a default
`generate` body, parameterised by associated types and constants, and
scope all internals to module-private — `Store` is the sole public
entry point for LLM-backed metadata.

- helper.rs: new `HelperAgent` trait (`pub(super)`) — `Input<'a>` (GAT),
  `Output`, `MODEL`, `INSTRUCTION`, `build_query`, `parse`, plus a
  default `async fn generate` that handles all dispatch boilerplate.
- parser.rs: `TitleAgent` and `PurposeAgent` collapse to private unit
  structs with a single trait impl each; only `get_title`/`get_purpose`
  are `pub(super)` (parent calls them via `Store::ingest*`).
- description.rs: `DescriptionAgent` becomes a private unit struct;
  multi-arg input lives in a private `DescriptionInput<'a>`; only
  `get_description` is `pub(super)`.
- mod.rs: drop now-unused `pub use description::{DescriptionAgent,
  fallback_description}` re-export.

Adding a new helper now costs one trait impl block (model + instruction
+ build_query + parse) — no `new`/`generate` boilerplate per helper.

Verified: cargo build --workspace; cargo test -p speedwagon --lib
(56 passed, 2 ignored).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Consolidate the LLM helpers' response handling into the trait so each
helper only declares the LLM-facing pieces (model, instruction,
build_query, fallback) and the dispatch / parsing / fallback plumbing
lives in one place.

Trait changes:
- `parse(raw) -> Output` becomes `process(llm_resp) -> Option<Output>`;
  `None` signals an unusable response and triggers `fallback`.
- New `fallback(input) -> Output` per helper, called by the default
  `generate` body with a `log::warn!` identifying the agent via
  `std::any::type_name`. Drops the per-impl `NAME` const.
- `Input<'a>` no longer needs `Copy`; `build_query` / `fallback` take
  `&Self::Input<'_>`. `DescriptionInput` drops its Copy/Clone derive.
- Default `process` body extracts the `"result"` string field via a
  shared `extract_string_field` helper. The JSON contract
  `{"result": "<string>"}` is the hidden contract — drops the per-impl
  `RESPONSE_FIELD` const, the `{FIELD}` template placeholder, and the
  per-helper `parse_*_response` functions.

Helper changes:
- `TitleAgent` instruction now requires a JSON `{"result": ...}` body
  (matches `PurposeAgent` / `DescriptionAgent`); empty / malformed
  responses route through the same wrapper-level fallback.
- `parse_title` renamed to `extract_title` to disambiguate the
  document-side fast path from LLM-response parsing.

Tests: per-agent `parse_*_response_*` and `*_instruction_*` checks
fold into a single `extract_string_field` test set in `helper.rs`
(8 cases).

Verified: cargo build --workspace; cargo test -p speedwagon --lib
(52 passed, 2 ignored).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ck when none registered

`HelperAgent` previously hardcoded a single OpenAI model id. If
`OPENAI_API_KEY` was missing — or the only registered provider was
Anthropic / Gemini — every helper failed at `Agent::try_new` and the
ingest / describe pipeline died with `"No provider found for model
'openai/gpt-5.4-mini'"`.

Replace the single `MODEL` const with a preference-ordered slice
`MODELS`. The default `generate` body asks ailoy's process-global
default provider (`AgentProvider::get_model`, which honors registered
glob patterns like `openai/*`, `anthropic/claude-*`, `google/gemini-*`)
which of the listed model ids has a backing provider, and uses the
first match. If none are registered, the helper logs a warning and
runs `Self::fallback` — same path used for empty/malformed responses.

`MODELS` defaults to `DEFAULT_HELPER_MODELS` (a private const in
`helper.rs`) so existing helpers don't need to declare anything.
The default list, in order: OpenAI gpt-5.4-mini → Anthropic Claude
Haiku 4.5 → Google Gemini 2.5 Flash. Override only if a helper needs
a different preference order.

Tests: new `generate_falls_back_when_no_provider_registered`
exercises the no-provider path with a synthetic helper whose model id
matches no glob, so it's robust against whatever any other test
registered on the global default.

Verified: cargo build --workspace; cargo test -p speedwagon --lib
(53 passed, 2 ignored).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@grf53 grf53 force-pushed the refactor-helpers branch from 3f0391c to 2b0f588 Compare May 6, 2026 07:00
@grf53 grf53 merged commit d699aae into refactoring-applied May 6, 2026
@grf53 grf53 deleted the refactor-helpers branch May 6, 2026 07:04
khj809 added a commit that referenced this pull request May 11, 2026
* refactor: rename knowledge-agent crate to speedwagon and restructure modules

Renames the crate and reorganizes source into store (indexer, parser, searcher, translator) and tool modules.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* add functions

* update tools

* add global tool function

* add cli & tests

* add get_many & ingest_many functions

* update internal features

* apply latest ailoy, initialize backend v2

* update ailoy the one ToolFactory is Send + Sync

* find: line-based matching with bare-word fallback and query syntax (#40)

Match line-by-line internally and report byte offsets externally.
Bare-word queries get progressive AND -> HALF -> OR fallback; the
level used is surfaced in the response so callers can tell strict
matches from relaxed ones. Replace the single case-insensitive
regex query with a small structured-query parser supporting
"phrase", +term/-term, AND/OR/NOT, (group), and /regex/. Any
explicit operator opts out of fallback so caller intent is
preserved.

Co-authored-by: nuri-yoo <nuri-yoo@users.noreply.github.com>

* Add purpose metadata field to Store ingest (#41)

Add LLM-generated `purpose` as a third indexed field alongside `title`
and `content`. The prompt and preview strategy match the v1 variant
from the metadata ablation report (first 3000 chars, "what queries
should find this document?" framing), which improved BM25 hit@5 by
+15.3pp on FinanceBench over the title-only baseline.

- `Document` gains a `purpose: String` field.
- `parser::get_purpose()` calls a new `PurposeAgent` (gpt-5.4-mini,
  JSON response parsed via `serde_json`). Empty/invalid responses
  fall back to an empty string with a warn log so ingest still
  succeeds; transport-level errors propagate.
- Tantivy schema adds `purpose` as `TEXT|STORED`. `open_or_create`
  detects schema mismatch on existing indexes and rebuilds the index
  directory automatically (corpus is preserved, so re-ingest only
  pays the LLM cost).
- `Store::ingest` and `Store::ingest_many` invoke `get_title` and
  `get_purpose` in parallel via `tokio::try_join!`.
- `QueryParser` default fields are extended to `[title, purpose,
  content]` so purpose terms participate in BM25 scoring.

Closes #37

Co-authored-by: nuri-yoo <nuri-yoo@users.noreply.github.com>

* add basic apis to test sandbox and messaging

* add keep-alive on SSE, factor out build_agent

* add GET/DELETE messages API

* use latest of ailoy PR#391

* update ailoy

* Add HTML support for Store (#46)

* feat: add HTML file type support for `Store`

* refactor(translator): split translator into files based on FileType

* refactor(speedwagon): centralize filetype mapping and translator dispatch

* feat(speedwagon-cli): apply shared filetype mapping to ingest

* fix: apply chrome-stripping universally instead of site-specific branch

* fix: use single-quoted string to avoid complex escapes

* fix: adjust condition to prepend title and

* fix: add more chrome types

* refactor: use `html_to_markdown_rs`, not `dom_smoothie` and `dom_query`

* feat(speedwagon): add DescriptionAgent for KB-level description generation (#47)

* feat(speedwagon): add DescriptionAgent for KB-level description generation

* refactor(speedwagon): self-anchor description prompt and trim doc-comments

- Drop the "alongside other KBs" framing from the prompt opening so the model
  isn't primed to emit comparison vocabulary; the existing "do not compare"
  clause now matches the framing.
- Note Korean output's ~1/3 char density at the same budget; per-language
  budgets are deferred (LLM can't count Korean words reliably either).
- Trim verbose doc-comments across description.rs and Store::describe; add
  cost note (~24K input chars at N=200) to discourage synchronous use on
  indexing hot paths.

Verified against the 4-KB / 12-probe harness: routing accuracy stays 12/12
across the shipped baseline, the prompt-only change, and word-budget
variants. cargo test -p speedwagon --lib description: 12 passed, 1 ignored.

* feat(speedwagon): force description output to English

* refactor(speedwagon): borrow doc slices in description path

Switch `&[(String, String)]` / `&[String]` to `&[(&str, &str)]` /
`&[&str]` in `generate`, `get_description`, `build_user_message`, and
`fallback_description`, and drop the upfront title/purpose clone in
`Store::describe`. Caller-side `Document` strings are borrowed directly,
and the fallback title vec is built only on the empty-LLM-response
branch.

---------

Co-authored-by: nuri-yoo <nuri-yoo@users.noreply.github.com>

* Integrating helper agents into a common interface (#50)

* refactor(speedwagon): use ailoy default_provider for LLM helpers

* refactor(speedwagon): consolidate LLM helpers under HelperAgent trait

* refactor(speedwagon): tighten HelperAgent contract and response handling

* refactor(speedwagon): pick helper model from preference list, fall back when none registered

* Test speedwagon tool (#48)

* UPDATE : create session with speedwagon

* ADD : Session message & e2e rough test

Co-authored-by: Copilot <copilot@github.com>

* chore : test enhance & making agent change graceful

Co-authored-by: Copilot <copilot@github.com>

* UPDATE : instructions of swcard/main-agent

* ADD : commented build_agent with direct speedwagon tool

Co-authored-by: Copilot <copilot@github.com>

* remove : duplicated dep in dev

* update ailoy

* align with latest ailoy develop

* refactor: remove dead code and deduplicate e2e test helpers

- Remove unused `into_runtime` and `into_runtime_with_provider` from
  SpeedwagonSpec (never called anywhere)
- Migrate e2e_test.rs to use shared `common` module helpers instead of
  local `json_request` and `extract_assistant_text` duplicates
- Fix `.sandbox()` → `.runenv()` in commented-out alternative build_agent

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

---------

Co-authored-by: Copilot <copilot@github.com>
Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
Co-authored-by: khj809 <onsealeatang@gmail.com>

* chore(deps): bump ailoy to post-#391 (AgentBuilder + sandbox sharing) (#51)

Bumps ailoy from c42231e1 (2026-04-21) to 098a8289 (PR #391, 2026-05-04).
This pulls in 27 commits worth of breaking changes; this PR only patches
speedwagon to compile and pass tests against the new API. AgentBuilder
itself is not adopted yet — that's PR #52.

Breaking changes absorbed
- ailoy#389: ToolFactory introduced; tool sources now live on
  ToolProvider.custom(ToolFactory::simple(desc, func)) rather than
  ToolSet.insert(name, desc, func).
- ailoy#390: RunEnv trait + sandbox feature split. AgentState carries
  an Arc<dyn RunEnv>, defaulting to Local. No direct touch in this PR;
  speedwagon never constructed sandboxes.
- ailoy#391: Provider registry overhaul. AgentProvider.models is now a
  LangModelProvider with .insert(pattern, LangModelProviderElem::API{..})
  / .get(name) glob-matching. The old default_provider_mut().model_openai()
  / model_claude() / model_gemini() helper constructors were removed,
  along with provider.get_model(...). Agent::try_with_tools(spec, provider,
  toolset) is gone — tools must live on provider.tools.

Speedwagon changes
- tool/mod.rs: build_toolset(store) -> ToolSet renamed to
  build_tool_provider(store) -> ToolProvider, using ToolFactory::simple.
- main.rs::build_agent: no separate toolset arg; clones the global
  provider, overrides provider.tools with the store-bound tools,
  Agent::try_with_provider(spec, &provider).
- store/helper.rs:88: provider.get_model(m) -> provider.models.get(m).
  Same semantics, new accessor location.
- New module speedwagon::provider with register_provider_from_env that
  reads OPENAI_API_KEY / ANTHROPIC_API_KEY / GEMINI_API_KEY and registers
  the same glob patterns the removed ailoy helpers used
  (openai/*, anthropic/claude-*, google/gemini-*). main.rs and the
  description.rs integration test both call this — keeps the env-key →
  glob-pattern mapping in one place, preserving PR #49's invariant that
  helper modules never read env directly.

Out of scope
- chat-agent / backend / knowledge-agent: these were already broken on
  the refactoring-applied baseline (workspace member vs. standalone
  ambiguity). They will be brought back in a separate PR alongside the
  AgentBuilder migration (planned PR #52).

Verification
  cargo check  -p speedwagon --tests --all-features  # clean
  cargo test   -p speedwagon --lib   --all-features  # 71 passed; 0 failed; 2 ignored

Co-authored-by: nuri-yoo <nuri-yoo@users.noreply.github.com>

* feat: add auth/user APIs (#54)

Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>

* ADD: batch document ingest + bulk purge with partial success (#57)

* feat: batch document ingest + bulk purge with partial success

- Add `Store::ingest_many` partial success (IngestResult/IngestFailure)
  with batch index optimization and best-effort cleanup on failure
- Add `Store::purge_many` (PurgeResult/PurgeFailure)
- POST /documents: multi-file multipart upload with per-file validation
- DELETE /documents: bulk purge via JSON body { ids: [...] }
- GET /documents/{id}: single document retrieval
- Response DTOs: BatchIngestResponse, BatchPurgeResponse, FailedItem
- 14 new document tests + e2e test rewritten for multi-doc HTTP flow

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* Preserve batch-ingest failure evidence while reducing helper duplication

Keep the document batch API behavior unchanged while removing a silent fallback in failure indexing and trimming duplicated test HTTP setup.

Constraint: Cleanup is scoped to PR #57 / a69222c document-batch changes only.\nRejected: Rewriting broader Speedwagon parser/tool clippy warnings | outside the requested commit scope.\nConfidence: high\nScope-risk: narrow\nDirective: Keep ingest_many response semantics provisional until the Store contract is hardened.\nTested: cargo fmt --check -p agent-k-backend -p speedwagon; cargo check -p agent-k-backend; cargo test -p agent-k-backend --test document_test; cargo test -p speedwagon --no-default-features --lib; cargo clippy -p agent-k-backend --tests\nNot-tested: live ignored e2e RAG test requiring OPENAI_API_KEY

* use aide::axum::routing

* Keep document batch failures item-scoped

Review feedback showed batch ingest and purge paths could hide item-level failures or over-clean existing artifacts. This keeps API ids string-shaped at the boundary while parsing per item before store operations.

Constraint: PR #57 review requested String id consistency and explicit multipart/corpus failure handling
Rejected: Converting speedwagon document ids to Uuid | would broaden index/tool/CLI scope beyond PR
Confidence: high
Scope-risk: narrow
Directive: Keep speedwagon index/tool IDs string-shaped unless a broader migration is planned
Tested: cargo fmt --check -p speedwagon -p agent-k-backend; cargo test -p speedwagon --lib; cargo test -p agent-k-backend --test document_test; cargo check -p speedwagon -p agent-k-backend; git diff --check
Not-tested: clippy -D warnings; blocked by preexisting warnings outside this change

---------

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
Co-authored-by: khj809 <onsealeatang@gmail.com>

* update ailoy

* feat: add Project workspace and session sharing model (#62)

Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>

---------

Co-authored-by: jhlee525 <bmrcreative90@gmail.com>
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-authored-by: nuri <yoonuri1@gmail.com>
Co-authored-by: nuri-yoo <nuri-yoo@users.noreply.github.com>
Co-authored-by: Park Woorak <wrpark@brekkylab.com>
Co-authored-by: JaehunLee <ljhh0611@gmail.com>
Co-authored-by: Copilot <copilot@github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Refector methodology to create and use 'specific task-dedicated agents'

3 participants