Feat/add middle out context strategy#141
Conversation
gusye1234
left a comment
gusye1234
left a comment
There was a problem hiding this comment.
Nice work!
You can move the e2e scripts into one complete script, and place it in src/client/acontext-py/examples or src/client/acontext-ts/examples
We have CI/CD to run the script in those folder automatically.
| return messages, nil | ||
| } | ||
| ctx := context.Background() | ||
| totalTokens, err := tokenizer.CountMessagePartsTokens(ctx, messages) |
There was a problem hiding this comment.
Maybe we can count the exact token of each message? So in the later section, once we removed some messages, we can just reduce the total tokens.
instead of re-compute the tokens of the rest messages in every loop.
| for i, msg := range messages { | ||
| for _, part := range msg.Parts { | ||
| if part.Type == "tool-result" && part.Meta != nil { | ||
| if id, ok := part.Meta["tool_call_id"].(string); ok && id == toolCallID { | ||
| if _, ok := toRemove[i]; !ok { | ||
| toRemove[i] = struct{}{} | ||
| queue = append(queue, i) | ||
| } | ||
| break | ||
| } | ||
| } | ||
| } | ||
| } | ||
| for i, msg := range messages { | ||
| for _, part := range msg.Parts { | ||
| if part.Type == "tool-call" && part.Meta != nil { | ||
| if id, ok := part.Meta["id"].(string); ok && id == toolCallID { | ||
| if _, ok := toRemove[i]; !ok { | ||
| toRemove[i] = struct{}{} | ||
| queue = append(queue, i) | ||
| } | ||
| break |
There was a problem hiding this comment.
Duplicate logic?? searches for tool-result twice
| } | ||
| } | ||
| for totalTokens > s.TokenReduceTo && len(result) > 0 { | ||
| result = removeWithToolPairing(result, 0) |
There was a problem hiding this comment.
maybe I am wrong but noo validation that first/last messages are not emoved this could break conversation context, no? @gusye1234
|
Thanks @gusye1234 @slyt3 for your reviews. I shall polish these up asap. Excited to see first pr merge 😇 |
riturajFi
force-pushed
the
feat/add_middle_out_context_strategy
branch
from
0e6ad20 to
a70f617
Compare
|
shouldnt it be this PR on dev branch? @gusye1234 |
|
LGTM! Thanks for clarifying the intent in the fallback comment. |
riturajFi
force-pushed
the
feat/add_middle_out_context_strategy
branch
from
ef7c051 to
15f4393
Compare
|
Hi @gusye1234 |
3 participants
Why we need this PR?
Describe your solution
...
Implementation Tasks
Please ensure your pull request meets the following requirements:
Impact Areas
Which part of Acontext would this feature affect?
Checklist
devbranch.