remove old tables we moved to clickhouse

[chitalian]

No description provided.

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Preview	Comments	Updated (UTC)
helicone-bifrost	Ready	Preview	Comment	Oct 22, 2025 1:53am

2 Skipped Deployments

Project	Deployment	Preview	Comments	Updated (UTC)
helicone	Skipped			Oct 22, 2025 1:53am
helicone-eu	Skipped			Oct 22, 2025 1:53am

[claude]

Claude finished @chitalian's task —— View job

---

PR Review Complete

Score: 2/10 - Critical blocking issues that will cause immediate production failures

Critical Issues Found

🚨 BLOCKING: Database Query Failures

1. experimentStore.ts - Multiple queries reference dropped tables:

• Lines 149-157: FROM response query in responseObjectString()
• Lines 160-170: FROM request re query in requestObjectString()
• Lines 1063-1096: JOIN response resp ON resp.request = r.id
• Lines 1450-1453: LEFT JOIN response resp ON resp.request = pir.source_request

2. DatasetManager.ts:209:

left join request on request.id = prompt_input_record.source_request

These queries will throw "relation does not exist" errors immediately when:

• Users access experiment features
• Dataset functionality is used
• Any experiment-related APIs are called

🚨 BLOCKING: Deprecated Table References

The migration drops prompt_input_record table, but it's still referenced in:

• experimentStore.ts (multiple locations)
• DatasetManager.ts
• PromptManager.ts (mentioned as deprecated but still used)

Code Quality Issues

📝 Missing Implementation in Modified Files

ScoreStore.ts: The old bumpRequestVersion() method was removed, but no deprecation handling or migration path was implemented. This could break existing API consumers.

VersionedRequestStore.ts: New methods added but the putPropertyAndBumpVersion() method referenced in migration comments is missing.

Suggestions Summary

Critical Fixes Needed:

1. Update experimentStore.ts queries to use ClickHouse request_response_rmt table instead of request/response
2. Fix DatasetManager.ts to eliminate request table join
3. Add deprecation error handling for methods that no longer work
4. Implement missing methods mentioned in migration comments

Recommended Approach:

// Example fix for experimentStore.ts
const responseObjectString = (filter: string) => {
  return `(
    SELECT jsonb_build_object(
      'createdAt', rr.response_created_at,
      'completionTokens', rr.completion_tokens,
      'promptTokens', rr.prompt_tokens,
      'delayMs', rr.latency,
      'model', rr.model
    )
    FROM request_response_rmt rr
    WHERE ${filter}
  )`;
};

Testing Required:

• Run experiment feature tests
• Test dataset creation/management
• Verify all experiment-related API endpoints
• Test legacy prompt UI graceful degradation

Migration Safety Concerns

The migration comments acknowledge these issues but the code wasn't updated. This creates a dangerous situation where the migration will succeed but the application will immediately start failing with database errors.

Recommendation: Do not merge until all database query references are updated to use ClickHouse tables.

[greptile-apps]

Greptile Overview

Summary

This PR drops legacy Postgres tables (request, response, asset, cache_hits, feedback, prompt_input_record) that were migrated to ClickHouse. Code changes deprecate methods that previously updated these tables.

Major changes:

• Migration drops 6 legacy tables and their foreign key constraints
• Updated 4 files to handle deprecated Postgres table operations
• ScoreStore.bumpRequestVersion() and VersionedRequestStore.putPropertyAndBumpVersion() now return errors
• InputsManager queries updated to use prompts_versions for org filtering
• RequestManager.waitForRequestAndResponse() now uses ClickHouse

Critical issues found:

• experimentStore.ts contains multiple queries using dropped request/response tables (lines 149-157, 160-170, 206-210, 307-310, 1063-1096, 1450-1453)
• DatasetManager.ts joins dropped request table (line 209)
• These files are listed in migration warnings but not updated in this PR
• Will cause runtime failures when experiment features are used

