Rishabh/clarify questions system prompt #181

rishsriv · 2024-06-24T15:31:16Z

made phrasing clearer for 3 questions where we wanted all months/years - including those which did not occur in the data. This does not change results for any of the models, but figured we might as well clean it up
updated the golden query for a question (Return the highest sale price for each make and model of cars that have been sold and are no longer in inventory, ordered by the sale price from highest to lowest.)
added a system prompt to prompt_cot.md
fixed anthropic and bedrock runners
fixed an answer for an advanced question where it was unnecessarily coalescing a value to 0

rishsriv · 2024-06-24T18:21:17Z

JP/Wendy - for whenever you review this, feel free to make any changes needed and merge!

wendy-aw · 2024-06-25T01:53:38Z

query_generators/anthropic.py

+        else:
+            # raise Exception("Replace this with your private data import")
+            from defog_data_private.metadata import dbs
+
        with open(self.prompt_file) as file:
            model_prompt = file.read()
        question_instructions = question + " " + instructions
        if table_metadata_string == "":


Oooh good catch that we missed out on this logic for the anthropic runner

wongjingping

Thanks for updating, left a small comment on the advanced question.

wongjingping · 2024-06-25T01:44:54Z

data/instruct_advanced_postgres.csv

@@ -41,7 +41,7 @@ SPM (Selling Profit Margin) = (Total Amount from Sells - (Tax + Commission)) / T
 TAC = Total Active Customers who joined within a specified timeframe
 CR = Rank customers by their total transaction volume, identifying the customer with the highest transaction volume as rank 1. This involves joining price data with ticker identifiers and filtering for a specified date range."
 car_dealership,instructions_cte_join,"What is the average number of days between the sale date and payment received date, rounded to 2 decimal places?","When getting duration between sale and payment date for each sale, get the latest payment for sale by aggregating over the payments_received table first.","WITH sale_payments AS (SELECT s.id AS sale_id, s.sale_date, MAX(p.payment_date) AS latest_payment_date FROM sales s JOIN payments_received p ON s.id = p.sale_id GROUP BY 1,2) SELECT ROUND(AVG(latest_payment_date - sale_date), 2) AS avg_days_to_payment FROM sale_payments","When getting duration between sale and payment date for each sale, get the latest payment for sale by aggregating over the payments_received table first. ASP = Calculate the average price of sales within a specific timeframe Last 30 days = Use a range from the current date minus a certain interval to the current date, always ensure to make the necessary joins before utilizing the sales data. TSC = Count of sales within a specified period"
-car_dealership,instructions_cte_join,"Return the highest sale price for each make and model of cars that have been sold and are no longer in inventory, ordered by the sale price from highest to lowest.","When getting a car's inventory status, always take the latest status from the inventory_snapshots table","WITH latest_inventory_status AS (SELECT car_id, is_in_inventory, ROW_NUMBER() OVER(PARTITION BY car_id ORDER BY snapshot_date DESC, crtd_ts DESC) AS rn FROM inventory_snapshots) SELECT c.make, c.model, MAX(s.sale_price) AS highest_sale_price FROM cars c JOIN sales s ON c.id = s.car_id JOIN latest_inventory_status lis ON c.id = lis.car_id WHERE lis.is_in_inventory = FALSE AND lis.rn = 1 GROUP BY c.make, c.model ORDER BY highest_sale_price DESC","TSC = Count of sales within a specified period
+car_dealership,instructions_cte_join,"Return the highest sale price for each make and model of cars that have been sold and are no longer in inventory, ordered by the sale price from highest to lowest.","When getting a car's inventory status, always take the latest status from the inventory_snapshots table","WITH latest_inventory_status AS (SELECT car_id, is_in_inventory FROM inventory_snapshots) SELECT c.make, c.model, MAX(s.sale_price) AS highest_sale_price FROM cars c JOIN sales s ON c.id = s.car_id JOIN latest_inventory_status lis ON c.id = lis.car_id WHERE lis.is_in_inventory = FALSE GROUP BY c.make, c.model ORDER BY highest_sale_price DESC","TSC = Count of sales within a specified period


