Merge pull request #428 from terraphim/feat/quickwit-haystack-integration

feat: Add Quickwit haystack integration with hybrid index discovery

Author: AlexMikhalev

.docs/design-quickwit-haystack-integration.md

@@ -0,0 +1,312 @@
+# Document Quality Evaluation Report (Revision 2)
+
+## Metadata
+- **Document**: /Users/alex/projects/terraphim/terraphim-ai/.docs/design-quickwit-haystack-integration.md
+- **Type**: Phase 2 Design (Updated with auto-discovery and Basic Auth)
+- **Evaluated**: 2026-01-13
+- **Evaluator**: disciplined-quality-evaluation skill
+- **Revision**: 2 (incorporates user decisions from Q1-Q3)
+
+---
+
+## Decision: **GO** ✅
+
+**Weighted Average Score**: 4.43 / 5.0
+**Simple Average Score**: 4.50 / 5.0
+**Blocking Dimensions**: None
+
+All dimensions meet minimum threshold (≥ 3.0) and weighted average significantly exceeds 3.5. Document approved for Phase 3 implementation.
+
+---
+
+## Dimension Scores
+
+| Dimension | Score | Weight | Weighted | Status |
+|-----------|-------|--------|----------|--------|
+| Syntactic | 4/5 | 1.5x | 6.0 | ✅ Pass |
+| Semantic | 5/5 | 1.0x | 5.0 | ✅ Pass |
+| Pragmatic | 4/5 | 1.5x | 6.0 | ✅ Pass |
+| Social | 5/5 | 1.0x | 5.0 | ✅ Pass |
+| Physical | 5/5 | 1.0x | 5.0 | ✅ Pass |
+| Empirical | 4/5 | 1.0x | 4.0 | ✅ Pass |
+
+*Note: Syntactic and Pragmatic weighted 1.5x for Phase 2 design documents*
+
+---
+
+## Improvements Since First Evaluation
+
+### Major Enhancements
+1. ✅ **QuickwitConfig fully defined** (lines 343-352) - addresses previous critical gap
+2. ✅ **Auto-discovery logic specified** (lines 356-366) - clear pseudocode implementation
+3. ✅ **Basic Auth support added** (Decision 3, lines 182-189) - dual authentication
+4. ✅ **Real try_search configuration incorporated** (lines 1124-1127) - production example
+5. ✅ **Three additional acceptance criteria** (AC-11, AC-12, AC-13) - comprehensive coverage
+6. ✅ **New helper methods specified** (fetch_available_indexes, filter_indexes, search_single_index)
+7. ✅ **Steps expanded to 14** (was 12) - auto-discovery implementation included
+8. ✅ **Hybrid strategy fully documented** (Decision 5, lines 194-207) - trade-offs explicit
+
+### Score Improvements
+- Syntactic: 4/5 (unchanged, but gaps filled with QuickwitConfig)
+- Semantic: 5/5 (improved from 4/5 - real config data, accurate auth patterns)
+- Pragmatic: 4/5 (improved clarity with defined structures)
+- Social: 5/5 (improved from 4/5 - resolved questions, clear decisions)
+
+---
+
+## Detailed Findings
+
+### 1. Syntactic Quality (4/5) ✅ [CRITICAL - Weighted 1.5x]
+
+**Strengths:**
+- **QuickwitConfig fully defined** (lines 343-352) with all 8 fields and types - MAJOR IMPROVEMENT
+- All 8 required Phase 2 sections present
+- Auto-discovery branching logic clearly specified (lines 356-366)
+- 14 acceptance criteria consistently numbered and mapped to tests
+- Implementation sequence renumbered to 14 steps (accounting for 4a, 4b sub-steps)
+- Resolved questions marked with ✅ RESOLVED (lines 984, 996, 1010, 1074)
+- Auth parameters added to config (auth_username, auth_password)
+- Consistent terminology: IndexMiddleware, ServiceType, Haystack
+
+**Weaknesses:**
+- **Line 41:** System Behavior still says "Supports bearer token authentication" but should say "Supports bearer token and basic auth"
+- **Line 254:** `Serialize` imported but never used (only Deserialize needed for response structs)
+- **Lines 293-314:** Helper method signatures still incomplete - missing return types
+  - `parse_config` should be `fn parse_config(&self, haystack: &Haystack) -> Result<QuickwitConfig>`
+  - `filter_indexes` should be `fn filter_indexes(&self, indexes: Vec<QuickwitIndexInfo>, pattern: &str) -> Vec<QuickwitIndexInfo>`
+- **Line 296:** `auth_token: Option<&str>` parameter name doesn't match new dual-auth design - should be more generic or split into two methods
+
+**Suggested Revisions:**
+- [ ] Update line 41: "Supports bearer token and basic authentication"
+- [ ] Remove unused `Serialize` import on line 254
+- [ ] Add complete method signatures:
+  ```rust
+  fn parse_config(&self, haystack: &Haystack) -> Result<QuickwitConfig>
+  async fn fetch_available_indexes(&self, base_url: &str, config: &QuickwitConfig) -> Result<Vec<QuickwitIndexInfo>>
+  fn filter_indexes(&self, indexes: Vec<QuickwitIndexInfo>, pattern: &str) -> Vec<QuickwitIndexInfo>
+  async fn search_single_index(&self, needle: &str, index: &str, base_url: &str, config: &QuickwitConfig) -> Result<Index>
+  fn build_search_url(&self, base_url: &str, index: &str, query: &str, config: &QuickwitConfig) -> String
+  fn hit_to_document(&self, hit: &serde_json::Value, index_name: &str, base_url: &str) -> Option<Document>
+  fn normalize_document_id(&self, index_name: &str, doc_id: &str) -> String
+  fn redact_token(&self, token: &str) -> String
+  ```
+
+---
+
+### 2. Semantic Quality (5/5) ✅
+
+**Strengths:**
+- **Accurate try_search configuration** (lines 1124-1127): URL, Basic Auth, available indexes verified
+- **Correct Basic Auth pattern**: username/password to base64 header (line 187)
+- **Accurate auto-discovery API**: `GET /v1/indexes` → `index_config.index_id` extraction (line 648)
+- **Realistic performance estimates**: ~300ms latency for auto-discovery (line 203)
+- **Correct Rust async patterns**: tokio::join! for concurrent searches (line 694)
+- **Accurate QuickwitConfig structure**: all fields match try_search usage
+- **Proper glob matching logic**: Simple pattern matching appropriate for index filtering
+- All file paths verified against actual codebase structure
+- Correct trait signatures and serde attributes
+
+**Weaknesses:**
+- None - all technical claims are accurate and verifiable
+
+**Suggested Revisions:**
+- None required
+
+---
+
+### 3. Pragmatic Quality (4/5) ✅ [CRITICAL - Weighted 1.5x]
+
+**Strengths:**
+- **QuickwitConfig structure defined** (lines 343-352) - implementers can code directly
+- **Auto-discovery implementation shown** (lines 356-366) - clear branching logic with code
+- **14-step implementation sequence** with sub-steps (4a, 4b) for incremental development
+- **14 acceptance criteria** mapped to specific test locations
+- **12 invariants** mapped to verification methods
+- **Both config examples provided**: explicit mode (lines 520-542) and auto-discovery mode (lines 544-568)
+- **Authentication priority specified**: Check auth_token first, then username/password (line 189)
+- **Each step includes**: Purpose, Files, Actions, Deployable status, Rollback
+
+**Weaknesses:**
+- **Helper method signatures incomplete** (lines 293-314) - implementers must infer types
+- **Line 296**: `fetch_available_indexes` signature shows `auth_token: Option<&str>` but should pass full `QuickwitConfig` for auth flexibility
+- **Line 491:** Import comment still vague: "appropriate modules" - which terraphim_agent structs/traits?
+- **Missing**: How to build Basic Auth header - need `base64` crate? Or use reqwest's built-in basic_auth()?
+- **Line 710**: "Add authentication header if token present" - should clarify "if any auth configured (token OR username/password)"
+
+**Suggested Revisions:**
+- [ ] Add complete method signatures (as listed in Syntactic section)
+- [ ] Update `fetch_available_indexes` signature to accept `&QuickwitConfig` instead of individual params
+- [ ] Specify Basic Auth implementation: "Use reqwest's `.basic_auth(username, Some(password))` method"
+- [ ] Clarify terraphim_agent imports or state "Use terraphim_agent test framework (no specific imports needed)"
+- [ ] Add auth header logic clarification: "If auth_token present, use Bearer; else if auth_username+password present, use Basic; else no auth"
+
+---
+
+### 4. Social Quality (5/5) ✅
+
+**Strengths:**
+- **Resolved questions clearly marked** (✅ RESOLVED) - no ambiguity about status
+- **Design decisions numbered and justified** (Decisions 1-5)
+- **Trade-off analysis referenced** explicitly (line 1006)
+- **User preference documented**: "Option B selected" (line 997)
+- **Both auth methods explained** with priority (lines 1079-1082)
+- **Two config examples** show explicit vs auto-discovery patterns clearly
+- Assumptions marked appropriately for unresolved questions (Q4-Q7)
+- Implementation priority specified: "Check auth_token first"
+
+**Weaknesses:**
+- None - all stakeholders will interpret identically
+
+**Suggested Revisions:**
+- None required
+
+---
+
+### 5. Physical Quality (5/5) ✅
+
+**Strengths:**
+- Exemplary markdown structure with numbered sections 1-8
+- Tables used effectively: File Change Plan, Acceptance Criteria (now 14 rows), Invariants, Risks
+- Two complete config examples (explicit and auto-discovery)
+- ASCII architecture diagram clear (lines 94-121)
+- Code blocks properly formatted with rust syntax
+- QuickwitConfig structure highlighted in "Key Implementation Notes"
+- Checkboxes for Prerequisites and revision items
+- Visual indicators: ✅, ⚠️, ◄─ NEW
+
+**Weaknesses:**
+- None - formatting excellent and enhanced with new examples
+
+**Suggested Revisions:**
+- None required
+
+---
+
+### 6. Empirical Quality (4/5) ✅
+
+**Strengths:**
+- QuickwitConfig definition makes auto-discovery logic immediately comprehensible
+- Auto-discovery pseudocode (lines 356-366) is digestible and clear
+- Information well-chunked into 14 discrete implementation steps
+- Two config examples provide concrete reference points
+- Tables reduce cognitive load
+- Summary section (lines 1105-1129) provides excellent overview
+
+**Weaknesses:**
+- **Section 6 tables** (lines 852-884): 33 rows across two tables - somewhat dense
+- **File 2 structure** (lines 248-338): Long code block with helper list could use more inline explanation
+- **Steps 4, 4a, 4b** (lines 628-668): Three related steps - could be confusing why split vs single Step 4
+
+**Suggested Revisions:**
+- [ ] Add separator text between AC table and Invariant table: "### Invariant Verification Tests" (already present, but could add brief intro)
+- [ ] Consider inline comments in File 2 code explaining each helper's role
+- [ ] Clarify step numbering: Consider renaming 4a/4b to Step 5/Step 6 for clarity (though current is acceptable)
+
+---
+
+## Phase 2 Compliance
+
+All required sections present and enhanced:
+- ✅ Section 1: Summary of Target Behavior (updated with auth modes)
+- ✅ Section 2: Key Invariants and Acceptance Criteria (14 AC, 12 INV - expanded)
+- ✅ Section 3: High-Level Design and Boundaries (5 design decisions)
+- ✅ Section 4: File/Module-Level Change Plan (8 files, detailed specs)
+- ✅ Section 5: Step-by-Step Implementation Sequence (14 steps with sub-steps)
+- ✅ Section 6: Testing & Verification Strategy (comprehensive mapping)
+- ✅ Section 7: Risk & Complexity Review (11 risks assessed)
+- ✅ Section 8: Open Questions (3 resolved, 7 with assumptions)
+
+---
+
+## Revision Checklist
+
+**Priority: HIGH** (Recommended for maximum clarity)
+- [ ] Add complete method signatures for all 8 helper methods
+- [ ] Update line 41: "bearer token and basic auth" (not just bearer)
+- [ ] Specify Basic Auth implementation: "Use reqwest's `.basic_auth()` method"
+
+**Priority: MEDIUM** (Nice to have)
+- [ ] Remove unused `Serialize` import from File 2
+- [ ] Update `fetch_available_indexes` to accept `&QuickwitConfig` for auth flexibility
+- [ ] Add inline comments to File 2 helper method list explaining each purpose
+
+**Priority: LOW** (Optional polish)
+- [ ] Consider renumbering 4a/4b to sequential numbers for clarity
+- [ ] Add brief text before Invariant table separating from AC table
+
+---
+
+## Comparison to First Evaluation
+
+| Aspect | First Eval | Second Eval | Change |
+|--------|-----------|-------------|---------|
+| Weighted Score | 4.14 | 4.43 | +0.29 ⬆️ |
+| Simple Score | 4.17 | 4.50 | +0.33 ⬆️ |
+| Semantic | 4/5 | 5/5 | +1 ⬆️ |
+| Social | 4/5 | 5/5 | +1 ⬆️ |
+| Acceptance Criteria | 10 | 14 | +4 ⬆️ |
+| Implementation Steps | 12 | 14 | +2 ⬆️ |
+| Design Decisions | 4 | 5 | +1 ⬆️ |
+| Resolved Questions | 0 | 3 | +3 ⬆️ |
+
+**Significant Improvements:**
+- QuickwitConfig definition added (critical gap filled)
+- Auto-discovery strategy fully specified
+- Basic Auth support integrated
+- Real production configuration from try_search
+- Three key questions resolved with clear decisions
+
+---
+
+## Quality Assessment Summary
+
+This is an **excellent Phase 2 design document** with:
+- ✅ Expert-level domain accuracy (5/5 semantic)
+- ✅ Exemplary formatting and examples (5/5 physical)
+- ✅ Unambiguous decisions and resolved questions (5/5 social)
+- ✅ Highly actionable with defined structures (4/5 pragmatic, weighted 1.5x)
+- ✅ Strong consistency with minor refinements possible (4/5 syntactic, weighted 1.5x)
+
+The document successfully incorporates user feedback (Option B for hybrid approach) and real-world configuration from try_search. The remaining suggestions are **non-blocking polish items** that would achieve near-perfect scores but are not essential for implementation success.
+
+---
+
+## Strengths Worthy of Recognition
+
+1. **Exceptional responsiveness**: User decisions (Q1-Q3) integrated completely and correctly
+2. **Real-world grounding**: try_search config and auth patterns incorporated accurately
+3. **Complete specifications**: QuickwitConfig, auto-discovery logic, dual auth - all defined
+4. **Comprehensive testing**: 14 AC + 12 INV = 26 distinct test requirements
+5. **Clear trade-offs**: Auto-discovery latency acknowledged and accepted (~300ms)
+6. **Production-ready examples**: Both localhost dev and production cloud configs provided
+
+---
+
+## Next Steps
+
+**✅ APPROVED FOR PHASE 3**
+
+The design is ready for implementation. Proceed with `zestic-engineering-skills:disciplined-implementation` to execute the 14-step plan.
+
+**Pre-Phase-3 Checklist:**
+- ✅ Q1 Resolved: Quickwit instance available at `https://logs.terraphim.cloud/api/`
+- ✅ Q2 Resolved: Hybrid approach (Option B) approved
+- ✅ Q3 Confirmed: Docker Compose + #[ignore] tests
+- ✅ Authentication: Basic Auth (cloudflare/password) and Bearer token supported
+- ✅ Indexes: workers-logs, cadro-service-layer available for testing
+
+**Optional Pre-Implementation:**
+- Address HIGH priority revisions (method signatures, auth description update)
+- Set up local Quickwit Docker instance for development
+- Obtain cloudflare password from wrangler secrets for testing
+
+**Phase 3 Implementation Guidance:**
+- Follow steps 1-14 in sequence
+- Test after each step as specified
+- Commit after each successful step (project policy)
+- Use provided acceptance criteria for verification
+- Reference QuickwitConfig structure (lines 343-352) and auto-discovery logic (lines 356-366)
+
+---
+
+**Evaluation Complete** - Document quality significantly improved and exceeds all thresholds. Ready for implementation.