From f17b6b26ae35f3dd2534ef5928fe16dabdadb871 Mon Sep 17 00:00:00 2001 From: Paul Elliott Date: Wed, 27 Aug 2025 09:32:39 -0400 Subject: [PATCH 1/4] fix: if scenario and scene linked, show valid experiment --- CLAUDE.md | 2 + align_browser/static/app.js | 77 ++++++++++++++++------- align_browser/static/state.js | 9 +-- align_browser/test_link_cascade.py | 98 ++++++++++++++++++++++++++++++ 4 files changed, 159 insertions(+), 27 deletions(-) create mode 100644 align_browser/test_link_cascade.py diff --git a/CLAUDE.md b/CLAUDE.md index dfabb80..7d5d1ec 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -4,6 +4,8 @@ - Use the uv created .venv when trying to run python code or python tests - Use semantic versioning commit messages - After you make non trival changes, run ruff linting, then ruff formating, then the tests +- Always use semantic commit messages. +- To build the frontend, always run from project root: `uv run python align_browser/build.py experiment-data/test-experiments --output-dir align_browser/static --build-only` ## Testing diff --git a/align_browser/static/app.js b/align_browser/static/app.js index f4a9352..457d925 100644 --- a/align_browser/static/app.js +++ b/align_browser/static/app.js @@ -14,6 +14,7 @@ import { getParameterValueFromRun, isParameterLinked, isResultParameter, + PARAMETER_CONFIG, } from './state.js'; import { @@ -24,21 +25,39 @@ import { getValidKDMAsForRun } from './table-formatter.js'; -// Constants -const KDMA_SLIDER_DEBOUNCE_MS = 500; +// Map state.js priority order (uses API names) to internal camelCase names +const PARAMETER_PRIORITY_ORDER = PARAMETER_CONFIG.PRIORITY_ORDER.map(param => { + const mapping = { + 'scenario': 'scenario', + 'scene': 'scene', + 'kdma_values': 'kdmaValues', + 'adm': 'admType', + 'llm': 'llmBackbone', + 'run_variant': 'runVariant' + }; + return mapping[param] || param; +}); // Generic function to preserve linked parameters after validation -function preserveLinkedParameters(validatedParams, originalParams, appState) { +function preserveLinkedParameters(validatedParams, originalParams, appState, changedParam) { const preserved = { ...validatedParams }; - // Iterate through all possible linked parameters - const linkableParams = ['scenario', 'scene', 'admType', 'llmBackbone', 'kdmaValues', 'runVariant']; - for (const paramName of linkableParams) { + // Get the priority index of the changed parameter + const changedIndex = PARAMETER_PRIORITY_ORDER.indexOf(changedParam); + + // Only preserve linked parameters that are HIGHER priority than the changed parameter + // Lower priority linked parameters should cascade to valid values + PARAMETER_PRIORITY_ORDER.forEach((paramName, paramIndex) => { if (isParameterLinked(paramName, appState)) { - // Preserve the original value for linked parameters - preserved[paramName] = originalParams[paramName]; + // Only preserve if this parameter has higher priority (lower index) than changed parameter + // OR if it's the same parameter (to prevent it from being reset) + if (paramIndex <= changedIndex) { + preserved[paramName] = originalParams[paramName]; + } + // Lower priority linked parameters will use the validated values + // WARNING: this may cause linked parameters to become out of sync across runs } - } + }); return preserved; } @@ -175,18 +194,33 @@ document.addEventListener("DOMContentLoaded", () => { const validParams = result.params; const validOptions = result.options; - // For propagated updates of linked parameters, use raw values without validation + // For propagated updates of linked parameters, we need to validate and cascade lower priority params if (isPropagatedUpdate && isParameterLinked(paramType, appState)) { - // Use the raw params values (don't validate for propagated linked parameters) - columnParameters.set(runId, createParameterStructure(params)); + // Get the corrected params with cascading for lower priority parameters + const kdmaValues = validParams.kdma_values || {}; + + const correctedParams = { + scenario: validParams.scenario, + scene: validParams.scene, + admType: validParams.adm, + llmBackbone: validParams.llm, + kdmaValues: kdmaValues, + runVariant: validParams.run_variant + }; + + // Use the special preserveLinkedParameters that respects priority hierarchy + const finalParams = preserveLinkedParameters(correctedParams, params, appState, paramType); + + // Store the parameters with proper cascading + columnParameters.set(runId, createParameterStructure(finalParams)); - // Update the actual run state with raw values - run.scenario = params.scenario; - run.scene = params.scene; - run.admType = params.admType; - run.llmBackbone = params.llmBackbone; - run.runVariant = params.runVariant; - run.kdmaValues = params.kdmaValues; + // Update the actual run state + run.scenario = finalParams.scenario; + run.scene = finalParams.scene; + run.admType = finalParams.admType; + run.llmBackbone = finalParams.llmBackbone; + run.runVariant = finalParams.runVariant; + run.kdmaValues = finalParams.kdmaValues; // Store the updated available options for UI dropdowns run.availableOptions = { @@ -200,7 +234,7 @@ document.addEventListener("DOMContentLoaded", () => { } }; - return params; // Return the raw params for propagated linked parameters + return finalParams; // Return the params with proper cascading } // For direct user updates (including on linked parameters), always validate for proper cascading @@ -219,7 +253,8 @@ document.addEventListener("DOMContentLoaded", () => { }; // Preserve any linked parameters - they should not be changed by validation - const finalParams = preserveLinkedParameters(correctedParams, params, appState); + // Pass the changed parameter so we know which linked params to preserve vs cascade + const finalParams = preserveLinkedParameters(correctedParams, params, appState, paramType); // Store corrected parameters columnParameters.set(runId, createParameterStructure(finalParams)); diff --git a/align_browser/static/state.js b/align_browser/static/state.js index dc1e071..d127cde 100644 --- a/align_browser/static/state.js +++ b/align_browser/static/state.js @@ -231,7 +231,7 @@ export function decodeStateFromURL() { } // Configuration for parameter validation system -const PARAMETER_CONFIG = { +export const PARAMETER_CONFIG = { // Priority order for parameter cascading PRIORITY_ORDER: ['scenario', 'scene', 'kdma_values', 'adm', 'llm', 'run_variant'], @@ -350,11 +350,8 @@ export function toggleParameterLink(paramName, appState, callbacks) { appState.linkedParameters.add(paramName); // When enabling link, propagate the leftmost column's value const firstRun = Array.from(appState.pinnedRuns.values())[0]; - let propagationResult = null; - if (firstRun) { - const currentValue = getParameterValueFromRun(firstRun, paramName); - propagationResult = propagateParameterToAllRuns(paramName, currentValue, firstRun.id, appState, callbacks); - } + const currentValue = getParameterValueFromRun(firstRun, paramName); + const propagationResult = propagateParameterToAllRuns(paramName, currentValue, firstRun.id, appState, callbacks); callbacks.renderTable(); callbacks.updateURL(); return propagationResult; diff --git a/align_browser/test_link_cascade.py b/align_browser/test_link_cascade.py new file mode 100644 index 0000000..7f03911 --- /dev/null +++ b/align_browser/test_link_cascade.py @@ -0,0 +1,98 @@ +"""Test that linked parameter cascading works properly""" +from playwright.sync_api import Page + + +def test_linked_scenario_scene_cascade(page: Page, real_data_test_server: str): + """Test that when scenario and scene are linked, changing scenario cascades scene properly.""" + page.goto(real_data_test_server) + + # Wait for page to load + page.wait_for_selector(".comparison-table", timeout=10000) + page.wait_for_function( + "document.querySelectorAll('.table-scenario-select').length > 0", timeout=10000 + ) + + # Add a second column for testing + add_column_btn = page.locator("#add-column-btn") + if add_column_btn.is_visible(): + add_column_btn.click() + page.wait_for_timeout(1000) + + # Get initial scenario values from both columns + scenario_selects = page.locator(".table-scenario-select") + assert scenario_selects.count() >= 2, "Need at least 2 columns to test" + + initial_scenario_col1 = scenario_selects.nth(0).input_value() + initial_scenario_col2 = scenario_selects.nth(1).input_value() + + # Get initial scene values - scene dropdowns use table-scenario-select class but in scene row + scene_row = page.locator("tr.parameter-row[data-parameter='scene']") + scene_selects = scene_row.locator(".table-scenario-select") + initial_scene_col1 = scene_selects.nth(0).input_value() + initial_scene_col2 = scene_selects.nth(1).input_value() + + print("Initial states:") + print(f" Column 1: scenario={initial_scenario_col1}, scene={initial_scene_col1}") + print(f" Column 2: scenario={initial_scenario_col2}, scene={initial_scene_col2}") + + # Link both scenario and scene parameters + scenario_row = page.locator("tr.parameter-row[data-parameter='scenario']") + scenario_link = scenario_row.locator(".link-icon") + scenario_link.click() + page.wait_for_timeout(500) + + scene_row = page.locator("tr.parameter-row[data-parameter='scene']") + scene_link = scene_row.locator(".link-icon") + scene_link.click() + page.wait_for_timeout(500) + + # Verify both are linked - the row itself gets the linked class + assert "linked" in scenario_row.get_attribute("class"), "Scenario row should have 'linked' class" + assert "linked" in scene_row.get_attribute("class"), "Scene row should have 'linked' class" + + # Change scenario in first column to a different value + # Find a different scenario option + scenario_select_col1 = scenario_selects.nth(0) + options = scenario_select_col1.locator("option").all() + new_scenario = None + for option in options: + value = option.get_attribute("value") + if value and value != initial_scenario_col1: + new_scenario = value + break + + assert new_scenario is not None, "Could not find a different scenario to switch to" + + print(f"\nChanging scenario from {initial_scenario_col1} to {new_scenario}") + scenario_select_col1.select_option(new_scenario) + page.wait_for_timeout(2000) # Wait for cascading and data reload + + # Check the results after changing scenario + final_scenario_col1 = scenario_selects.nth(0).input_value() + final_scenario_col2 = scenario_selects.nth(1).input_value() + final_scene_col1 = scene_selects.nth(0).input_value() + final_scene_col2 = scene_selects.nth(1).input_value() + + print("\nFinal states:") + print(f" Column 1: scenario={final_scenario_col1}, scene={final_scene_col1}") + print(f" Column 2: scenario={final_scenario_col2}, scene={final_scene_col2}") + + # Verify scenarios are synced (both should have the new scenario) + assert final_scenario_col1 == new_scenario, "Column 1 should have the new scenario" + assert final_scenario_col2 == new_scenario, "Column 2 should also have the new scenario (linked)" + + # Verify scenes are synced AND valid for the new scenario + assert final_scene_col1 == final_scene_col2, "Scenes should be synced across columns" + + # Check that we don't have "No data available" messages + no_data_messages = page.locator(".no-data-message").all() + for msg in no_data_messages: + if msg.is_visible(): + parent_cell = msg.locator("..").first + print(f"WARNING: Found 'No data available' message in cell: {parent_cell.inner_text()}") + + # The key test: scenes should have cascaded to valid values, not preserved invalid ones + assert len([msg for msg in no_data_messages if msg.is_visible()]) == 0, \ + "Should not have any 'No data available' messages after linked cascade" + + print("\nTest passed! Linked parameters cascade properly.") \ No newline at end of file From a2593be22891c131add3384df81f887c664b7324 Mon Sep 17 00:00:00 2001 From: Paul Elliott Date: Wed, 27 Aug 2025 13:35:12 -0400 Subject: [PATCH 2/4] refactor: state to app parameter handling --- align_browser/static/app.js | 82 ++++++++++-------------------- align_browser/static/state.js | 69 ++++++++++++------------- align_browser/test_link_cascade.py | 4 +- 3 files changed, 61 insertions(+), 94 deletions(-) diff --git a/align_browser/static/app.js b/align_browser/static/app.js index 457d925..854233c 100644 --- a/align_browser/static/app.js +++ b/align_browser/static/app.js @@ -14,7 +14,9 @@ import { getParameterValueFromRun, isParameterLinked, isResultParameter, - PARAMETER_CONFIG, + PARAMETER_PRIORITY_ORDER, + API_TO_APP, + APP_TO_API } from './state.js'; import { @@ -25,41 +27,38 @@ import { getValidKDMAsForRun } from './table-formatter.js'; -// Map state.js priority order (uses API names) to internal camelCase names -const PARAMETER_PRIORITY_ORDER = PARAMETER_CONFIG.PRIORITY_ORDER.map(param => { - const mapping = { - 'scenario': 'scenario', - 'scene': 'scene', - 'kdma_values': 'kdmaValues', - 'adm': 'admType', - 'llm': 'llmBackbone', - 'run_variant': 'runVariant' - }; - return mapping[param] || param; -}); // Generic function to preserve linked parameters after validation -function preserveLinkedParameters(validatedParams, originalParams, appState, changedParam) { - const preserved = { ...validatedParams }; +// Takes snake_case params from API and returns mixed camelCase/snake_case for internal use +function preserveLinkedParameters(validatedSnakeParams, originalCamelParams, appState, changedParamCamel) { + // Start with the validated params from API (in snake_case) + const preserved = { ...validatedSnakeParams }; - // Get the priority index of the changed parameter - const changedIndex = PARAMETER_PRIORITY_ORDER.indexOf(changedParam); + // Convert changed param to snake_case for priority checking + const changedParamSnake = APP_TO_API[changedParamCamel] || changedParamCamel; + const changedIndex = PARAMETER_PRIORITY_ORDER.indexOf(changedParamSnake); // Only preserve linked parameters that are HIGHER priority than the changed parameter - // Lower priority linked parameters should cascade to valid values - PARAMETER_PRIORITY_ORDER.forEach((paramName, paramIndex) => { - if (isParameterLinked(paramName, appState)) { + PARAMETER_PRIORITY_ORDER.forEach((snakeParam, paramIndex) => { + const camelParam = API_TO_APP[snakeParam]; + + if (camelParam && isParameterLinked(camelParam, appState)) { // Only preserve if this parameter has higher priority (lower index) than changed parameter // OR if it's the same parameter (to prevent it from being reset) if (paramIndex <= changedIndex) { - preserved[paramName] = originalParams[paramName]; + // Get the original value using the camelCase key from originalCamelParams + preserved[snakeParam] = originalCamelParams[camelParam]; } // Lower priority linked parameters will use the validated values - // WARNING: this may cause linked parameters to become out of sync across runs } }); - return preserved; + // Convert back to camelCase format for internal use + const result = {}; + Object.entries(APP_TO_API).forEach(([camelKey, snakeKey]) => { + result[camelKey] = preserved[snakeKey]; + }); + return result; } // CSV Download functionality @@ -196,20 +195,9 @@ document.addEventListener("DOMContentLoaded", () => { // For propagated updates of linked parameters, we need to validate and cascade lower priority params if (isPropagatedUpdate && isParameterLinked(paramType, appState)) { - // Get the corrected params with cascading for lower priority parameters - const kdmaValues = validParams.kdma_values || {}; - - const correctedParams = { - scenario: validParams.scenario, - scene: validParams.scene, - admType: validParams.adm, - llmBackbone: validParams.llm, - kdmaValues: kdmaValues, - runVariant: validParams.run_variant - }; - // Use the special preserveLinkedParameters that respects priority hierarchy - const finalParams = preserveLinkedParameters(correctedParams, params, appState, paramType); + // Pass the API params directly (snake_case) and original params (camelCase) + const finalParams = preserveLinkedParameters(validParams, params, appState, paramType); // Store the parameters with proper cascading columnParameters.set(runId, createParameterStructure(finalParams)); @@ -240,21 +228,10 @@ document.addEventListener("DOMContentLoaded", () => { // For direct user updates (including on linked parameters), always validate for proper cascading // This ensures the source column gets valid parameter combinations - // For unlinked parameters, use validated parameters - const kdmaValues = validParams.kdma_values || {}; - - const correctedParams = { - scenario: validParams.scenario, - scene: validParams.scene, - admType: validParams.adm, - llmBackbone: validParams.llm, - kdmaValues: kdmaValues, - runVariant: validParams.run_variant - }; - // Preserve any linked parameters - they should not be changed by validation // Pass the changed parameter so we know which linked params to preserve vs cascade - const finalParams = preserveLinkedParameters(correctedParams, params, appState, paramType); + // Pass the API params directly (snake_case) and original params (camelCase) + const finalParams = preserveLinkedParameters(validParams, params, appState, paramType); // Store corrected parameters columnParameters.set(runId, createParameterStructure(finalParams)); @@ -735,12 +712,7 @@ document.addEventListener("DOMContentLoaded", () => { // Update link state visual indicators const linkIcon = row.querySelector('.link-icon'); // Map snake_case parameter names to camelCase for link checking - const linkParamName = { - 'adm_type': 'admType', - 'llm_backbone': 'llmBackbone', - 'run_variant': 'runVariant', - 'kdma_values': 'kdmaValues' - }[paramName] || paramName; + const linkParamName = API_TO_APP[paramName] || paramName; if (isParameterLinked(linkParamName, appState)) { row.classList.add('linked'); diff --git a/align_browser/static/state.js b/align_browser/static/state.js index d127cde..263e05d 100644 --- a/align_browser/static/state.js +++ b/align_browser/static/state.js @@ -3,19 +3,27 @@ import { showWarning } from './notifications.js'; -// Centralized parameter mappings -export const PARAMETER_MAPPINGS = { - // Maps validation API names back to internal names - API_TO_INTERNAL: { - 'scenario': 'scenario', - 'scene': 'scene', - 'adm': 'admType', - 'llm': 'llmBackbone', - 'kdma_values': 'kdmaValues', - 'run_variant': 'runVariant' - } +export const API_TO_APP = { + 'scenario': 'scenario', + 'scene': 'scene', + 'adm': 'admType', + 'llm': 'llmBackbone', + 'kdma_values': 'kdmaValues', + 'run_variant': 'runVariant' +}; + +export const APP_TO_API = { + 'scenario': 'scenario', + 'scene': 'scene', + 'admType': 'adm', + 'llmBackbone': 'llm', + 'kdmaValues': 'kdma_values', + 'runVariant': 'run_variant' }; +// Priority order for parameter cascading +export const PARAMETER_PRIORITY_ORDER = ['scenario', 'scene', 'kdma_values', 'adm', 'llm', 'run_variant']; + // Constants for KDMA processing const KDMA_CONSTANTS = { DECIMAL_PRECISION: 10, // For 1 decimal place normalization @@ -230,15 +238,6 @@ export function decodeStateFromURL() { return null; } -// Configuration for parameter validation system -export const PARAMETER_CONFIG = { - // Priority order for parameter cascading - PRIORITY_ORDER: ['scenario', 'scene', 'kdma_values', 'adm', 'llm', 'run_variant'], - - // Parameters that require special handling - SPECIAL_COMPARISON_PARAMS: new Set(['kdma_values']) -}; - // Parameter update system with priority-based cascading const updateParametersBase = (priorityOrder) => (manifest) => (currentParams, changes) => { const newParams = { ...currentParams, ...changes }; @@ -260,15 +259,13 @@ const updateParametersBase = (priorityOrder) => (manifest) => (currentParams, ch // Only check constraint if the current selection has a non-null value for this parameter if (currentSelection[param] !== null && currentSelection[param] !== undefined) { - // Special handling for parameters that need custom comparison - if (PARAMETER_CONFIG.SPECIAL_COMPARISON_PARAMS.has(param)) { - if (param === 'kdma_values') { - const manifestKdmas = manifestEntry[param]; - const selectionKdmas = currentSelection[param]; - - if (!KDMAUtils.deepEqual(manifestKdmas, selectionKdmas)) { - return false; - } + // Special handling for kdma_values which needs deep comparison + if (param === 'kdma_values') { + const manifestKdmas = manifestEntry[param]; + const selectionKdmas = currentSelection[param]; + + if (!KDMAUtils.deepEqual(manifestKdmas, selectionKdmas)) { + return false; } } else if (manifestEntry[param] !== currentSelection[param]) { return false; @@ -310,13 +307,11 @@ const updateParametersBase = (priorityOrder) => (manifest) => (currentParams, ch // Only change if current value is invalid let isValid = validOptions.includes(currentValue); - // For special parameters, use custom comparison logic - if (PARAMETER_CONFIG.SPECIAL_COMPARISON_PARAMS.has(param) && !isValid) { - if (param === 'kdma_values') { - isValid = validOptions.some(option => { - return KDMAUtils.deepEqual(option, currentValue); - }); - } + // For kdma_values, use custom comparison logic + if (param === 'kdma_values' && !isValid) { + isValid = validOptions.some(option => { + return KDMAUtils.deepEqual(option, currentValue); + }); } if (!isValid) { @@ -338,7 +333,7 @@ const updateParametersBase = (priorityOrder) => (manifest) => (currentParams, ch }; // Export updateParameters with priority order already curried -export const updateParameters = updateParametersBase(PARAMETER_CONFIG.PRIORITY_ORDER); +export const updateParameters = updateParametersBase(PARAMETER_PRIORITY_ORDER); export function toggleParameterLink(paramName, appState, callbacks) { if (appState.linkedParameters.has(paramName)) { diff --git a/align_browser/test_link_cascade.py b/align_browser/test_link_cascade.py index 7f03911..04beb4c 100644 --- a/align_browser/test_link_cascade.py +++ b/align_browser/test_link_cascade.py @@ -37,12 +37,12 @@ def test_linked_scenario_scene_cascade(page: Page, real_data_test_server: str): # Link both scenario and scene parameters scenario_row = page.locator("tr.parameter-row[data-parameter='scenario']") - scenario_link = scenario_row.locator(".link-icon") + scenario_link = scenario_row.locator(".link-toggle") scenario_link.click() page.wait_for_timeout(500) scene_row = page.locator("tr.parameter-row[data-parameter='scene']") - scene_link = scene_row.locator(".link-icon") + scene_link = scene_row.locator(".link-toggle") scene_link.click() page.wait_for_timeout(500) From 20db388247a2b6c72c57a8de74aa6c84792d724f Mon Sep 17 00:00:00 2001 From: Paul Elliott Date: Wed, 27 Aug 2025 14:37:18 -0400 Subject: [PATCH 3/4] feat: add choice_info field to CSV export with ICL truncation - Add choice_info and label fields to InputOutputItem model - Add choice_info column to CSV export containing full JSON data - Truncate icl_example_responses to first example + "truncated" indicator - Reduces CSV size from 130MB to 14MB for large datasets while preserving sample data --- align_browser/csv_exporter.py | 31 ++++++++++++++ align_browser/experiment_models.py | 2 + align_browser/test_link_cascade.py | 66 ++++++++++++++++++------------ 3 files changed, 72 insertions(+), 27 deletions(-) diff --git a/align_browser/csv_exporter.py b/align_browser/csv_exporter.py index bf62dac..4190566 100644 --- a/align_browser/csv_exporter.py +++ b/align_browser/csv_exporter.py @@ -67,6 +67,35 @@ def extract_justification(item: InputOutputItem) -> str: return action.get("justification", "") +def extract_choice_info(item: InputOutputItem) -> str: + """Extract the choice_info as a JSON string, truncating ICL examples.""" + if not item.choice_info: + return "" + + import json + + # Create a copy to avoid modifying the original + filtered_choice_info = {} + + for key, value in item.choice_info.items(): + if key == "icl_example_responses" and isinstance(value, dict): + # Keep only first example for each KDMA, truncate the rest + truncated_icl = {} + for kdma, examples in value.items(): + if isinstance(examples, list) and len(examples) > 0: + # Keep first example, replace rest with "truncated" + truncated_icl[kdma] = [examples[0]] + if len(examples) > 1: + truncated_icl[kdma].append("truncated") + else: + truncated_icl[kdma] = examples + filtered_choice_info[key] = truncated_icl + else: + filtered_choice_info[key] = value + + return json.dumps(filtered_choice_info, separators=(",", ":")) + + def get_decision_time( timing_data: Optional[Dict[str, Any]], item_index: int ) -> Optional[float]: @@ -156,6 +185,7 @@ def experiment_to_csv_rows( else "", "choice_text": extract_choice_text(item), "choice_kdma_association": extract_choice_kdma(item), + "choice_info": extract_choice_info(item), "justification": extract_justification(item), "decision_time_s": get_decision_time(timing_data, idx), "score": get_score(scores_data, idx), @@ -188,6 +218,7 @@ def write_experiments_to_csv( "state_description", "choice_text", "choice_kdma_association", + "choice_info", "justification", "decision_time_s", "score", diff --git a/align_browser/experiment_models.py b/align_browser/experiment_models.py index 8227be7..7f920f5 100644 --- a/align_browser/experiment_models.py +++ b/align_browser/experiment_models.py @@ -184,6 +184,8 @@ class InputOutputItem(BaseModel): input: InputData output: Optional[Dict[str, Any]] = None + choice_info: Optional[Dict[str, Any]] = None + label: Optional[List[Dict[str, Any]]] = None original_index: int # Index in the original file diff --git a/align_browser/test_link_cascade.py b/align_browser/test_link_cascade.py index 04beb4c..120f0ce 100644 --- a/align_browser/test_link_cascade.py +++ b/align_browser/test_link_cascade.py @@ -1,55 +1,60 @@ """Test that linked parameter cascading works properly""" + from playwright.sync_api import Page def test_linked_scenario_scene_cascade(page: Page, real_data_test_server: str): """Test that when scenario and scene are linked, changing scenario cascades scene properly.""" page.goto(real_data_test_server) - + # Wait for page to load page.wait_for_selector(".comparison-table", timeout=10000) page.wait_for_function( "document.querySelectorAll('.table-scenario-select').length > 0", timeout=10000 ) - + # Add a second column for testing add_column_btn = page.locator("#add-column-btn") if add_column_btn.is_visible(): add_column_btn.click() page.wait_for_timeout(1000) - + # Get initial scenario values from both columns scenario_selects = page.locator(".table-scenario-select") assert scenario_selects.count() >= 2, "Need at least 2 columns to test" - + initial_scenario_col1 = scenario_selects.nth(0).input_value() initial_scenario_col2 = scenario_selects.nth(1).input_value() - + # Get initial scene values - scene dropdowns use table-scenario-select class but in scene row scene_row = page.locator("tr.parameter-row[data-parameter='scene']") scene_selects = scene_row.locator(".table-scenario-select") initial_scene_col1 = scene_selects.nth(0).input_value() initial_scene_col2 = scene_selects.nth(1).input_value() - + print("Initial states:") print(f" Column 1: scenario={initial_scenario_col1}, scene={initial_scene_col1}") print(f" Column 2: scenario={initial_scenario_col2}, scene={initial_scene_col2}") - + # Link both scenario and scene parameters scenario_row = page.locator("tr.parameter-row[data-parameter='scenario']") scenario_link = scenario_row.locator(".link-toggle") scenario_link.click() page.wait_for_timeout(500) - - scene_row = page.locator("tr.parameter-row[data-parameter='scene']") + + scene_row = page.locator("tr.parameter-row[data-parameter='scene']") scene_link = scene_row.locator(".link-toggle") scene_link.click() page.wait_for_timeout(500) - + # Verify both are linked - the row itself gets the linked class - assert "linked" in scenario_row.get_attribute("class"), "Scenario row should have 'linked' class" - assert "linked" in scene_row.get_attribute("class"), "Scene row should have 'linked' class" - + assert "linked" in scenario_row.get_attribute("class"), ( + "Scenario row should have 'linked' class" + ) + assert "linked" in scene_row.get_attribute("class"), ( + "Scene row should have 'linked' class" + ) + # Change scenario in first column to a different value # Find a different scenario option scenario_select_col1 = scenario_selects.nth(0) @@ -60,39 +65,46 @@ def test_linked_scenario_scene_cascade(page: Page, real_data_test_server: str): if value and value != initial_scenario_col1: new_scenario = value break - + assert new_scenario is not None, "Could not find a different scenario to switch to" - + print(f"\nChanging scenario from {initial_scenario_col1} to {new_scenario}") scenario_select_col1.select_option(new_scenario) page.wait_for_timeout(2000) # Wait for cascading and data reload - + # Check the results after changing scenario final_scenario_col1 = scenario_selects.nth(0).input_value() final_scenario_col2 = scenario_selects.nth(1).input_value() final_scene_col1 = scene_selects.nth(0).input_value() final_scene_col2 = scene_selects.nth(1).input_value() - + print("\nFinal states:") print(f" Column 1: scenario={final_scenario_col1}, scene={final_scene_col1}") print(f" Column 2: scenario={final_scenario_col2}, scene={final_scene_col2}") - + # Verify scenarios are synced (both should have the new scenario) assert final_scenario_col1 == new_scenario, "Column 1 should have the new scenario" - assert final_scenario_col2 == new_scenario, "Column 2 should also have the new scenario (linked)" - + assert final_scenario_col2 == new_scenario, ( + "Column 2 should also have the new scenario (linked)" + ) + # Verify scenes are synced AND valid for the new scenario - assert final_scene_col1 == final_scene_col2, "Scenes should be synced across columns" - + assert final_scene_col1 == final_scene_col2, ( + "Scenes should be synced across columns" + ) + # Check that we don't have "No data available" messages no_data_messages = page.locator(".no-data-message").all() for msg in no_data_messages: if msg.is_visible(): parent_cell = msg.locator("..").first - print(f"WARNING: Found 'No data available' message in cell: {parent_cell.inner_text()}") - + print( + f"WARNING: Found 'No data available' message in cell: {parent_cell.inner_text()}" + ) + # The key test: scenes should have cascaded to valid values, not preserved invalid ones - assert len([msg for msg in no_data_messages if msg.is_visible()]) == 0, \ + assert len([msg for msg in no_data_messages if msg.is_visible()]) == 0, ( "Should not have any 'No data available' messages after linked cascade" - - print("\nTest passed! Linked parameters cascade properly.") \ No newline at end of file + ) + + print("\nTest passed! Linked parameters cascade properly.") From 3a39580841958e40761f3f102e4de00f7bcb75e4 Mon Sep 17 00:00:00 2001 From: Paul Elliott Date: Wed, 27 Aug 2025 14:54:28 -0400 Subject: [PATCH 4/4] feat: add scene_id column and fix choice_kdma_association formatting - Add scene_id column after scenario_id, extracted from full_state.meta_info - Fix choice_kdma_association to format dictionary as string (e.g., "affiliation:0.0,medical:0.991304348") - Reorder choice_kdma_association column to be after state_description for better logical grouping --- align_browser/csv_exporter.py | 24 ++++++++++++++++++++++-- 1 file changed, 22 insertions(+), 2 deletions(-) diff --git a/align_browser/csv_exporter.py b/align_browser/csv_exporter.py index 4190566..6bd0a90 100644 --- a/align_browser/csv_exporter.py +++ b/align_browser/csv_exporter.py @@ -24,6 +24,15 @@ def format_kdma_config(kdma_values: List[Dict[str, Any]]) -> str: return ",".join(kdma_strings) +def extract_scene_id(item: InputOutputItem) -> str: + """Extract the scene_id from an input/output item.""" + if not item.input.full_state: + return "" + + meta_info = item.input.full_state.get("meta_info", {}) + return meta_info.get("scene_id", "") + + def extract_choice_text(item: InputOutputItem) -> str: """Extract the human-readable choice text from an input/output item.""" if not item.output or "choice" not in item.output: @@ -55,7 +64,16 @@ def extract_choice_kdma(item: InputOutputItem) -> str: return "" selected_choice = choices[choice_index] - return selected_choice.get("kdma_association", "") + kdma_association = selected_choice.get("kdma_association", "") + + # Format the KDMA association dictionary as a string + if isinstance(kdma_association, dict) and kdma_association: + kdma_strings = [] + for kdma_name, value in kdma_association.items(): + kdma_strings.append(f"{kdma_name}:{value}") + return ",".join(kdma_strings) + + return str(kdma_association) if kdma_association else "" def extract_justification(item: InputOutputItem) -> str: @@ -180,6 +198,7 @@ def experiment_to_csv_rows( "kdma_config": kdma_config, "alignment_target_id": alignment_target_id, "scenario_id": item.input.scenario_id, + "scene_id": extract_scene_id(item), "state_description": item.input.state if hasattr(item.input, "state") else "", @@ -215,9 +234,10 @@ def write_experiments_to_csv( "kdma_config", "alignment_target_id", "scenario_id", + "scene_id", "state_description", - "choice_text", "choice_kdma_association", + "choice_text", "choice_info", "justification", "decision_time_s",