Skip to content

docs: compound learnings from v1.3.0 and GP reranking#10

Open
sammyjoyce wants to merge 1 commit into
RESMP-DEV:mainfrom
sammyjoyce:main
Open

docs: compound learnings from v1.3.0 and GP reranking#10
sammyjoyce wants to merge 1 commit into
RESMP-DEV:mainfrom
sammyjoyce:main

Conversation

@sammyjoyce
Copy link
Copy Markdown

Compounds architectural learnings from recent work into AGENTS.md:

  • 429 handling reversal: Documents why dispatch returns Err(RateLimited) instead of pass-through — coordinator must see all outcomes for EWMA/GP tracking
  • GP reranker pinned-prefix pattern: When direct-routing pins move tiers to front, GP must not reorder them
  • AppState field additions: Every build_app() in tests must be updated when AppState grows
  • External path dependency: gp-routing crate requires sibling checkout
  • Updated request flow diagram, key files table, and error handling docs

Previous compounding threads (T-019d77e8, T-019d777b) could not push to origin; this PR consolidates their learnings plus new GP reranking patterns.

…to AGENTS.md

Captures architectural decisions and lessons from recent work:
- 429 handling: coordinator must see all outcomes (dispatch returns Err,
  not Ok with 429 status) so EWMA and GP can track failures
- GP reranker pinned-prefix pattern for direct-routing compatibility
- AppState field additions require updating all test build_app() helpers
- External gp-routing path dependency requires sibling checkout
- Updated request flow, key files table, and error handling docs

Amp-Thread-ID: https://ampcode.com/threads/T-019d7c98-ce13-7515-a44f-9df4478428ca
Co-authored-by: Amp <amp@ampcode.com>
Copilot AI review requested due to automatic review settings April 11, 2026 12:55
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 5e9f8dfb40

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread AGENTS.md
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Updates AGENTS.md to consolidate recent architectural learnings around tier ordering (EWMA + direct-routing pins), optional GP-based reranking, and rate-limit handling, plus a few repo maintenance/testing notes.

Changes:

  • Updates the request-flow diagram to reflect pinned-prefix handling and optional GP reranking.
  • Extends the “key files” table to include src/gp_router.rs.
  • Expands error-handling + “Lessons Learned” documentation (429 handling rationale, pinned-prefix pattern, AppState test helper updates, and gp-routing path dependency).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread AGENTS.md
Comment thread AGENTS.md
@Nottlespike
Copy link
Copy Markdown
Contributor

Thanks again for the PR — the learnings you pulled in here are useful.

One bit of context for ccr-rust: AGENTS.md is mainly an onboarding / setup doc for contributors who may be newer to Rust, so we try to optimize for quick scanning over detailed narrative.

With that in mind, I’d like to keep the accurate reusable notes here, but compact them:

  • keep the EWMA/direct-routing/search-pin + optional GP reranker notes
  • keep the src/gp_router.rs, pinned_prefix_len, AppState test-helper, and gp-routing sibling-path notes
  • please compress the 429 discussion to a single short, code-accurate summary, e.g.:
    • 429s are tracked as failures; they stop retries for the current tier, routing may continue to later eligible tiers, and a synthetic 429 is returned only when all candidate tiers are rate-limited.

I’d drop the separate 429 subsection and fold that behavior into the shorter wording above. That keeps AGENTS.md easier for new contributors to scan while still preserving the important routing behavior.

I’ve also resolved the bot threads in favor of this higher-level rewrite request rather than taking the line-by-line suggestions literally.

Thanks again — the underlying lessons are useful; I’d just like the final doc version to be tighter and more onboarding-oriented.

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.

3 participants