Confidence Score: 0/5

• This PR will cause immediate production failures when experiment/dataset features are accessed
• Multiple critical files (experimentStore.ts, DatasetManager.ts) still query the request and response tables being dropped. The migration itself acknowledges these files "may still reference these tables" but they were not updated. This will cause database errors immediately upon deployment when users access experiment or dataset functionality.
• valhalla/jawn/src/lib/stores/experimentStore.ts and valhalla/jawn/src/managers/dataset/DatasetManager.ts must be updated to use ClickHouse before this migration can be safely deployed

Important Files Changed

File Analysis

Filename	Score	Overview
supabase/migrations/20251021000000_drop_legacy_request_response_tables.sql	3/5	Migration drops legacy request, response, and other tables that were migrated to ClickHouse; includes foreign key cleanup
valhalla/jawn/src/lib/stores/experimentStore.ts	0/5	Multiple queries still reference dropped request/response tables - will fail at runtime
valhalla/jawn/src/managers/dataset/DatasetManager.ts	1/5	Query joins dropped request table - will cause database errors

Sequence Diagram

sequenceDiagram
    participant Client
    participant API
    participant Jawn
    participant Postgres
    participant ClickHouse

    Note over Postgres: Legacy tables:<br/>request, response, asset, etc.

    Client->>API: Request experiment data
    API->>Jawn: experimentStore.getExperiments()
    Jawn->>Postgres: SELECT with JOIN request/response
    Postgres--xJawn: ERROR: relation does not exist
    Jawn--xAPI: Query failure
    API--xClient: 500 Error

    Note over Jawn,ClickHouse: Fixed files use ClickHouse
    Client->>API: Request score update
    API->>Jawn: ScoreStore.bumpRequestVersion()
    Jawn-->>API: Returns error (deprecated)
    
    Client->>API: Request data (via fixed code)
    API->>Jawn: RequestManager.waitForRequestAndResponse()
    Jawn->>ClickHouse: Query request_response_rmt
    ClickHouse-->>Jawn: Data returned
    Jawn-->>API: Success
    API-->>Client: Data

Loading

Additional Comments (7)

1. valhalla/jawn/src/lib/stores/experimentStore.ts, line 149-157 (link)

   logic: queries response and request tables that are being dropped in this PR

2. valhalla/jawn/src/lib/stores/experimentStore.ts, line 160-170 (link)

   logic: queries request table being dropped

3. valhalla/jawn/src/lib/stores/experimentStore.ts, line 206-210 (link)

   logic: joins request table being dropped - should use ClickHouse or alternative approach

4. valhalla/jawn/src/lib/stores/experimentStore.ts, line 307-310 (link)

   logic: joins request table being dropped

5. valhalla/jawn/src/lib/stores/experimentStore.ts, line 1063-1096 (link)

   logic: joins both request and response tables being dropped

6. valhalla/jawn/src/lib/stores/experimentStore.ts, line 1450-1453 (link)

   logic: joins request and response tables being dropped

7. valhalla/jawn/src/managers/dataset/DatasetManager.ts, line 209-210 (link)

   logic: joins request table being dropped

_{6 files reviewed, 8 comments}

_{Edit Code Review Agent Settings | Greptile}

[greptile-apps]

style: malformed bash commands - remove debugging artifacts

Suggested change

	"Bash(psql:*)",
	"Bash(for table in asset cache_hits experiment_output experiment_v2_hypothesis_run feedback finetune_dataset_data job_node_request prompt_input_record properties request_job_task score_value)",
	"Bash(do echo -n \"$table: \")",
	"Bash(done)"
	"Bash(psql:*)"

Prompt To Fix With AI

This is a comment left during a code review.
Path: .claude/settings.local.json
Line: 61:64

Comment:
**style:** malformed bash commands - remove debugging artifacts

```suggestion
      "Bash(psql:*)"
```

How can I resolve this? If you propose a fix, please make it concise.