This is a tricky one and it took me awhile to figure out what's going on here. I think the golden query seems right, and the key lies in the instruction:

"When getting a car's inventory status, always take the latest status from the inventory_snapshots table"

ie to get the inventory status, we'll need to first get the latest row for each car in inventory_snapshot, hence the window function that partitions by car id and orders by snapshot_date (crtd_ts can be safely ignored). That said, I think this might still be ambiguous because this could mean either:

get the latest snapshot date, and only consider cars whose is_in_inventory on that specific date. in our data, the latest snapshot date is 2023-04-15, and there is only 1 entry on 2023-04-15 (car 9), and that means only that car is considered.

for each car, get its respective latest date. eg car 1's latest date is 2023-03-28, while car 9's latest date is 2023-04-15, and then use the is_in_inventory for that latest date. in our data this means that we have 10 cars that we can consider, out of which 8 is_in_inventory is false.

I think this is hard to clarify much further without an extensive glossary about how the tables work, and I think we might need to adapt the data, such that we get full snapshots of all cars on each snapshot date so that either of both interpretations return the same result. wdyt?

Ok i'm slightly confused. I'm not getting how the instructions could mean the first point. From "When getting a car's inventory status" I just thought it's pretty straightforward for it to mean an individual car, the same way your second pointer implies. Which means the row_number() is still important to the question.

Aye! From looking at the data underneath,the same car can have multiple entries in the inventory_snapshots table. For example, if this was (say) a second hand car store that can have the same car floating in and out of the system. M

Just clarifying this in either the instructions or questions should work (without updating the underlying data). From the question + instructions, it’s unclear that this would be the case!

I'll do this in a bit :D Thanks JP and Wendy!

Thanks for the inputs all! On a second read, I agree that the second point is the more natural interpretation and am for clarifying the question/instructions, possibly something along the lines of for each car, get its latest entry in the snapshots table and use that entry's inventory status

Fixed in latest push!

wongjingping · 2024-06-25T01:51:13Z

eval/bedrock_runner.py

+    generated_query = (
+        generated_query.split("```sql")[-1].split("```")[0].split(";")[0].strip() + ";"
+    )
+    print(generated_query)


nit: should we remove the print statement?

wongjingping · 2024-06-25T01:52:45Z

query_generators/anthropic.py

+                    shuffle,
+                )
+                pruned_metadata_str = pruned_metadata_ddl + join_str
+            elif columns_to_keep == 0:


thanks for adding this! sorry I missed this while updating the columns_to_keep earlier.

wendy-aw

Thanks for the improvements!

rishsriv · 2024-06-25T04:46:03Z

Made a final change (couldn't find this yesterday because it was a Monday)!

For this question in v1: show me the daily total amount of payments received in the whole of last week, split by the payment_method

Our previous golden query was:

SELECT
  payment_date,
  payment_method,
  SUM(payment_amount) AS total_amount
FROM
  payments_received
WHERE
  payment_date BETWEEN
    DATE_TRUNC('WEEK', CURRENT_DATE) - INTERVAL '1 week'
    AND DATE_TRUNC('WEEK', CURRENT_DATE)
GROUP BY
  payment_date, payment_method
ORDER BY payment_date DESC, payment_method ASC;

However, the question was ambiguous. Last week can mean both the last 7 days, and the last ISO week. So I have changed this to the harder and intended variant show me the daily total amount of payments received in the whole of the last ISO week, split by the payment_method

Additionally, the BETWEEN clause in inclusive. So this query would give answers for a 8 day period instead of a 7 day one. So the golden query is now updated to:

SELECT
  payment_date,
  payment_method,
  SUM(payment_amount) AS total_amount
FROM
  payments_received
WHERE
  payment_date >= DATE_TRUNC('WEEK', CURRENT_DATE) - INTERVAL '1 week'
  AND payment_date < DATE_TRUNC('WEEK', CURRENT_DATE)
GROUP BY
  payment_date, payment_method
ORDER BY payment_date DESC, payment_method ASC;

wendy-aw · 2024-06-25T04:48:57Z

ooh good find. thanks for the change!

wongjingping

lgtm thanks for the detailed fixes and explanation as well!

- Remove 'MoM will always be zero for the first month that appears in your answer.'

rishsriv added 11 commits June 24, 2024 22:49

added clarifications to questions

5d90242

added system prompt

170a9e1

fixed erroneous sql for a question

3e4562b

updated system prompt

c7d0651

Merge branch 'main' into rishabh/clarify-questions-system-prompt

6be87fd

updated prompted_cot

6a11bd6

updated upload_results to prevent breakage

93b1860

fixed anthropic runner issues

4be51eb

update max tokens for anthropic to 600

625d9ad

fix bedrock runner

2d30595

fixed answer for a question

8e75079

wendy-aw reviewed Jun 25, 2024

View reviewed changes

wongjingping reviewed Jun 25, 2024

View reviewed changes

wongjingping approved these changes Jun 25, 2024

View reviewed changes

wendy-aw approved these changes Jun 25, 2024

View reviewed changes

rishsriv added 3 commits June 25, 2024 11:51

removed unnecessary print statement

6316c7d

clarified phrasing for a question

12d524b

clarified phrasing for a question

1410b48

wongjingping approved these changes Jun 25, 2024

View reviewed changes

wongjingping merged commit 062638c into main Jun 25, 2024
2 checks passed

wongjingping deleted the rishabh/clarify-questions-system-prompt branch June 25, 2024 04:50

wendy-aw added a commit that referenced this pull request Jun 26, 2024

clarify questions, instructions according to #181

38076c1

wendy-aw added a commit that referenced this pull request Jun 26, 2024

- Standardize changes of #181 across dialects

082417b

- Remove 'MoM will always be zero for the first month that appears in your answer.'

wendy-aw mentioned this pull request Jun 26, 2024

Add on to #181 Question clarification #185

Merged

rishsriv pushed a commit that referenced this pull request Jun 26, 2024

clarify questions, instructions according to #181

f0a0ec5

rishsriv pushed a commit that referenced this pull request Jun 26, 2024

- Standardize changes of #181 across dialects

d635583

- Remove 'MoM will always be zero for the first month that appears in your answer.'

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Rishabh/clarify questions system prompt #181

Rishabh/clarify questions system prompt #181

rishsriv commented Jun 24, 2024 •

edited

Loading

rishsriv commented Jun 24, 2024

wendy-aw Jun 25, 2024

wongjingping left a comment

wongjingping Jun 25, 2024

wendy-aw Jun 25, 2024

rishsriv Jun 25, 2024

wongjingping Jun 25, 2024 •

edited

Loading

rishsriv Jun 25, 2024

wongjingping Jun 25, 2024

rishsriv Jun 25, 2024

wongjingping Jun 25, 2024

rishsriv Jun 25, 2024

wendy-aw left a comment

rishsriv commented Jun 25, 2024 •

edited

Loading

wendy-aw commented Jun 25, 2024

wongjingping left a comment

Rishabh/clarify questions system prompt #181

Rishabh/clarify questions system prompt #181

Conversation

rishsriv commented Jun 24, 2024 • edited Loading

rishsriv commented Jun 24, 2024

Choose a reason for hiding this comment

wongjingping left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wongjingping Jun 25, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wendy-aw left a comment

Choose a reason for hiding this comment

rishsriv commented Jun 25, 2024 • edited Loading

wendy-aw commented Jun 25, 2024

wongjingping left a comment

Choose a reason for hiding this comment

rishsriv commented Jun 24, 2024 •

edited

Loading

wongjingping Jun 25, 2024 •

edited

Loading

rishsriv commented Jun 25, 2024 •

edited

Loading