From 7529ed5863508c830b4d6eabfa4c4ca7f2cd0c72 Mon Sep 17 00:00:00 2001 From: hz Date: Tue, 14 Jul 2026 15:34:38 -0500 Subject: [PATCH] Add post-answer review and per-question loading --- api/QuestionsApi/Models/QuestionRequests.cs | 1 + api/QuestionsApi/Services/QuestionStore.cs | 13 +- scripts/autoload/block_session_service.gd | 17 ++- scripts/autoload/question_bank_service.gd | 40 ++++- scripts/ui/login_screen.gd | 1 - scripts/ui/question_player.gd | 157 +++++++++++++++++--- scripts/ui/study_selector.gd | 12 +- tests/post_answer_review_test.gd | 65 ++++++++ tests/post_answer_review_test.gd.uid | 1 + 9 files changed, 265 insertions(+), 42 deletions(-) create mode 100644 tests/post_answer_review_test.gd create mode 100644 tests/post_answer_review_test.gd.uid diff --git a/api/QuestionsApi/Models/QuestionRequests.cs b/api/QuestionsApi/Models/QuestionRequests.cs index 6ea90b4..f15d3e0 100644 --- a/api/QuestionsApi/Models/QuestionRequests.cs +++ b/api/QuestionsApi/Models/QuestionRequests.cs @@ -9,6 +9,7 @@ public class QuestionSearchRequest public int? DifficultyMax { get; set; } public int Limit { get; set; } = 40; public bool SupportsChoicePools { get; set; } + public List ExcludeExternalIds { get; set; } = []; } public class UpsertQuestionRequest diff --git a/api/QuestionsApi/Services/QuestionStore.cs b/api/QuestionsApi/Services/QuestionStore.cs index 7282b0b..445bada 100644 --- a/api/QuestionsApi/Services/QuestionStore.cs +++ b/api/QuestionsApi/Services/QuestionStore.cs @@ -37,11 +37,20 @@ public class QuestionStore filters.Add(builder.Gte(q => q.Difficulty, req.DifficultyMin.Value)); if (req.DifficultyMax is not null) filters.Add(builder.Lte(q => q.Difficulty, req.DifficultyMax.Value)); + var excludedIds = req.ExcludeExternalIds + .Where(id => !string.IsNullOrWhiteSpace(id)) + .Select(id => id.Trim()) + .Distinct(StringComparer.Ordinal) + .ToList(); + if (excludedIds.Count > 0) + filters.Add(builder.Nin(q => q.ExternalId, excludedIds)); var filter = filters.Count == 0 ? builder.Empty : builder.And(filters); var limit = Math.Clamp(req.Limit, 1, 80); - var questions = await _col.Find(filter).Limit(limit).ToListAsync(); - return questions.OrderBy(_ => Guid.NewGuid()).ToList(); + return await _col.Aggregate() + .Match(filter) + .Sample(limit) + .ToListAsync(); } public async Task GetByIdAsync(string id) => diff --git a/scripts/autoload/block_session_service.gd b/scripts/autoload/block_session_service.gd index e20c47e..f980815 100644 --- a/scripts/autoload/block_session_service.gd +++ b/scripts/autoload/block_session_service.gd @@ -246,7 +246,13 @@ func advance_core_question() -> bool: if question.is_empty(): core_status_message = "All available questions cleared." return false - active_questions.append(question) + return append_core_question(question) + + +func append_core_question(question: Dictionary) -> bool: + if question.is_empty(): + return false + active_questions.append(_prepare_question_choices(question.duplicate(true))) answers.append(null) current_index = active_questions.size() - 1 current_question_started_at = Time.get_ticks_msec() / 1000.0 @@ -257,6 +263,15 @@ func advance_core_question() -> bool: return true +func get_core_excluded_question_ids() -> Array: + var excluded: Array = ProfileService.get_profile().get("correct_question_ids", []).duplicate() + for question in active_questions: + var question_id := str(question.get("id", "")) + if question_id != "" and not excluded.has(question_id): + excluded.append(question_id) + return excluded + + func get_current_elapsed_seconds() -> float: if current_question_started_at <= 0.0: return 0.0 diff --git a/scripts/autoload/question_bank_service.gd b/scripts/autoload/question_bank_service.gd index 8470955..af22cfb 100644 --- a/scripts/autoload/question_bank_service.gd +++ b/scripts/autoload/question_bank_service.gd @@ -43,6 +43,7 @@ var pending_report_question_id := "" var attempt_queue: Array = [] var active_attempt_index := -1 var attempt_request_active := false +var pending_excluded_question_ids: Array = [] func _ready() -> void: questions_api_url = EndpointConfig.resolve( @@ -92,30 +93,34 @@ func _ready() -> void: _process_attempt_queue() -func load_questions_from_api(reset_retry: bool = true) -> void: +func load_questions_from_api(reset_retry: bool = true, excluded_question_ids: Array = []) -> void: if AuthState.access_token.is_empty(): _set_load_error("Login required before loading questions.", false) return if reset_retry: + pending_excluded_question_ids = excluded_question_ids.duplicate() retry_attempt = 0 next_retry_seconds = 0 retry_timer.stop() + var should_load_ratings := load_state == "idle" is_loading = true load_state = "retrying" if retry_attempt > 0 else "loading" last_error = "" questions.clear() questions_changed.emit() - load_ratings_from_api() + if should_load_ratings: + load_ratings_from_api() var headers := PackedStringArray([ "Content-Type: application/json", "Authorization: Bearer %s" % AuthState.access_token ]) var payload := { - "limit": 80, - "supportsChoicePools": true + "limit": 1, + "supportsChoicePools": true, + "excludeExternalIds": pending_excluded_question_ids } var err := request.request(questions_api_url, headers, HTTPClient.METHOD_POST, JSON.stringify(payload)) if err != OK: @@ -156,7 +161,11 @@ func retry_now() -> void: retry_attempt = 0 next_retry_seconds = 0 retry_timer.stop() - load_questions_from_api(false) + load_questions_from_api(false, pending_excluded_question_ids) + + +func load_next_random_question(excluded_question_ids: Array) -> void: + load_questions_from_api(true, excluded_question_ids) func clear_questions() -> void: @@ -185,6 +194,7 @@ func clear_questions() -> void: category_elos = {} last_rating_change = {} pending_report_question_id = "" + pending_excluded_question_ids.clear() questions_changed.emit() ratings_changed.emit() @@ -592,6 +602,24 @@ func _normalize_api_question(item: Variant) -> Dictionary: "labs": item.get("labs", []), "media": item.get("media", []), "tags": item.get("tags", []), - "references": item.get("references", []) + "references": _normalize_references(item.get("references", [])) } return normalized + + +func _normalize_references(raw_references: Variant) -> Array: + var normalized: Array = [] + if typeof(raw_references) != TYPE_ARRAY: + return normalized + for raw_reference in raw_references: + if typeof(raw_reference) != TYPE_DICTIONARY: + continue + normalized.append({ + "title": str(raw_reference.get("title", raw_reference.get("Title", ""))).strip_edges(), + "publisher": str(raw_reference.get("publisher", raw_reference.get("Publisher", ""))).strip_edges(), + "url": str(raw_reference.get("url", raw_reference.get("Url", ""))).strip_edges(), + "supports": str(raw_reference.get("supports", raw_reference.get("Supports", ""))).strip_edges(), + "accessed_on": str(raw_reference.get("accessedOn", raw_reference.get("accessed_on", raw_reference.get("AccessedOn", "")))).strip_edges(), + "reviewer": str(raw_reference.get("reviewer", raw_reference.get("Reviewer", ""))).strip_edges() + }) + return normalized diff --git a/scripts/ui/login_screen.gd b/scripts/ui/login_screen.gd index d9fd446..195c812 100644 --- a/scripts/ui/login_screen.gd +++ b/scripts/ui/login_screen.gd @@ -235,7 +235,6 @@ func _on_login_request_completed(result: int, response_code: int, _headers: Pack String(response.get("role", "")), String(response.get("exp", "")) ) - QuestionBankService.load_questions_from_api() UISoundService.play_sound("correct") password_input.text = "" registration_password_input.text = "" diff --git a/scripts/ui/question_player.gd b/scripts/ui/question_player.gd index a75e3a1..2aacdfb 100644 --- a/scripts/ui/question_player.gd +++ b/scripts/ui/question_player.gd @@ -41,6 +41,7 @@ var timer_color_state := "" var pending_correct_advance := false var question_text_scale := 1.0 var presented_choices: Array = [] +var awaiting_next_question := false var progress_label: Label var timer_label: Label @@ -48,6 +49,8 @@ var player_stats_label: Label var stem_label: RichTextLabel var lead_label: Label var explanation_label: RichTextLabel +var references_container: VBoxContainer +var question_scroll: ScrollContainer var question_elo_label: Label var next_button: Button var skip_button: Button @@ -107,6 +110,10 @@ func _ready() -> void: QuestionBankService.question_report_submitted.connect(_on_question_report_submitted) if not QuestionBankService.question_report_failed.is_connected(_on_question_report_failed): QuestionBankService.question_report_failed.connect(_on_question_report_failed) + if not QuestionBankService.questions_changed.is_connected(_on_random_question_changed): + QuestionBankService.questions_changed.connect(_on_random_question_changed) + if not QuestionBankService.questions_load_failed.is_connected(_on_random_question_failed): + QuestionBankService.questions_load_failed.connect(_on_random_question_failed) var top := HBoxContainer.new() top.add_theme_constant_override("separation", 12) @@ -212,15 +219,15 @@ func _ready() -> void: question_panel.size_flags_vertical = Control.SIZE_EXPAND_FILL add_child(question_panel) - var scroll := ScrollContainer.new() - scroll.size_flags_vertical = Control.SIZE_EXPAND_FILL - scroll.horizontal_scroll_mode = ScrollContainer.SCROLL_MODE_DISABLED - question_panel.add_child(scroll) + question_scroll = ScrollContainer.new() + question_scroll.size_flags_vertical = Control.SIZE_EXPAND_FILL + question_scroll.horizontal_scroll_mode = ScrollContainer.SCROLL_MODE_DISABLED + question_panel.add_child(question_scroll) var content_margin := MarginContainer.new() content_margin.size_flags_horizontal = Control.SIZE_EXPAND_FILL content_margin.add_theme_constant_override("margin_right", 16) - scroll.add_child(content_margin) + question_scroll.add_child(content_margin) question_content = VBoxContainer.new() question_content.size_flags_horizontal = Control.SIZE_EXPAND_FILL @@ -329,6 +336,11 @@ func _ready() -> void: _set_scaled_rich_text(explanation_label, 17) question_content.add_child(explanation_label) + references_container = VBoxContainer.new() + references_container.visible = false + references_container.add_theme_constant_override("separation", 6) + question_content.add_child(references_container) + var actions := HBoxContainer.new() actions.add_theme_constant_override("separation", 10) skip_button = Button.new() @@ -341,6 +353,7 @@ func _ready() -> void: next_button = Button.new() next_button.text = "Submit" + next_button.tooltip_text = "" next_button.custom_minimum_size = Vector2(140, 48) next_button.size_flags_horizontal = Control.SIZE_EXPAND_FILL Style.style_primary_button(next_button) @@ -389,6 +402,7 @@ func on_show(_payload: Variant = null) -> void: func on_hide() -> void: + awaiting_next_question = false _hide_report_modal(true) _hide_labs_modal() @@ -444,6 +458,7 @@ func _load_question() -> void: flag_button.button_pressed = false _refresh_toolbar_state() explanation_label.visible = false + _clear_references() question_elo_label.visible = false question_elo_label.text = "" next_button.text = "Submit" @@ -489,13 +504,7 @@ func _load_question() -> void: flagged = bool(saved_answer.get("flagged", false)) flag_button.button_pressed = flagged _refresh_toolbar_state() - if BlockSessionService.mode == "Tutor": - _show_explanation() - next_button.text = "Next" - else: - _apply_saved_choice_state() - next_button.text = "Next" - next_button.disabled = false + _show_answer_review() _update_question_elo_label(true) _refresh_nav_palette() @@ -628,6 +637,8 @@ func _toggle_eliminate_selected() -> void: func _submit_or_next() -> void: + if awaiting_next_question: + return if pending_correct_advance: return if current_answer.is_empty(): @@ -649,6 +660,7 @@ func _submit_or_next() -> void: if bool(result.get("is_correct", false)): current_answer = answer _update_question_elo_label(true) + _show_answer_review() _trigger_correct_feedback() return _lock_wrong_choice(attempted_choice) @@ -666,14 +678,58 @@ func _submit_or_next() -> void: _presented_choice_ids() ) current_answer = BlockSessionService.answer_current(selected_choice, eliminated, flagged) - if BlockSessionService.mode == "Tutor": - _show_explanation() - next_button.text = "Next" - else: - next_button.text = "Next" - _refresh_nav_palette() + _show_answer_review() else: - _advance_or_finish() + if BlockSessionService.session_kind == "core": + _request_next_core_question() + else: + _advance_or_finish() + + +func _request_next_core_question() -> void: + awaiting_next_question = true + next_button.text = "Loading..." + next_button.disabled = true + QuestionBankService.load_next_random_question(BlockSessionService.get_core_excluded_question_ids()) + + +func _on_random_question_changed() -> void: + if not awaiting_next_question or QuestionBankService.load_state != "ready": + return + var loaded_questions := QuestionBankService.get_all_questions() + if loaded_questions.is_empty(): + _on_random_question_failed("No additional questions are available.") + return + if not BlockSessionService.append_core_question(loaded_questions[0]): + _on_random_question_failed("The next question could not be prepared.") + return + awaiting_next_question = false + _load_question() + UISoundService.play_sound("question") + + +func _on_random_question_failed(message: String) -> void: + if not awaiting_next_question: + return + awaiting_next_question = false + next_button.text = "Retry" + next_button.disabled = false + next_button.tooltip_text = message + + +func _show_answer_review() -> void: + _show_explanation() + _show_references(BlockSessionService.get_current_question().get("references", [])) + next_button.text = "Next" + next_button.disabled = false + if skip_button != null: + skip_button.disabled = true + skip_button.visible = false + if question_scroll != null: + var review_target: Control = explanation_label + if references_container.visible: + review_target = references_container + question_scroll.call_deferred("ensure_control_visible", review_target) func _show_explanation() -> void: @@ -698,6 +754,62 @@ func _show_explanation() -> void: _refresh_nav_palette() +func _show_references(raw_references: Variant) -> void: + _clear_references() + if typeof(raw_references) != TYPE_ARRAY: + return + var valid_references: Array = [] + for reference in raw_references: + if typeof(reference) != TYPE_DICTIONARY: + continue + var url := str(reference.get("url", "")).strip_edges() + if not _is_safe_reference_url(url): + continue + valid_references.append(reference) + if valid_references.is_empty(): + return + + var heading := Label.new() + heading.text = "References" + _set_scaled_label(heading, 18, Style.TEXT) + references_container.add_child(heading) + for reference in valid_references: + var title := str(reference.get("title", "Reference")).strip_edges() + var publisher := str(reference.get("publisher", "")).strip_edges() + var url := str(reference.get("url", "")).strip_edges() + var link := LinkButton.new() + link.text = title if publisher == "" else "%s - %s" % [title, publisher] + link.tooltip_text = url + link.size_flags_horizontal = Control.SIZE_EXPAND_FILL + link.text_overrun_behavior = TextServer.OVERRUN_TRIM_ELLIPSIS + link.add_theme_color_override("font_color", Style.CYAN) + link.add_theme_color_override("font_hover_color", Style.MAGENTA) + link.set_meta("question_base_font_size", 16) + link.add_theme_font_size_override("font_size", _scaled_font_size(16)) + link.pressed.connect(_open_reference_url.bind(url)) + references_container.add_child(link) + references_container.visible = true + + +func _clear_references() -> void: + if references_container == null: + return + for child in references_container.get_children(): + references_container.remove_child(child) + child.queue_free() + references_container.visible = false + + +func _is_safe_reference_url(url: String) -> bool: + return url.begins_with("https://") and url.length() > 8 + + +func _open_reference_url(url: String) -> void: + if not _is_safe_reference_url(url): + return + OS.shell_open(url) + + func _advance_or_finish() -> void: if BlockSessionService.move_next(): _load_question() @@ -757,11 +869,8 @@ func _trigger_wrong_feedback() -> void: func _trigger_correct_feedback() -> void: UISoundService.play_sound("correct") - pending_correct_advance = true - correct_advance_delay = CORRECT_ADVANCE_DELAY_SECONDS red_fog_time = 0.0 green_fog_time = FEEDBACK_FADE_SECONDS - next_button.disabled = true if skip_button != null: skip_button.disabled = true for button in choice_buttons: @@ -948,6 +1057,8 @@ func _refresh_scaled_fonts(node: Node) -> void: node.add_theme_font_size_override("font_size", _scaled_font_size(base_size)) elif node is Button: node.add_theme_font_size_override("font_size", _scaled_font_size(base_size)) + elif node is LinkButton: + node.add_theme_font_size_override("font_size", _scaled_font_size(base_size)) elif node is RichTextLabel: node.add_theme_font_size_override("normal_font_size", _scaled_font_size(base_size)) node.add_theme_font_size_override("bold_font_size", _scaled_font_size(base_size)) @@ -1543,7 +1654,7 @@ func _apply_saved_choice_state() -> void: else: choice_buttons[index].modulate = Color.WHITE - if pending_correct_advance: + if pending_correct_advance or bool(current_answer.get("is_correct", false)): _apply_correct_answer_preview() if followup_required: diff --git a/scripts/ui/study_selector.gd b/scripts/ui/study_selector.gd index 77cc9ac..ddc8343 100644 --- a/scripts/ui/study_selector.gd +++ b/scripts/ui/study_selector.gd @@ -184,15 +184,9 @@ func _show_stage(stage_name: String) -> void: func _select_question_bank() -> void: - if QuestionBankService.has_questions() and QuestionBankService.load_state == "ready": - _start_question_bank() - return - pending_question_bank_launch = true - if QuestionBankService.load_state == "idle": - QuestionBankService.load_questions_from_api() - elif QuestionBankService.load_state == "failed": - QuestionBankService.retry_now() + var excluded_ids: Array = ProfileService.get_profile().get("correct_question_ids", []).duplicate() + QuestionBankService.load_next_random_question(excluded_ids) _refresh_bank_state() @@ -227,7 +221,7 @@ func _refresh_bank_state() -> void: elif state == "retrying": bank_status_label.text = "Question bank unavailable. Retrying in %d seconds." % QuestionBankService.next_retry_seconds elif state == "ready": - bank_status_label.text = "%d questions ready" % QuestionBankService.get_all_questions().size() + bank_status_label.text = "Question bank ready" elif state == "failed": bank_status_label.text = "The question bank could not be loaded." else: diff --git a/tests/post_answer_review_test.gd b/tests/post_answer_review_test.gd new file mode 100644 index 0000000..f4cb244 --- /dev/null +++ b/tests/post_answer_review_test.gd @@ -0,0 +1,65 @@ +extends SceneTree + +var failures := 0 + + +func _initialize() -> void: + _test_reference_normalization() + _test_reference_url_filter() + _test_correct_feedback_does_not_auto_advance() + quit(1 if failures > 0 else 0) + + +func _test_reference_normalization() -> void: + var service = load("res://scripts/autoload/question_bank_service.gd").new() + var references: Array = service._normalize_references([{ + "Title": "Clinical guideline", + "Publisher": "Example Society", + "Url": "https://example.org/guideline", + "Supports": "Recommended management", + "AccessedOn": "2026-07-14", + "Reviewer": "reviewer" + }]) + _assert(references.size() == 1, "a structured reference should be retained") + _assert(references[0].get("title", "") == "Clinical guideline", "reference title should be normalized") + _assert(references[0].get("accessed_on", "") == "2026-07-14", "reference access date should be normalized") + service.free() + + +func _test_reference_url_filter() -> void: + var player = load("res://scripts/ui/question_player.gd").new() + _assert(player._is_safe_reference_url("https://example.org/source"), "HTTPS references should be accepted") + _assert(not player._is_safe_reference_url("http://example.org/source"), "HTTP references should be rejected") + _assert(not player._is_safe_reference_url("javascript:alert(1)"), "script URLs should be rejected") + player.references_container = VBoxContainer.new() + player._show_references([ + {"title": "Clinical guideline", "publisher": "Example Society", "url": "https://example.org/source"}, + {"title": "Unsafe", "publisher": "Example", "url": "http://example.org/source"} + ]) + _assert(player.references_container.visible, "valid references should show the references section") + _assert(player.references_container.get_child_count() == 2, "the section should contain a heading and one safe link") + var link := player.references_container.get_child(1) as LinkButton + _assert(link != null and link.text == "Clinical guideline - Example Society", "reference link text should identify its source") + player.references_container.free() + player.free() + + +func _test_correct_feedback_does_not_auto_advance() -> void: + var player = load("res://scripts/ui/question_player.gd").new() + player.next_button = Button.new() + player.skip_button = Button.new() + player.feedback_fog = ColorRect.new() + player.pending_correct_advance = false + player._trigger_correct_feedback() + _assert(not player.pending_correct_advance, "correct feedback must not schedule automatic advancement") + player.next_button.free() + player.skip_button.free() + player.feedback_fog.free() + player.free() + + +func _assert(condition: bool, message: String) -> void: + if condition: + return + failures += 1 + push_error(message) diff --git a/tests/post_answer_review_test.gd.uid b/tests/post_answer_review_test.gd.uid new file mode 100644 index 0000000..0fb0313 --- /dev/null +++ b/tests/post_answer_review_test.gd.uid @@ -0,0 +1 @@ +uid://6va06epmux5u