Skip to content

chore: post-PR-79 refinements for Vertex AI and curation parser #84

Description

@yvonnedevlinrh

Context

PR #79 fixed three production bugs (#76, #77, #78) in Vertex AI curation reliability. During review, several non-blocking quality refinements were identified that should be addressed in a follow-up.

Tasks

1. Extract Vertex AI magic numbers to named constants

File: llm/vertex.go

Timeout: 300 * time.Second (line 85) and MaxTokens: 16000 (line 119) are inline literals. The existing codebase defines retry-related values as named constants (vertexSynthMaxRetries, vertexSynthBaseDelay, vertexSynthMaxDelay). These two should follow the same pattern:

const (
    // vertexSynthTimeout is the HTTP client timeout for Vertex AI requests.
    // Large curation prompts (36K+ tokens) need ~120-180s for response generation.
    vertexSynthTimeout   = 300 * time.Second
    // vertexSynthMaxTokens is the maximum output tokens for Vertex AI responses.
    // Curation extractions routinely exceed 4K tokens; 16K provides headroom.
    vertexSynthMaxTokens = 16000
)

Convention: CS-007 (magic numbers should be named constants)

2. Update ParseExtractionResponse doc comment

File: curate/curate.go, lines 319-321
The doc comment doesn't mention that individual unparseable items are silently skipped. A caller reading only the doc would expect any parse failure to return an error. Add a line:
// Silently skips individual items that cannot be parsed after fallback attempts.
Convention: DR-001 (GoDoc accuracy on exported functions)

3. Strengthen regression test assertions

File: curate/curate_test.go, TestParseExtractionResponse_QualityFlagsString (line 648)

The test verifies item count and QualityFlags length but doesn't assert that other fields survived the delete + json.Marshal + json.Unmarshal round-trip. Add:

if files[0].Content != "Use OAuth2." {
    t.Errorf("files[0].Content = %q, want %q", files[0].Content, "Use OAuth2.")
}
if files[0].Confidence != "high" {
    t.Errorf("files[0].Confidence = %q, want %q", files[0].Confidence, "high")
}

Convention: TC-006 (regression test completeness)

Out of scope (informational, no action needed)

  • Fallback scope: The fallback path deletes quality_flags on any unmarshal failure, not just quality_flags-specific failures. This is intentionally lenient for LLM output parsing — items that can't be fixed by removing quality_flags are correctly skipped via continue. No change needed.
  • Unreachable error check: json.Marshal(fields) after a successful json.Unmarshal into the same type cannot fail. The defensive guard is harmless. No change needed.
  • Global MaxTokens: MaxTokens: 16000 applies to both curation and compile callers. Compile outputs are typically smaller but the higher limit has no functional impact. No change needed unless per-caller configuration is added later.

####Scope

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions