Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 38 additions & 1 deletion internal/adminapi/handlers.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ import (
"telegram-agent/internal/llm"
)

const maxVisibleDescriptionEnrich = 8

// --- View data ---

type uiRole struct {
Expand Down Expand Up @@ -474,6 +476,7 @@ func (s *Server) buildIndexData(r *http.Request) indexData {
attachMarketSignals(models, marketSignals)
attachModelTelemetry(models, s.loadModelTelemetry(ctx5, catalogProv))
attachModelChecks(models, catalogProv, s.loadModelChecks(ctx5))
s.enrichVisibleDescriptions(ctx5, catalogProv, models)
decorateModelDisplay(models, preset)
sections := buildModelSections(models, true)
filters := uiFilters{
Expand Down Expand Up @@ -591,7 +594,6 @@ func (s *Server) buildIndexData(r *http.Request) indexData {
attachMarketSignals(models, marketSignals)
attachModelTelemetry(models, s.loadModelTelemetry(ctx5, catalogProv))
attachModelChecks(models, catalogProv, s.loadModelChecks(ctx5))
decorateModelDisplay(models, "")
asc := sortDir == "asc"
sort.Slice(models, func(i, j int) bool {
var less bool
Expand Down Expand Up @@ -633,6 +635,8 @@ func (s *Server) buildIndexData(r *http.Request) indexData {
}
return !less
})
s.enrichVisibleDescriptions(ctx5, catalogProv, models)
decorateModelDisplay(models, "")

return indexData{
Routing: s.buildRouting(),
Expand All @@ -643,6 +647,39 @@ func (s *Server) buildIndexData(r *http.Request) indexData {
}
}

func (s *Server) enrichVisibleDescriptions(ctx context.Context, provider string, models []uiModel) {
if provider != "openrouter" || s.capStore == nil {
return
}
remaining := maxVisibleDescriptionEnrich
for i := range models {
if remaining <= 0 {
return
}
if !llm.OpenRouterDescriptionLooksTruncated(models[i].Description) {
continue
}
remaining--
full, err := llm.FetchOpenRouterModelDescription(ctx, models[i].ID, models[i].Description)
if err != nil {
s.logger.Debug("openrouter description enrichment failed", "model", models[i].ID, "err", err)
continue
}
if len(full) <= len(models[i].Description) {
continue
}
models[i].Description = full
caps := s.lookupCapsFor(ctx, provider, models[i].ID)
caps.Description = full
if caps.Name == "" {
caps.Name = models[i].Name
}
if err := s.capStore.PutCapabilities(ctx, provider, models[i].ID, caps); err != nil {
Comment on lines +672 to +677

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Prevent cache row clobbering when capability lookup misses.

At Line 672, lookupCapsFor can return a zero-value struct on miss/error; then Line 677 upserts that struct, which can overwrite persisted capability fields (pricing, flags, context length) with zeros.

Proposed fix
-		caps := s.lookupCapsFor(ctx, provider, models[i].ID)
+		caps, found, err := s.capStore.GetCapabilities(ctx, provider, models[i].ID)
+		if err != nil {
+			s.logger.Debug("openrouter description cache lookup failed", "model", models[i].ID, "err", err)
+			continue
+		}
+		if !found {
+			continue
+		}
 		caps.Description = full
 		if caps.Name == "" {
 			caps.Name = models[i].Name
 		}
 		if err := s.capStore.PutCapabilities(ctx, provider, models[i].ID, caps); err != nil {
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@internal/adminapi/handlers.go` around lines 672 - 677, The lookupCapsFor
function can return a zero-value struct when a lookup misses or encounters an
error, and the current code proceeds to modify and persist this empty struct via
PutCapabilities, which overwrites existing capability data (such as pricing,
flags, and context length) with zeros. After the lookupCapsFor call on line 672,
add a check to validate that the returned capabilities struct is valid and not a
zero-value before proceeding with the modification and PutCapabilities call on
line 677. If the lookup fails or misses, skip the persistence operation entirely
to prevent data loss.

s.logger.Debug("openrouter description cache update failed", "model", models[i].ID, "err", err)
}
}
}

// orBlendedPrice mirrors AA's 3:1 input/output weighting so prices are
// directly comparable across sources.
func orBlendedPrice(promptPrice, completionPrice float64) float64 {
Expand Down
88 changes: 87 additions & 1 deletion internal/llm/capabilities.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,11 @@ import (
"context"
"encoding/json"
"fmt"
"html"
"io"
"net/http"
"net/url"
"regexp"
"strconv"
"strings"
"time"
Expand Down Expand Up @@ -62,7 +65,11 @@ type ConfigurableProvider interface {

// --- OpenRouter /models fetcher ---

var openRouterHTTPClient = &http.Client{Timeout: 20 * time.Second}
var (
openRouterHTTPClient = &http.Client{Timeout: 20 * time.Second}
openRouterPageBaseURL = "https://openrouter.ai"
openRouterDescriptionPayload = regexp.MustCompile(`\\"description\\":\\"((?:\\\\.|[^\\"])*)\\"`)
)

// FetchOpenRouterModels pulls the full catalog from OpenRouter and returns a
// map keyed by model id. Prices are normalised to USD per 1M tokens (OpenRouter
Expand Down Expand Up @@ -133,6 +140,85 @@ func parseOpenRouterModels(body []byte) (map[string]Capabilities, error) {
return out, nil
}

// OpenRouterDescriptionLooksTruncated reports descriptions that already arrive
// from OpenRouter with an ellipsis. Those are upstream-shortened strings, not a
// UI clamp, so callers may optionally enrich them from the model page payload.
func OpenRouterDescriptionLooksTruncated(desc string) bool {
desc = strings.TrimSpace(desc)
return strings.HasSuffix(desc, "...") || strings.HasSuffix(desc, "…")
}

// FetchOpenRouterModelDescription fetches a model page and extracts the longest
// matching description from its SSR payload. OpenRouter's public /models API can
// return shortened descriptions ending in "...", while the model page payload
// usually carries the full text.
func FetchOpenRouterModelDescription(ctx context.Context, modelID, currentDescription string) (string, error) {
req, err := http.NewRequestWithContext(ctx, http.MethodGet, openRouterModelPageURL(modelID), nil)
if err != nil {
return "", err
}
resp, err := openRouterHTTPClient.Do(req)
if err != nil {
return "", fmt.Errorf("openrouter model page: %w", err)
}
defer resp.Body.Close()

body, err := io.ReadAll(io.LimitReader(resp.Body, 4*1024*1024))
if err != nil {
return "", fmt.Errorf("openrouter model page: read: %w", err)
Comment on lines +160 to +168

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

# Read the file around the specified lines
cat -n internal/llm/capabilities.go | sed -n '155,175p'

Repository: Dzarlax-AI/personal_assistant

Length of output: 1061


🏁 Script executed:

# Check for other similar patterns in the file to understand error handling patterns
rg -n "defer.*Close\(\)" internal/llm/capabilities.go -A 5 | head -40

Repository: Dzarlax-AI/personal_assistant

Length of output: 467


🏁 Script executed:

# Search for proper error handling of resp.Body.Close() in the codebase
rg -n "closeErr\s*:=\s*resp\.Body\.Close\(\)" --type go -A 2

Repository: Dzarlax-AI/personal_assistant

Length of output: 55


🏁 Script executed:

# Search for patterns where Close() errors are explicitly handled
rg "\.Close\(\)" --type go -A 2 | grep -E "(closeErr|errClose|err.*Close)" | head -20

Repository: Dzarlax-AI/personal_assistant

Length of output: 55


🏁 Script executed:

# Look for other HTTP response body reading patterns in the codebase
rg "io\.ReadAll.*resp\.Body" --type go -B 2 -A 3 | head -60

Repository: Dzarlax-AI/personal_assistant

Length of output: 3010


🏁 Script executed:

# Check if there's any linter configuration for errcheck in the repo
find . -name ".golangci.yml" -o -name ".golangci.yaml" -o -name "golangci.yml" | head -5

Repository: Dzarlax-AI/personal_assistant

Length of output: 55


🏁 Script executed:

# Look for linting configuration
fd -e "golangci*" -o -e "*.lint*"

Repository: Dzarlax-AI/personal_assistant

Length of output: 190


🏁 Script executed:

# Look for linting configuration files
find . -maxdepth 2 -type f \( -name "*.golangci*" -o -name ".lint*" \) 2>/dev/null

Repository: Dzarlax-AI/personal_assistant

Length of output: 55


🏁 Script executed:

# Check the exact proposed fix in the review - verify the error message issue
# Let me see if there are any other close error patterns in the repo
rg "resp\.Body" --type go -A 5 | grep -E "(Close|err)" | head -30

Repository: Dzarlax-AI/personal_assistant

Length of output: 2158


🏁 Script executed:

# Check if there's any pattern of handling close errors elsewhere in Go files
rg "defer.*Close" --type go -B 2 -A 8 | grep -E "(defer|closeErr|Close|\.Close)" | head -40

Repository: Dzarlax-AI/personal_assistant

Length of output: 2000


Handle resp.Body.Close() errors in the new fetch path.

Line 164 currently ignores close errors via defer, which can mask transport/resource issues. The proposed fix has a variable name mismatch—the error message still references err instead of readErr.

Proposed fix
-	defer resp.Body.Close()
-
-	body, err := io.ReadAll(io.LimitReader(resp.Body, 4*1024*1024))
-	if err != nil {
-		return "", fmt.Errorf("openrouter model page: read: %w", err)
+	body, readErr := io.ReadAll(io.LimitReader(resp.Body, 4*1024*1024))
+	closeErr := resp.Body.Close()
+	if readErr != nil {
+		return "", fmt.Errorf("openrouter model page: read: %w", readErr)
+	}
+	if closeErr != nil {
+		return "", fmt.Errorf("openrouter model page: close: %w", closeErr)
 	}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
resp, err := openRouterHTTPClient.Do(req)
if err != nil {
return "", fmt.Errorf("openrouter model page: %w", err)
}
defer resp.Body.Close()
body, err := io.ReadAll(io.LimitReader(resp.Body, 4*1024*1024))
if err != nil {
return "", fmt.Errorf("openrouter model page: read: %w", err)
resp, err := openRouterHTTPClient.Do(req)
if err != nil {
return "", fmt.Errorf("openrouter model page: %w", err)
}
body, readErr := io.ReadAll(io.LimitReader(resp.Body, 4*1024*1024))
closeErr := resp.Body.Close()
if readErr != nil {
return "", fmt.Errorf("openrouter model page: read: %w", readErr)
}
if closeErr != nil {
return "", fmt.Errorf("openrouter model page: close: %w", closeErr)
}
🧰 Tools
🪛 golangci-lint (2.12.2)

[error] 164-164: Error return value of resp.Body.Close is not checked

(errcheck)

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@internal/llm/capabilities.go` around lines 160 - 168, Replace the simple
defer statement for resp.Body.Close() with explicit error handling that captures
and checks for close errors, rather than silently ignoring them. Additionally,
ensure the error variable used in the fmt.Errorf call for the io.ReadAll
operation matches the actual variable name being assigned (if you're using
readErr for the read operation error, update the error message to reference
readErr instead of err). This applies to the HTTP request handling block where
openRouterHTTPClient.Do(req) is called and resp.Body is read with
io.LimitReader.

Source: Linters/SAST tools

}
if resp.StatusCode != http.StatusOK {
return "", fmt.Errorf("openrouter model page: HTTP %d: %s", resp.StatusCode, string(body))
}
full := extractOpenRouterPageDescription(body, currentDescription)
if full == "" {
return "", fmt.Errorf("openrouter model page: full description not found")
}
return full, nil
}

func openRouterModelPageURL(modelID string) string {
parts := strings.Split(modelID, "/")
for i := range parts {
parts[i] = url.PathEscape(parts[i])
}
return strings.TrimRight(openRouterPageBaseURL, "/") + "/" + strings.Join(parts, "/")
}

func extractOpenRouterPageDescription(body []byte, currentDescription string) string {
current := normalizeDescriptionText(currentDescription)
prefix := strings.TrimSpace(strings.TrimSuffix(strings.TrimSuffix(current, "..."), "…"))
if len(prefix) > 120 {
prefix = prefix[:120]
}
Comment on lines +191 to +193

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

cd /dev/null || true
# Search for the file and examine the context
fd -t f capabilities.go internal/llm

Repository: Dzarlax-AI/personal_assistant

Length of output: 170


🏁 Script executed:

# Read the relevant section around lines 191-193
cat -n internal/llm/capabilities.go | sed -n '170,210p'

Repository: Dzarlax-AI/personal_assistant

Length of output: 1706


🏁 Script executed:

# Get more context to understand what prefix is and how it's used
cat -n internal/llm/capabilities.go | sed -n '145,220p'

Repository: Dzarlax-AI/personal_assistant

Length of output: 3206


Avoid byte-slicing UTF-8 when capping the prefix.

At line 192, prefix = prefix[:120] slices at the byte level, not the rune level. For any multi-byte UTF-8 characters in the description, this can split a rune and cause the subsequent strings.HasPrefix comparison at line 208 to fail, preventing valid description enrichment.

Proposed fix
	if len(prefix) > 120 {
-		prefix = prefix[:120]
+		r := []rune(prefix)
+		prefix = string(r[:120])
	}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if len(prefix) > 120 {
prefix = prefix[:120]
}
if len(prefix) > 120 {
r := []rune(prefix)
prefix = string(r[:120])
}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@internal/llm/capabilities.go` around lines 191 - 193, The prefix variable is
being sliced at the byte level with prefix[:120], which can split multi-byte
UTF-8 characters and cause subsequent strings.HasPrefix comparisons to fail.
Convert the prefix string to runes using []rune(), slice to 120 runes instead of
120 bytes, then convert back to a string using string() to ensure proper UTF-8
character boundaries are respected.

var best string
for _, m := range openRouterDescriptionPayload.FindAllSubmatch(body, -1) {
if len(m) != 2 {
continue
}
raw := string(m[1])
decoded, err := strconv.Unquote(`"` + raw + `"`)
if err != nil {
continue
}
decoded = normalizeDescriptionText(html.UnescapeString(decoded))
if decoded == "" || len(decoded) <= len(current) || OpenRouterDescriptionLooksTruncated(decoded) {
continue
}
if prefix != "" && !strings.HasPrefix(decoded, prefix) {
continue
}
if len(decoded) > len(best) {
best = decoded
}
}
return best
}

func normalizeDescriptionText(s string) string {
return strings.Join(strings.Fields(strings.TrimSpace(s)), " ")
}

func containsStr(s []string, target string) bool {
for _, v := range s {
if strings.EqualFold(v, target) {
Expand Down
23 changes: 23 additions & 0 deletions internal/llm/capabilities_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -128,3 +128,26 @@ func TestCapabilitiesFree(t *testing.T) {
})
}
}

func TestExtractOpenRouterPageDescriptionUsesFullPayload(t *testing.T) {
current := "NVIDIA Nemotron 3 Nano Omni is a 30B-A3B open multimodal model designed to function as a perception and context sub-agent in enterprise agent systems. It accepts text, image, video, and..."
full := "NVIDIA Nemotron 3 Nano Omni is a 30B-A3B open multimodal model designed to function as a perception and context sub-agent in enterprise agent systems. It accepts text, image, video, and audio inputs and produces text output, enabling agents to perceive and reason across modalities in a single inference loop."
body := []byte(`prefix {\"description\":\"Short page meta.\"} middle {\"description\":\"` + full + `\"} suffix`)

got := extractOpenRouterPageDescription(body, current)
if got != full {
t.Fatalf("description mismatch:\nwant %q\ngot %q", full, got)
}
}

func TestOpenRouterDescriptionLooksTruncated(t *testing.T) {
if !OpenRouterDescriptionLooksTruncated("accepts text, image, video, and...") {
t.Fatal("ASCII ellipsis should be treated as truncated")
}
if !OpenRouterDescriptionLooksTruncated("accepts text, image, video, and…") {
t.Fatal("unicode ellipsis should be treated as truncated")
}
if OpenRouterDescriptionLooksTruncated("Full sentence.") {
t.Fatal("complete sentence should not be treated as truncated")
}
}
Loading