From 8747bf255116b26a2325a48366fe6d13c34c6c62 Mon Sep 17 00:00:00 2001 From: Xander Vertegaal Date: Fri, 5 Sep 2025 11:11:09 +0200 Subject: [PATCH 01/42] Remove unused random parameter --- backend/problem/problem_details.py | 8 +++----- backend/problem/views/problem.py | 5 +---- frontend/src/app/types.ts | 1 - 3 files changed, 4 insertions(+), 10 deletions(-) diff --git a/backend/problem/problem_details.py b/backend/problem/problem_details.py index e9ee589..e7e1c53 100644 --- a/backend/problem/problem_details.py +++ b/backend/problem/problem_details.py @@ -5,9 +5,9 @@ def get_related_problem_ids( problem_id: int, -) -> Tuple[int | None, int | None, int | None]: +) -> Tuple[int | None, int | None]: """ - Retrieves the IDs of the next, previous, and random Problem objects + Retrieves the IDs of the next and previous Problem objects in the database relative to the given problem ID. """ @@ -15,14 +15,12 @@ def get_related_problem_ids( problem = Problem.objects.get(id=problem_id) except Problem.DoesNotExist: logger.warning(f"Problem ID {problem_id} does not exist.") - return None, None, None + return None, None next_problem = Problem.objects.filter(id__gt=problem.pk).order_by("id").first() previous_problem = Problem.objects.filter(id__lt=problem.pk).order_by("-id").first() - random_problem = Problem.objects.exclude(id=problem.pk).order_by("?").first() return ( next_problem.pk if next_problem else None, previous_problem.pk if previous_problem else None, - random_problem.pk if random_problem else None, ) diff --git a/backend/problem/views/problem.py b/backend/problem/views/problem.py index 23f4a51..af5d1dc 100644 --- a/backend/problem/views/problem.py +++ b/backend/problem/views/problem.py @@ -15,7 +15,6 @@ class ProblemResponse: error: str | None = None next: str | None = None previous: str | None = None - random: str | None = None def json_response(self, status=200) -> JsonResponse: return JsonResponse( @@ -26,7 +25,6 @@ def json_response(self, status=200) -> JsonResponse: "error": self.error, "next": self.next, "previous": self.previous, - "random": self.random, }, status=status, ) @@ -43,7 +41,7 @@ def get(self, request, problem_id: int): problem_index = problem.get_index() - next_problem_id, previous_problem_id, random_problem_id = ( + next_problem_id, previous_problem_id = ( get_related_problem_ids(problem_id) ) @@ -53,5 +51,4 @@ def get(self, request, problem_id: int): problem=problem, next=str(next_problem_id), previous=str(previous_problem_id), - random=str(random_problem_id), ).json_response(status=200) diff --git a/frontend/src/app/types.ts b/frontend/src/app/types.ts index 31526f8..c66a08e 100644 --- a/frontend/src/app/types.ts +++ b/frontend/src/app/types.ts @@ -57,7 +57,6 @@ export interface ProblemResponse { index: number | null; next: string | null; previous: string | null; - random: string | null; error: string | null; problem: Problem | null; } From 06e4a93915c2ffdf2344e64c0cddc987ddd6b7c9 Mon Sep 17 00:00:00 2001 From: Xander Vertegaal Date: Fri, 5 Sep 2025 21:07:21 +0200 Subject: [PATCH 02/42] User-added problems WIP --- backend/problem/urls.py | 1 + backend/problem/views/problem.py | 140 +++++++++++++++++- .../src/app/annotate/annotate.component.html | 85 +++++++++-- .../src/app/annotate/annotate.component.ts | 36 ++++- .../annotation-input.component.html | 36 ++++- .../annotation-input.component.ts | 35 ++++- .../problem-details.component.html | 88 +++++------ .../problem-details.component.scss | 12 -- .../annotate/navigator/navigator.component.ts | 4 +- .../src/app/services/app-mode.service.spec.ts | 16 ++ frontend/src/app/services/app-mode.service.ts | 31 ++++ frontend/src/app/services/problem.service.ts | 57 +++++-- frontend/src/app/types.ts | 18 ++- 13 files changed, 461 insertions(+), 98 deletions(-) create mode 100644 frontend/src/app/services/app-mode.service.spec.ts create mode 100644 frontend/src/app/services/app-mode.service.ts diff --git a/backend/problem/urls.py b/backend/problem/urls.py index 6bdec83..9f2aba8 100644 --- a/backend/problem/urls.py +++ b/backend/problem/urls.py @@ -7,6 +7,7 @@ urlpatterns = [ path("", ProblemView.as_view(), name="problem_view"), + path("new", ProblemView.as_view(), name="new_problem"), path("proofbank-stats", ProofBankStatsView.as_view(), name="proofbank_stats"), path("parse", ParseView.as_view(), name="parse_view"), ] diff --git a/backend/problem/views/problem.py b/backend/problem/views/problem.py index af5d1dc..b719c94 100644 --- a/backend/problem/views/problem.py +++ b/backend/problem/views/problem.py @@ -1,10 +1,12 @@ -from dataclasses import dataclass +from dataclasses import asdict, dataclass +from typing import TypedDict from django.http import JsonResponse from rest_framework.views import APIView +from langpro_annotator.logger import logger from problem.problem_details import get_related_problem_ids -from problem.models import Problem +from problem.models import KnowledgeBase, Problem, Sentence @dataclass @@ -30,8 +32,17 @@ def json_response(self, status=200) -> JsonResponse: ) +@dataclass +class SaveProblemResponse: + id: str | None = None + error: str | None = None + + def json_response(self, status=200) -> JsonResponse: + return JsonResponse(asdict(self), status=status) + + class ProblemView(APIView): - def get(self, request, problem_id: int): + def get(self, request, problem_id: int) -> JsonResponse: try: problem = Problem.objects.get(id=problem_id) except Problem.DoesNotExist: @@ -41,9 +52,7 @@ def get(self, request, problem_id: int): problem_index = problem.get_index() - next_problem_id, previous_problem_id = ( - get_related_problem_ids(problem_id) - ) + next_problem_id, previous_problem_id = get_related_problem_ids(problem_id) return ProblemResponse( id=problem.pk, @@ -52,3 +61,122 @@ def get(self, request, problem_id: int): next=str(next_problem_id), previous=str(previous_problem_id), ).json_response(status=200) + + def post(self, request) -> JsonResponse: + parse_input = request.data + + try: + validated_input = validate_input(parse_input) + except ValueError as e: + logger.error(f"Input validation error: {e}") + return SaveProblemResponse( + id=None, + error=str(e), + ).json_response(status=400) + + try: + problem = create_problem_from_input(validated_input) + except Exception as e: + logger.exception(f"Error saving problem: {e}") + return SaveProblemResponse( + id=None, + error=str(e), + ).json_response(status=500) + + return SaveProblemResponse( + id=str(problem.pk), + error=None, + ).json_response() + + +class KBItem(TypedDict): + entity1: str + relationship: str + entity2: str + + +class ParseInput(TypedDict): + premises: list[str] + hypothesis: str + kbItems: list[KBItem] + + +def validate_input(parse_input: dict) -> ParseInput: + """ + Validate the parse input data. + """ + if not isinstance(parse_input, dict): + raise ValueError("Input must be a dictionary") + + if "premises" not in parse_input or not isinstance(parse_input["premises"], list): + raise ValueError("Missing or invalid 'premises' field") + + if "hypothesis" not in parse_input or not isinstance( + parse_input["hypothesis"], str + ): + raise ValueError("Missing or invalid 'hypothesis' field") + + if "kbItems" not in parse_input or not isinstance(parse_input["kbItems"], list): + raise ValueError("Missing or invalid 'kbItems' field") + + for item in parse_input["kbItems"]: + if not isinstance(item, dict): + raise ValueError("Each kbItem must be a dictionary") + if "entity1" not in item or not isinstance(item["entity1"], str): + raise ValueError("Missing or invalid 'entity1' in kbItem") + if "relationship" not in item or not isinstance(item["relationship"], str): + raise ValueError("Missing or invalid 'relationship' in kbItem") + if item["relationship"].lower() not in KnowledgeBase.Relationship.values: + raise ValueError(f"Invalid 'relationship' in kbItem.") + if "entity2" not in item or not isinstance(item["entity2"], str): + raise ValueError("Missing or invalid 'entity2' in kbItem") + + return ParseInput( + premises=parse_input["premises"], + hypothesis=parse_input["hypothesis"], + kbItems=parse_input["kbItems"], + ) + + +def create_problem_from_input(parse_input: ParseInput) -> Problem: + """ + Save a new Problem instance from the given parse input data. + """ + try: + premise_sentences = [ + Sentence.objects.create(text=premise) for premise in parse_input["premises"] + ] + except Exception as e: + raise ValueError(f"Error creating premise sentences: {e}") + + try: + hypothesis_sentence = Sentence.objects.create(text=parse_input["hypothesis"]) + except Exception as e: + raise ValueError(f"Error creating hypothesis sentence: {e}") + + problem = Problem.objects.create( + hypothesis=hypothesis_sentence, + dataset=Problem.Dataset.USER, + # TODO: Determine entailment label based on LangPro parser output. + entailment_label=Problem.EntailmentLabel.UNKNOWN, + extra_data={}, + ) + + problem.premises.set(premise_sentences) + + for item in parse_input["kbItems"]: + entity1 = item.get("entity1") + relationship = item.get("relationship") + entity2 = item.get("entity2") + + try: + knowledge_base = KnowledgeBase.objects.create( + entity1=entity1, + relationship=relationship.lower(), + entity2=entity2, + problem=problem, + ) + except Exception as e: + raise ValueError(f"Error creating knowledge base items: {e}") + + return problem diff --git a/frontend/src/app/annotate/annotate.component.html b/frontend/src/app/annotate/annotate.component.html index 47faefd..bb51252 100644 --- a/frontend/src/app/annotate/annotate.component.html +++ b/frontend/src/app/annotate/annotate.component.html @@ -1,23 +1,88 @@ +@let firstProblemId = firstProblemId$ | async;
-
- - -
-
+
+ @if (browsing$ | async) { + } @else { + + } +
+ @if (browsing$ | async) { +
+ +
+ } @else if (adding$ | async) { +
+

+ You are currently adding a new problem. +

+

+ Hit 'Start Parse' to send your problem data to the parser and + inspect the results. +

+

+ If you are happy with the changes, save the problem by clicking + the 'Save' button. +

+

This will add the problem to the database.

+
+ } @else if (editing$ | async) { +
+

+ You are currently editing an existing problem. +

+

+ Hit 'Start Parse' to send your problem data to the parser and + inspect the results. +

+

+ If you are happy with the changes, save the changes by clicking + the 'Save' button. +

+ @if (isUserProblem$ | async) { +

+ This will update this problem in the database. +

+ } @else { +

+ This will create a new problem in the database, leaving the + original problem unchanged. +

+ } +
+ } @else { +
+ +
+ }
diff --git a/frontend/src/app/annotate/annotate.component.ts b/frontend/src/app/annotate/annotate.component.ts index 225072e..4bc2cfc 100644 --- a/frontend/src/app/annotate/annotate.component.ts +++ b/frontend/src/app/annotate/annotate.component.ts @@ -1,10 +1,16 @@ -import { Component } from "@angular/core"; +import { Component, inject } from "@angular/core"; import { AnnotationMenuComponent } from "./annotation-menu/annotation-menu.component"; import { NavigatorComponent } from "./navigator/navigator.component"; import { AnnotationInputComponent } from "./annotation-input/annotation-input.component"; import { SearchComponent } from "./search/search.component"; import { FontAwesomeModule } from "@fortawesome/angular-fontawesome"; -import { faTree } from "@fortawesome/free-solid-svg-icons"; +import { faPlus } from "@fortawesome/free-solid-svg-icons"; +import { Router } from "@angular/router"; +import { ProblemService } from "@/services/problem.service"; +import { map } from "rxjs"; +import { CommonModule } from "@angular/common"; +import { Dataset } from "@/types"; +import { AppModeService } from "@/services/app-mode.service"; @Component({ selector: "la-annotate", @@ -15,10 +21,34 @@ import { faTree } from "@fortawesome/free-solid-svg-icons"; AnnotationInputComponent, SearchComponent, FontAwesomeModule, + CommonModule, ], templateUrl: "./annotate.component.html", styleUrl: "./annotate.component.scss", }) export class AnnotateComponent { - public faTree = faTree; + private router = inject(Router); + private problemService = inject(ProblemService); + private appModeService = inject(AppModeService); + public faPlus = faPlus; + + public browsing$ = this.appModeService.browsing$; + public adding$ = this.appModeService.adding$; + public editing$ = this.appModeService.editing$; + + public firstProblemId$ = this.problemService.proofBankStats$.pipe( + map((stats) => stats?.firstProblemId ?? null), + ); + + public isUserProblem$ = this.problemService.problem$.pipe( + map(problem => problem?.problem?.dataset === Dataset.USER) + ); + + public goToProblem(problemId: string): void { + this.router.navigate(["/", "annotate", problemId]); + } + + public addProblem(): void { + this.router.navigate(["/", "annotate", "new"]); + } } diff --git a/frontend/src/app/annotate/annotation-input/annotation-input.component.html b/frontend/src/app/annotate/annotation-input/annotation-input.component.html index 3bcc8e4..b4c87d9 100644 --- a/frontend/src/app/annotate/annotation-input/annotation-input.component.html +++ b/frontend/src/app/annotate/annotation-input/annotation-input.component.html @@ -3,9 +3,39 @@ @if (form) {
- -
- + +
+ +
+
+ + @if (showSaveButton$ | async) { + + }
}
diff --git a/frontend/src/app/annotate/annotation-input/annotation-input.component.ts b/frontend/src/app/annotate/annotation-input/annotation-input.component.ts index 154ddd3..d896b92 100644 --- a/frontend/src/app/annotate/annotation-input/annotation-input.component.ts +++ b/frontend/src/app/annotate/annotation-input/annotation-input.component.ts @@ -15,12 +15,14 @@ import { } from "./knowledge-base-form/knowledge-base-form.component"; import { takeUntilDestroyed } from "@angular/core/rxjs-interop"; import { ProblemResponse } from "../../types"; -import { faCheck } from "@fortawesome/free-solid-svg-icons"; +import { faCheck, faFloppyDisk, faTree } from "@fortawesome/free-solid-svg-icons"; import { ProblemDetailsComponent } from "./problem-details/problem-details.component"; import { combineLatest, Subject } from "rxjs"; -import { ActivatedRoute } from "@angular/router"; +import { ActivatedRoute, Router } from "@angular/router"; import { ProblemService } from "@/services/problem.service"; import { ParseService } from "@/services/parse.service"; +import { FontAwesomeModule } from "@fortawesome/angular-fontawesome"; +import { AppModeService } from "@/services/app-mode.service"; export type ParseInputForm = FormGroup<{ premises: FormArray>; @@ -46,6 +48,7 @@ export type ParseInput = ReturnType; FormsModule, ReactiveFormsModule, ProblemDetailsComponent, + FontAwesomeModule, ], templateUrl: "./annotation-input.component.html", styleUrl: "./annotation-input.component.scss", @@ -55,6 +58,8 @@ export class AnnotationInputComponent implements OnInit { private destroyRef = inject(DestroyRef); private problemService = inject(ProblemService); private parseService = inject(ParseService); + private appModeService = inject(AppModeService); + private router = inject(Router); public form: ParseInputForm | null = null; public problem: ProblemResponse | null = null; @@ -62,6 +67,8 @@ export class AnnotationInputComponent implements OnInit { public submit$ = new Subject(); public faCheck = faCheck; + public faTree = faTree; + public faFloppyDisk = faFloppyDisk; ngOnInit(): void { this.problemService.problem$ @@ -75,6 +82,12 @@ export class AnnotationInputComponent implements OnInit { this.form = this.buildForm(problem); }); + this.problemService.saveProblem$.pipe( + takeUntilDestroyed(this.destroyRef) + ).subscribe((response) => { + this.router.navigate(["/", "annotate", response.id]); + }); + // Subscription needed to ensure a request is actually made. // TODO: replace this with actual parse results. this.parseService.parse$ @@ -94,6 +107,24 @@ export class AnnotationInputComponent implements OnInit { }); } + public showSaveButton$ = this.appModeService.adding$; + + public startParse(): void { + if (!this.form || this.form.invalid) { + return; + } + const input = this.form.getRawValue(); + this.parseService.submit.next(input); + } + + public saveProblem(): void { + if (!this.form || this.form.invalid) { + return; + } + const input = this.form.getRawValue(); + this.problemService.submit$.next(input); + } + private buildForm(response: ProblemResponse): ParseInputForm { const premises = response.problem?.premises || []; const hypothesis = response.problem?.hypothesis || ""; diff --git a/frontend/src/app/annotate/annotation-input/problem-details/problem-details.component.html b/frontend/src/app/annotate/annotation-input/problem-details/problem-details.component.html index cb3910d..e5bf8c5 100644 --- a/frontend/src/app/annotate/annotation-input/problem-details/problem-details.component.html +++ b/frontend/src/app/annotate/annotation-input/problem-details/problem-details.component.html @@ -1,46 +1,46 @@ @if (problemDetails(); as details) { -
-
- Entailment label: - -
- - - - - - - - - - - - @if (sectionString()) { - - - - - } - @if (details.comment) { - - - - - } - -
ID:{{ details.problemId }}
Dataset:{{ datasetLabels[details.dataset] }}
Section:{{ sectionString() }}
- Comment: - - {{ details.comment }} -
-
+
+ + + + + + + + + + + + + + + @if (sectionString()) { + + + + + } @if (details.comment) { + + + + + } + +
ID:{{ details.problemId }}
Dataset:{{ datasetLabels[details.dataset] }}
Entailment label: + +
Section:{{ sectionString() }}
+ Comment: + + {{ details.comment }} +
+
} diff --git a/frontend/src/app/annotate/annotation-input/problem-details/problem-details.component.scss b/frontend/src/app/annotate/annotation-input/problem-details/problem-details.component.scss index dd4f874..9f33078 100644 --- a/frontend/src/app/annotate/annotation-input/problem-details/problem-details.component.scss +++ b/frontend/src/app/annotate/annotation-input/problem-details/problem-details.component.scss @@ -1,15 +1,3 @@ -.problem-detail-container { - position: relative; -} - -.entailment-label-container { - position: absolute; - top: 1em; - right: 1em; - display: flex; - gap: 0.5em; -} - .table { td:first-child { width: 8em; diff --git a/frontend/src/app/annotate/navigator/navigator.component.ts b/frontend/src/app/annotate/navigator/navigator.component.ts index 02a14a6..eefe05e 100644 --- a/frontend/src/app/annotate/navigator/navigator.component.ts +++ b/frontend/src/app/annotate/navigator/navigator.component.ts @@ -35,6 +35,8 @@ export class NavigatorComponent { if (!id) { return; } - this.router.navigate(["/annotate", id]); + this.router.navigate(["/annotate", id], { + queryParamsHandling: "preserve", + }); } } diff --git a/frontend/src/app/services/app-mode.service.spec.ts b/frontend/src/app/services/app-mode.service.spec.ts new file mode 100644 index 0000000..6428aa1 --- /dev/null +++ b/frontend/src/app/services/app-mode.service.spec.ts @@ -0,0 +1,16 @@ +import { TestBed } from '@angular/core/testing'; + +import { AppModeService } from './app-mode.service'; + +describe('AppModeService', () => { + let service: AppModeService; + + beforeEach(() => { + TestBed.configureTestingModule({}); + service = TestBed.inject(AppModeService); + }); + + it('should be created', () => { + expect(service).toBeTruthy(); + }); +}); diff --git a/frontend/src/app/services/app-mode.service.ts b/frontend/src/app/services/app-mode.service.ts new file mode 100644 index 0000000..6b6974c --- /dev/null +++ b/frontend/src/app/services/app-mode.service.ts @@ -0,0 +1,31 @@ +import { inject, Injectable } from '@angular/core'; +import { Observable, map, startWith } from 'rxjs'; +import { ProblemService } from './problem.service'; + +export enum AppMode { + BROWSE = "browse", + EDIT = "edit", + ADD = "add", +} + +@Injectable({ + providedIn: 'root' +}) +export class AppModeService { + private problemService = inject(ProblemService); + + private viewMode$: Observable = this.problemService.problem$.pipe( + map(problem => { + if (problem?.id === "new") { + return AppMode.ADD; + } + return AppMode.BROWSE; + }), + startWith(undefined) + ); + + public browsing$ = this.viewMode$.pipe(map(mode => mode === AppMode.BROWSE)); + public adding$ = this.viewMode$.pipe(map(mode => mode === AppMode.ADD)); + public editing$ = this.viewMode$.pipe(map(mode => mode === AppMode.EDIT)); + public loading$ = this.viewMode$.pipe(map(mode => mode === undefined)); +} diff --git a/frontend/src/app/services/problem.service.ts b/frontend/src/app/services/problem.service.ts index 24133d5..a0675bb 100644 --- a/frontend/src/app/services/problem.service.ts +++ b/frontend/src/app/services/problem.service.ts @@ -1,8 +1,9 @@ -import { ProblemResponse, ProofBankStats } from '@/types'; +import { ParseInput } from '@/annotate/annotation-input/annotation-input.component'; +import { Dataset, EntailmentLabel, ProblemResponse, ProofBankStats, SaveProblemResponse } from '@/types'; import { HttpClient, HttpParams } from '@angular/common/http'; import { inject, Injectable } from '@angular/core'; import { ParamMap } from '@angular/router'; -import { Subject, Observable, switchMap, of, catchError, shareReplay } from 'rxjs'; +import { Subject, Observable, switchMap, of, catchError, shareReplay, exhaustMap } from 'rxjs'; @Injectable({ providedIn: 'root' @@ -12,25 +13,59 @@ export class ProblemService { public allParams$ = new Subject<{ params: ParamMap, queryParams: ParamMap; }>(); + // Submit a new problem to be saved to the database. + public submit$ = new Subject(); + public problem$: Observable = this.allParams$.pipe( switchMap(({ params, queryParams }) => { const problemId = params.get("problemId"); if (!problemId) { return of(null); } - - const httpParams = this.extractSearchParams(queryParams); - - return this.http.get(`/api/problem/${problemId}`, { params: httpParams }).pipe( - catchError((error) => { - console.error(`Error fetching problem ${problemId}:`, error); - return of(null); - }) - ); + return problemId === "new" ? this.newProblem$() : this.existingProblem$(problemId, queryParams); }), shareReplay(1) ); + public saveProblem$ = this.submit$.pipe( + exhaustMap((problem) => this.http.post('/api/problem/new', problem).pipe( + catchError((error) => { + console.error('Error saving problem:', error); + return of({ id: null, error: 'Failed to save problem' }); + }) + )) + ); + + private newProblem$(): Observable { + return of({ + id: "new", + index: null, + next: null, + previous: null, + error: null, + problem: { + id: "new", + hypothesis: "", + dataset: Dataset.USER, + premises: [], + entailmentLabel: EntailmentLabel.UNKNOWN, + extraData: null, + + }, + }); + } + + private existingProblem$(problemId: string, queryParams: ParamMap): Observable { + const httpParams = this.extractSearchParams(queryParams); + + return this.http.get(`/api/problem/${problemId}`, { params: httpParams }).pipe( + catchError((error) => { + console.error(`Error fetching problem ${problemId}:`, error); + return of(null); + }) + ); + }; + private extractSearchParams(routeParams: ParamMap): HttpParams { const text = routeParams.get("text"); const dataset = routeParams.get("dataset"); diff --git a/frontend/src/app/types.ts b/frontend/src/app/types.ts index c66a08e..049de70 100644 --- a/frontend/src/app/types.ts +++ b/frontend/src/app/types.ts @@ -24,7 +24,7 @@ interface SNLIData { } interface ProblemBase { - id: number; + id: string; premises: string[]; hypothesis: string | null; entailmentLabel: EntailmentLabel; @@ -52,21 +52,27 @@ interface UserProblem extends ProblemBase { type Problem = SickProblem | FracasProblem | SNLIProblem | UserProblem; -export interface ProblemResponse { - id: number; +interface BaseResponse { + error: string | null; +} + +export interface ProblemResponse extends BaseResponse { + id: string; index: number | null; next: string | null; previous: string | null; - error: string | null; problem: Problem | null; } - -export interface ProofBankStats { +export interface ProofBankStats extends BaseResponse { firstProblemId: string; lastProblemId: string; totalProblems: number; } +export interface SaveProblemResponse extends BaseResponse { + id: string | null; +} + export enum Dataset { SICK = "sick", FRACAS = "fracas", From 9c11b77c9fae8cdd47db1842bb84ba91d6cae919 Mon Sep 17 00:00:00 2001 From: Xander Vertegaal Date: Sun, 7 Sep 2025 14:39:23 +0200 Subject: [PATCH 03/42] Clean up templates --- frontend/src/app/annotate/annotate.component.html | 4 +--- frontend/src/app/annotate/annotate.component.ts | 4 +++- .../annotate/annotation-input/annotation-input.component.html | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/frontend/src/app/annotate/annotate.component.html b/frontend/src/app/annotate/annotate.component.html index bb51252..d1fa30f 100644 --- a/frontend/src/app/annotate/annotate.component.html +++ b/frontend/src/app/annotate/annotate.component.html @@ -10,7 +10,6 @@ > @@ -23,8 +22,7 @@ (click)="firstProblemId && goToProblem(firstProblemId)" > diff --git a/frontend/src/app/annotate/annotate.component.ts b/frontend/src/app/annotate/annotate.component.ts index 61bc940..ed79023 100644 --- a/frontend/src/app/annotate/annotate.component.ts +++ b/frontend/src/app/annotate/annotate.component.ts @@ -4,7 +4,7 @@ import { NavigatorComponent } from "./navigator/navigator.component"; import { AnnotationInputComponent } from "./annotation-input/annotation-input.component"; import { SearchComponent } from "./search/search.component"; import { FontAwesomeModule } from "@fortawesome/angular-fontawesome"; -import { faPlus } from "@fortawesome/free-solid-svg-icons"; +import { faBinoculars, faPlus } from "@fortawesome/free-solid-svg-icons"; import { Router } from "@angular/router"; import { ProblemService } from "@/services/problem.service"; import { map } from "rxjs"; @@ -30,7 +30,9 @@ export class AnnotateComponent { private router = inject(Router); private problemService = inject(ProblemService); private appModeService = inject(AppModeService); + public faPlus = faPlus; + public faBinoculars = faBinoculars; public browsing$ = this.appModeService.browsing$; public adding$ = this.appModeService.adding$; diff --git a/frontend/src/app/annotate/annotation-input/annotation-input.component.html b/frontend/src/app/annotate/annotation-input/annotation-input.component.html index 064e7dc..67860c1 100644 --- a/frontend/src/app/annotate/annotation-input/annotation-input.component.html +++ b/frontend/src/app/annotate/annotation-input/annotation-input.component.html @@ -19,8 +19,8 @@ class="fa-md" aria-hidden="true" focusable="false" - > - Start parse + /> + Start parse @if (showSaveButton$ | async) { - } @else { - - } - - @if (browsing$ | async) { + @if (browsing) {
- } @else if (adding$ | async) { + } @else if (adding) {

You are currently adding a new problem. @@ -48,9 +25,11 @@ If you are happy with the changes, save the problem by clicking the 'Save' button.

-

This will add the problem to the database.

+

+ This will add the problem to the database. +

- } @else if (editing$ | async) { + } @else if (editing) {

You are currently editing an existing problem. @@ -61,7 +40,7 @@

If you are happy with the changes, save the changes by clicking - the 'Save' button. + the 'Save problem' button.

@if (isUserProblem$ | async) {

@@ -72,7 +51,7 @@ This will create a new problem in the database, leaving the original problem unchanged.

- } + }
} @else {
@@ -81,6 +60,35 @@
} +
+ @if (browsing) { + + } @else { + + } +
diff --git a/frontend/src/app/annotate/annotate.component.ts b/frontend/src/app/annotate/annotate.component.ts index ed79023..7dd9860 100644 --- a/frontend/src/app/annotate/annotate.component.ts +++ b/frontend/src/app/annotate/annotate.component.ts @@ -1,16 +1,17 @@ -import { Component, inject } from "@angular/core"; +import { Component, DestroyRef, inject, OnInit } from "@angular/core"; import { AnnotationMenuComponent } from "./annotation-menu/annotation-menu.component"; import { NavigatorComponent } from "./navigator/navigator.component"; import { AnnotationInputComponent } from "./annotation-input/annotation-input.component"; import { SearchComponent } from "./search/search.component"; import { FontAwesomeModule } from "@fortawesome/angular-fontawesome"; import { faBinoculars, faPlus } from "@fortawesome/free-solid-svg-icons"; -import { Router } from "@angular/router"; +import { ActivatedRoute, Router } from "@angular/router"; import { ProblemService } from "@/services/problem.service"; -import { map } from "rxjs"; +import { combineLatest, map, tap } from "rxjs"; import { CommonModule } from "@angular/common"; import { Dataset } from "@/types"; import { AppModeService } from "@/services/app-mode.service"; +import { takeUntilDestroyed } from "@angular/core/rxjs-interop"; @Component({ selector: "la-annotate", @@ -26,24 +27,41 @@ import { AppModeService } from "@/services/app-mode.service"; templateUrl: "./annotate.component.html", styleUrl: "./annotate.component.scss", }) -export class AnnotateComponent { +export class AnnotateComponent implements OnInit { private router = inject(Router); + private route = inject(ActivatedRoute); private problemService = inject(ProblemService); private appModeService = inject(AppModeService); - + private destroyRef = inject(DestroyRef); + public faPlus = faPlus; public faBinoculars = faBinoculars; - public browsing$ = this.appModeService.browsing$; - public adding$ = this.appModeService.adding$; - public editing$ = this.appModeService.editing$; - + public viewMode$ = this.appModeService.viewMode$; public firstProblemId$ = this.problemService.firstProblemId$; public isUserProblem$ = this.problemService.problem$.pipe( - map(problem => problem?.problem?.dataset === Dataset.USER) + map(problem => problem?.dataset === Dataset.USER) ); + ngOnInit(): void { + const editParam$ = this.route.url.pipe( + map(segments => segments.some(segment => segment.path === "edit")) + ); + + combineLatest([ + this.route.paramMap, + this.route.queryParamMap, + editParam$ + ]) + .pipe( + takeUntilDestroyed(this.destroyRef) + ) + .subscribe(([params, queryParams, edit]) => { + this.problemService.allParams$.next({ params, queryParams, edit }); + }); + } + public goToProblem(problemId: string): void { this.router.navigate(["/", "annotate", problemId]); } diff --git a/frontend/src/app/annotate/annotation-input/annotation-input.component.html b/frontend/src/app/annotate/annotation-input/annotation-input.component.html index 67860c1..fbb434e 100644 --- a/frontend/src/app/annotate/annotation-input/annotation-input.component.html +++ b/frontend/src/app/annotate/annotation-input/annotation-input.component.html @@ -1,4 +1,7 @@ -@if (problem?.id) { +@let problem = problem$ | async; +@let viewMode = viewMode$ | async; +@let userProblem = isUserProblem$ | async; +@if (problem) {
@@ -22,7 +25,7 @@ /> Start parse - @if (showSaveButton$ | async) { + @if ((viewMode === 'edit' || viewMode === 'add') && userProblem) { + } @else if (userProblem) { + }
} diff --git a/frontend/src/app/annotate/annotation-input/annotation-input.component.ts b/frontend/src/app/annotate/annotation-input/annotation-input.component.ts index 6823836..73c9c2d 100644 --- a/frontend/src/app/annotate/annotation-input/annotation-input.component.ts +++ b/frontend/src/app/annotate/annotation-input/annotation-input.component.ts @@ -118,20 +118,8 @@ export class AnnotationInputComponent implements OnInit { .subscribe((response) => { console.log("Parse response:", response); }); - - // Listen to route changes only after subscribing to ProblemService.problem$. - combineLatest([ - this.route.paramMap, - this.route.queryParamMap]) - .pipe( - takeUntilDestroyed(this.destroyRef) - ) - .subscribe(([params, queryParams]) => { - this.problemService.allParams$.next({ params, queryParams }); - }); } - public startParse(): void { if (!this.form || this.form.invalid) { return; From cc0bbde99fbcbef25a4c21613bff2cf108cb11d8 Mon Sep 17 00:00:00 2001 From: Xander Vertegaal Date: Mon, 15 Sep 2025 19:10:40 +0200 Subject: [PATCH 08/42] Add AppMode service --- .../annotation-input.component.ts | 13 +++++------ .../knowledge-base-form.component.html | 19 +++++++++++++--- .../knowledge-base-form.component.ts | 7 +++++- .../premises-form.component.html | 22 ++++++++++++++----- .../premises-form/premises-form.component.ts | 7 +++++- frontend/src/app/services/app-mode.service.ts | 17 +++++++------- 6 files changed, 60 insertions(+), 25 deletions(-) diff --git a/frontend/src/app/annotate/annotation-input/annotation-input.component.ts b/frontend/src/app/annotate/annotation-input/annotation-input.component.ts index 73c9c2d..b3184ed 100644 --- a/frontend/src/app/annotate/annotation-input/annotation-input.component.ts +++ b/frontend/src/app/annotate/annotation-input/annotation-input.component.ts @@ -73,12 +73,12 @@ export class AnnotationInputComponent implements OnInit { public faFloppyDisk = faFloppyDisk; public faExclamationCircle = faExclamationCircle; public faTrash = faTrash; + public faWrench = faWrench; - public showSaveButton$ = this.appModeService.adding$; - - public isUserProblem(problem: ProblemResponse | null): boolean { - return problem?.problem?.dataset === Dataset.USER; - } + public viewMode$ = this.appModeService.viewMode$; + public isUserProblem$ = this.problem$.pipe( + map(problem => problem?.dataset === Dataset.USER) + ); ngOnInit(): void { this.problemService.problem$ @@ -87,8 +87,7 @@ export class AnnotationInputComponent implements OnInit { // Navigate away if the backend provides a new Problem ID. this.navigateToNewProblem(problem); - // Otherwise, update local state and form. - this.problem = problem; + // Otherwise, update local form. this.form = problem ? this.buildForm(problem) : null; }); diff --git a/frontend/src/app/annotate/annotation-input/knowledge-base-form/knowledge-base-form.component.html b/frontend/src/app/annotate/annotation-input/knowledge-base-form/knowledge-base-form.component.html index ec6d7f5..983a6f8 100644 --- a/frontend/src/app/annotate/annotation-input/knowledge-base-form/knowledge-base-form.component.html +++ b/frontend/src/app/annotate/annotation-input/knowledge-base-form/knowledge-base-form.component.html @@ -1,4 +1,5 @@ @let kbControls = form().controls.kbItems.controls; +@let viewMode = viewMode$ | async;
@@ -6,6 +7,7 @@ class="form-label h6 fw-bold d-flex align-items-center justify-content-between" > Knowledge base items + @if (viewMode === 'add' || viewMode === 'edit') { + }
@@ -28,12 +31,16 @@
- @@ -45,11 +52,16 @@ + @if (viewMode === 'add' || viewMode === 'edit') { + }
} @empty { diff --git a/frontend/src/app/annotate/annotation-input/knowledge-base-form/knowledge-base-form.component.ts b/frontend/src/app/annotate/annotation-input/knowledge-base-form/knowledge-base-form.component.ts index 2d953ec..a1dacd3 100644 --- a/frontend/src/app/annotate/annotation-input/knowledge-base-form/knowledge-base-form.component.ts +++ b/frontend/src/app/annotate/annotation-input/knowledge-base-form/knowledge-base-form.component.ts @@ -1,10 +1,11 @@ -import { Component, input } from "@angular/core"; +import { Component, inject, input } from "@angular/core"; import { FormGroup, Validators, FormControl } from "@angular/forms"; import { CommonModule } from "@angular/common"; import { ReactiveFormsModule } from "@angular/forms"; import { FontAwesomeModule } from "@fortawesome/angular-fontawesome"; import { faPlus, faTrash } from "@fortawesome/free-solid-svg-icons"; import { ParseInputForm } from "../annotation-input.component"; +import { AppModeService } from "@/services/app-mode.service"; export enum KnowledgeBaseRelationship { EQUAL = "EQUAL", @@ -28,6 +29,8 @@ const relationshipDisplayMapping: Record = { styleUrls: ["./knowledge-base-form.component.scss"], }) export class KnowledgeBaseFormComponent { + private appModeService = inject(AppModeService); + public form = input.required(); public relationshipTypes = Object.values(KnowledgeBaseRelationship); @@ -35,6 +38,8 @@ export class KnowledgeBaseFormComponent { public faPlus = faPlus; public faTrash = faTrash; + public viewMode$ = this.appModeService.viewMode$; + public addKnowledgeBaseItem(): void { const newItem = new FormGroup({ entity1: new FormControl("", { diff --git a/frontend/src/app/annotate/annotation-input/premises-form/premises-form.component.html b/frontend/src/app/annotate/annotation-input/premises-form/premises-form.component.html index 25c6631..99599af 100644 --- a/frontend/src/app/annotate/annotation-input/premises-form/premises-form.component.html +++ b/frontend/src/app/annotate/annotation-input/premises-form/premises-form.component.html @@ -1,4 +1,5 @@ @let premiseControls = form().controls.premises.controls; +@let viewMode = viewMode$ | async;
@@ -6,6 +7,7 @@ class="form-label h6 fw-bold d-flex align-items-center justify-content-between" > Premises + @if (viewMode === 'add' || viewMode === 'edit') { + }
    - @for (control of premiseControls; let i = $index; - track $index) { + @for (control of premiseControls; let i = $index; track $index) {
  • + @if (viewMode === 'add' || viewMode === 'edit') { + }
    • } @empty { -
  • No premises defined for this problem.
    • +
  • + No premises defined for this problem. +
    • }
@@ -65,7 +75,9 @@
diff --git a/frontend/src/app/annotate/annotation-input/premises-form/premises-form.component.ts b/frontend/src/app/annotate/annotation-input/premises-form/premises-form.component.ts index c5781cd..e5b1642 100644 --- a/frontend/src/app/annotate/annotation-input/premises-form/premises-form.component.ts +++ b/frontend/src/app/annotate/annotation-input/premises-form/premises-form.component.ts @@ -1,9 +1,10 @@ -import { Component, input } from "@angular/core"; +import { Component, inject, input } from "@angular/core"; import { ReactiveFormsModule, FormControl, Validators } from "@angular/forms"; import { CommonModule } from "@angular/common"; import { FontAwesomeModule } from "@fortawesome/angular-fontawesome"; import { faPlus, faTrash } from "@fortawesome/free-solid-svg-icons"; import { ParseInputForm } from "../annotation-input.component"; +import { AppModeService } from "@/services/app-mode.service"; export interface Premises { premises: string[]; @@ -18,11 +19,15 @@ export interface Premises { styleUrl: "./premises-form.component.scss", }) export class PremisesFormComponent { + private appModeService = inject(AppModeService); + public form = input.required(); public faPlus = faPlus; public faTrash = faTrash; + public viewMode$ = this.appModeService.viewMode$; + public addPremise(value: string = ""): void { const premisesArray = this.form().controls.premises; premisesArray.push( diff --git a/frontend/src/app/services/app-mode.service.ts b/frontend/src/app/services/app-mode.service.ts index 6b6974c..fab7600 100644 --- a/frontend/src/app/services/app-mode.service.ts +++ b/frontend/src/app/services/app-mode.service.ts @@ -1,5 +1,5 @@ import { inject, Injectable } from '@angular/core'; -import { Observable, map, startWith } from 'rxjs'; +import { filter, map, shareReplay } from 'rxjs'; import { ProblemService } from './problem.service'; export enum AppMode { @@ -14,18 +14,19 @@ export enum AppMode { export class AppModeService { private problemService = inject(ProblemService); - private viewMode$: Observable = this.problemService.problem$.pipe( - map(problem => { - if (problem?.id === "new") { + public viewMode$ = this.problemService.allParams$.pipe( + filter(allParams => allParams !== null), + map(({ params, edit }) => { + if (params.get("problemId") === "new") { return AppMode.ADD; } + if (edit) { + return AppMode.EDIT; + } return AppMode.BROWSE; }), - startWith(undefined) + shareReplay(1) ); - public browsing$ = this.viewMode$.pipe(map(mode => mode === AppMode.BROWSE)); - public adding$ = this.viewMode$.pipe(map(mode => mode === AppMode.ADD)); - public editing$ = this.viewMode$.pipe(map(mode => mode === AppMode.EDIT)); public loading$ = this.viewMode$.pipe(map(mode => mode === undefined)); } From c22a67bc6110363aa93b725a2c577b643f4fb6f7 Mon Sep 17 00:00:00 2001 From: Xander Vertegaal Date: Mon, 15 Sep 2025 19:14:24 +0200 Subject: [PATCH 09/42] Properly separate problemResponse from problem --- .../annotation-input.component.ts | 2 +- .../problem-details.component.ts | 26 +++++++------------ frontend/src/app/services/parse.service.ts | 4 +-- frontend/src/app/services/problem.service.ts | 10 +++++-- 4 files changed, 21 insertions(+), 21 deletions(-) diff --git a/frontend/src/app/annotate/annotation-input/annotation-input.component.ts b/frontend/src/app/annotate/annotation-input/annotation-input.component.ts index b3184ed..3ce6f70 100644 --- a/frontend/src/app/annotate/annotation-input/annotation-input.component.ts +++ b/frontend/src/app/annotate/annotation-input/annotation-input.component.ts @@ -124,7 +124,7 @@ export class AnnotationInputComponent implements OnInit { return; } const input = this.form.getRawValue(); - this.parseService.submit.next(input); + this.parseService.submit$.next(input); } public saveProblem(): void { diff --git a/frontend/src/app/annotate/annotation-input/problem-details/problem-details.component.ts b/frontend/src/app/annotate/annotation-input/problem-details/problem-details.component.ts index 248eb1b..ebd1136 100644 --- a/frontend/src/app/annotate/annotation-input/problem-details/problem-details.component.ts +++ b/frontend/src/app/annotate/annotation-input/problem-details/problem-details.component.ts @@ -1,5 +1,5 @@ import { Component, computed, input } from "@angular/core"; -import { Dataset, EntailmentLabel, ProblemResponse } from "../../../types"; +import { Dataset, EntailmentLabel, Problem } from "../../../types"; import { EntailmentLabelBadgeComponent } from "./entailment-label-badge/entailment-label-badge.component"; import { faQuestionCircle } from "@fortawesome/free-solid-svg-icons"; import { FontAwesomeModule } from "@fortawesome/angular-fontawesome"; @@ -27,7 +27,7 @@ export interface ProblemDetails { styleUrl: "./problem-details.component.scss", }) export class ProblemDetailsComponent { - public readonly problem = input.required(); + public readonly problem = input.required(); public problemDetails = computed(() => { const problem = this.problem(); @@ -57,23 +57,17 @@ export class ProblemDetailsComponent { return null; }); - private extractDetails( - response: ProblemResponse | null, - ): ProblemDetails | null { - if (!response?.problem) { - return null; - } - + private extractDetails(problem: Problem): ProblemDetails | null { const shared: Pick< ProblemDetails, "problemId" | "dataset" | "entailmentLabel" > = { - problemId: response.problem.id.toString(), - dataset: response.problem.dataset, - entailmentLabel: response.problem.entailmentLabel, + problemId: problem.id.toString(), + dataset: problem.dataset, + entailmentLabel: problem.entailmentLabel, }; - switch (response.problem.dataset) { + switch (problem.dataset) { case Dataset.SICK: return { ...shared, @@ -84,9 +78,9 @@ export class ProblemDetailsComponent { case Dataset.FRACAS: return { ...shared, - section: response.problem.extraData.sectionName, - subsection: response.problem.extraData.subsectionName, - comment: response.problem.extraData.note || null, + section: problem.extraData.sectionName, + subsection: problem.extraData.subsectionName, + comment: problem.extraData.note || null, }; case Dataset.SNLI: return { diff --git a/frontend/src/app/services/parse.service.ts b/frontend/src/app/services/parse.service.ts index 3605f6d..c5e0b12 100644 --- a/frontend/src/app/services/parse.service.ts +++ b/frontend/src/app/services/parse.service.ts @@ -10,9 +10,9 @@ import { Subject, switchMap, catchError, of } from 'rxjs'; export class ParseService { private http = inject(HttpClient); - public submit = new Subject(); + public submit$ = new Subject(); - public parse$ = this.submit.pipe( + public parse$ = this.submit$.pipe( switchMap((form) => this.http.post("/api/problem/parse", form).pipe( catchError((error) => { diff --git a/frontend/src/app/services/problem.service.ts b/frontend/src/app/services/problem.service.ts index fc24348..fa42d32 100644 --- a/frontend/src/app/services/problem.service.ts +++ b/frontend/src/app/services/problem.service.ts @@ -17,7 +17,8 @@ export class ProblemService { // Submit a new problem to be saved to the database. public submit$ = new Subject(); - public problem$: Observable = this.allParams$.pipe( + public problemResponse$: Observable = this.allParams$.pipe( + filter(allParams => allParams !== null), switchMap(({ params, queryParams }) => { const problemId = params.get("problemId"); if (!problemId) { @@ -28,6 +29,11 @@ export class ProblemService { shareReplay(1) ); + public problem$ = this.problemResponse$.pipe( + map(response => response?.problem ?? null), + shareReplay(1) + ); + public saveProblem$ = this.submit$.pipe( exhaustMap((problem) => this.http.post('/api/problem/new', problem).pipe( catchError((error) => { @@ -72,7 +78,7 @@ export class ProblemService { }; public firstProblemId$ = this.existingProblem$().pipe( - map(problem => problem?.id ?? null), + map(response => response?.problem?.id ?? null), ); private extractSearchParams(routeParams: ParamMap): HttpParams { From 978a35e4cb0fe644a07ee03ff1a9d37756c55ee2 Mon Sep 17 00:00:00 2001 From: Xander Vertegaal Date: Mon, 15 Sep 2025 19:19:18 +0200 Subject: [PATCH 10/42] Return KBs from backend; update in frontend --- backend/problem/models.py | 10 ++ backend/problem/urls.py | 3 +- backend/problem/views/problem.py | 111 ++++++++++++++---- .../annotation-input.component.ts | 56 ++++++--- .../knowledge-base-form.component.ts | 11 +- frontend/src/app/app.routes.ts | 5 + frontend/src/app/services/problem.service.ts | 11 +- frontend/src/app/types.ts | 18 ++- 8 files changed, 173 insertions(+), 52 deletions(-) diff --git a/backend/problem/models.py b/backend/problem/models.py index b44e81f..7650701 100644 --- a/backend/problem/models.py +++ b/backend/problem/models.py @@ -73,6 +73,8 @@ def serialize(self) -> dict: case _: serialized_extra_data = {} + kb_items = KnowledgeBase.objects.filter(problem=self) + return { "id": self.pk, "dataset": self.dataset, @@ -80,6 +82,7 @@ def serialize(self) -> dict: "hypothesis": self.hypothesis.text, "entailmentLabel": self.entailment_label, "extraData": serialized_extra_data, + "kbItems": [item.serialize() for item in kb_items], } @@ -105,3 +108,10 @@ class Relationship(models.TextChoices): on_delete=models.CASCADE, related_name="knowledge_bases", ) + + def serialize(self) -> dict: + return { + "entity1": self.entity1, + "entity2": self.entity2, + "relationship": self.relationship, + } diff --git a/backend/problem/urls.py b/backend/problem/urls.py index 7bc1d97..071bf8f 100644 --- a/backend/problem/urls.py +++ b/backend/problem/urls.py @@ -5,8 +5,7 @@ urlpatterns = [ - path("", ProblemView.as_view(), name="problem_view"), - path("new", ProblemView.as_view(), name="new_problem"), + path("", ProblemView.as_view(), name="problem_view"), path("parse", ParseView.as_view(), name="parse_view"), path("", ProblemView.as_view(), name="first_problem_view"), ] diff --git a/backend/problem/views/problem.py b/backend/problem/views/problem.py index 8c9e730..bddc19d 100644 --- a/backend/problem/views/problem.py +++ b/backend/problem/views/problem.py @@ -11,9 +11,8 @@ @dataclass class ProblemResponse: - id: int | None = None - index: int | None = None problem: Problem | None = None + index: int | None = None error: str | None = None first: str | None = None @@ -25,7 +24,6 @@ class ProblemResponse: def json_response(self, status=200) -> JsonResponse: return JsonResponse( { - "id": self.id, "index": self.index, "problem": self.problem.serialize() if self.problem else None, "error": self.error, @@ -50,6 +48,9 @@ def json_response(self, status=200) -> JsonResponse: class ProblemView(APIView): def get(self, request, problem_id: int | None = None) -> JsonResponse: + """ + If a Problem ID is provided, attempts to retrieve the requested Problem. Otherwise, simply returns the first problem of the QS. + """ filters = get_filters(request.query_params) qs = Problem.objects.all() @@ -73,9 +74,8 @@ def get(self, request, problem_id: int | None = None) -> JsonResponse: related_problem_ids = get_related_problem_ids(qs, problem_id) return ProblemResponse( - id=problem.pk if problem else None, - index=problem_index, problem=problem, + index=problem_index, first=related_problem_ids.first, previous=related_problem_ids.previous, next=related_problem_ids.next, @@ -84,6 +84,9 @@ def get(self, request, problem_id: int | None = None) -> JsonResponse: ).json_response(status=200) def post(self, request) -> JsonResponse: + """ + If a Problem ID is provided, attempts to update the associated Problem; else a new Problem is created. + """ parse_input = request.data try: @@ -95,28 +98,38 @@ def post(self, request) -> JsonResponse: error=str(e), ).json_response(status=400) - try: - problem = create_problem_from_input(validated_input) - except Exception as e: - logger.exception(f"Error saving problem: {e}") + problem: Problem | None = None + error: str | None = None + + if validated_input["id"] == "new": + try: + problem = create_problem_from_input(validated_input) + except Exception as e: + error = f"Error creating problem: {str(e)}" + else: + try: + problem = update_problem_from_input(validated_input) + except Exception as e: + error = f"Error updating problem: {str(e)}" + + if problem is None or error is not None: return SaveProblemResponse( id=None, - error=str(e), + error=error, ).json_response(status=500) - return SaveProblemResponse( - id=str(problem.pk), - error=None, - ).json_response() + return SaveProblemResponse(id=problem.pk, error=None).json_response() class KBItem(TypedDict): + id: str entity1: str relationship: str entity2: str class ParseInput(TypedDict): + id: str premises: list[str] hypothesis: str kbItems: list[KBItem] @@ -129,6 +142,9 @@ def validate_input(parse_input: dict) -> ParseInput: if not isinstance(parse_input, dict): raise ValueError("Input must be a dictionary") + if "id" not in parse_input or not isinstance(parse_input["id"], str): + raise ValueError("Missing or invalid 'id' field") + if "premises" not in parse_input or not isinstance(parse_input["premises"], list): raise ValueError("Missing or invalid 'premises' field") @@ -143,6 +159,8 @@ def validate_input(parse_input: dict) -> ParseInput: for item in parse_input["kbItems"]: if not isinstance(item, dict): raise ValueError("Each kbItem must be a dictionary") + if "id" not in item or not isinstance(item["id"], str): + raise ValueError("Missing or invalid 'id' in kbItem") if "entity1" not in item or not isinstance(item["entity1"], str): raise ValueError("Missing or invalid 'entity1' in kbItem") if "relationship" not in item or not isinstance(item["relationship"], str): @@ -153,6 +171,7 @@ def validate_input(parse_input: dict) -> ParseInput: raise ValueError("Missing or invalid 'entity2' in kbItem") return ParseInput( + id=parse_input["id"], premises=parse_input["premises"], hypothesis=parse_input["hypothesis"], kbItems=parse_input["kbItems"], @@ -185,19 +204,63 @@ def create_problem_from_input(parse_input: ParseInput) -> Problem: problem.premises.set(premise_sentences) - for item in parse_input["kbItems"]: + update_or_create_kb_items(problem=problem, kb_items=parse_input["kbItems"]) + + return problem + + +def update_or_create_kb_items(problem: Problem, kb_items: list[KBItem]) -> None: + kb_ids: list[str] = [] + for item in kb_items: + id = item.get("id") entity1 = item.get("entity1") relationship = item.get("relationship") entity2 = item.get("entity2") - try: - knowledge_base = KnowledgeBase.objects.create( - entity1=entity1, - relationship=relationship.lower(), - entity2=entity2, - problem=problem, - ) - except Exception as e: - raise ValueError(f"Error creating knowledge base items: {e}") + if id == "new": + try: + kb = KnowledgeBase.objects.create( + entity1=entity1, + relationship=relationship.lower(), + entity2=entity2, + problem=problem, + ) + kb_ids.append(kb.pk) + except Exception as e: + raise ValueError(f"Error creating knowledge base items: {e}.") + else: + try: + kb = KnowledgeBase.objects.get(id=id, problem=problem) + except KnowledgeBase.DoesNotExist: + raise ValueError(f"Unable to find knowledge base item with id {id}.") + kb.entity1 = entity1 + kb.relationship = relationship.lower() + kb.entity2 = entity2 + kb.save() + kb_ids.append(kb.pk) + + # Delete existing knowledge bases associated to this problem that are + # not included in the input. + KnowledgeBase.objects.filter(problem=problem).exclude(id__in=kb_ids).delete() + + +def update_problem_from_input(parse_input: ParseInput) -> Problem: + try: + problem = Problem.objects.get(id=parse_input["id"]) + except Problem.DoesNotExist: + raise ValueError(f"Cannot find Problem with ID: {parse_input["id"]}") + + problem.hypothesis = Sentence.objects.get_or_create(text=parse_input["hypothesis"])[ + 0 + ] + + premises: list[Sentence] = [] + for input_premise in parse_input["premises"]: + premise = Sentence.objects.get_or_create(text=input_premise)[0] + premises.append(premise) + + problem.premises.set(premises) + + update_or_create_kb_items(problem, parse_input["kbItems"]) return problem diff --git a/frontend/src/app/annotate/annotation-input/annotation-input.component.ts b/frontend/src/app/annotate/annotation-input/annotation-input.component.ts index 3ce6f70..4321a36 100644 --- a/frontend/src/app/annotate/annotation-input/annotation-input.component.ts +++ b/frontend/src/app/annotate/annotation-input/annotation-input.component.ts @@ -9,15 +9,12 @@ import { Validators, } from "@angular/forms"; import { PremisesFormComponent } from "./premises-form/premises-form.component"; -import { - KnowledgeBaseFormComponent, - KnowledgeBaseRelationship, -} from "./knowledge-base-form/knowledge-base-form.component"; +import { KnowledgeBaseFormComponent } from "./knowledge-base-form/knowledge-base-form.component"; import { takeUntilDestroyed } from "@angular/core/rxjs-interop"; -import { Dataset, ProblemResponse } from "../../types"; -import { faCheck, faExclamationCircle, faFloppyDisk, faTrash, faTree } from "@fortawesome/free-solid-svg-icons"; +import { Dataset, KnowledgeBaseRelationship, Problem } from "../../types"; +import { faCheck, faExclamationCircle, faFloppyDisk, faTrash, faTree, faWrench } from "@fortawesome/free-solid-svg-icons"; import { ProblemDetailsComponent } from "./problem-details/problem-details.component"; -import { combineLatest, Subject } from "rxjs"; +import { map, Subject } from "rxjs"; import { ActivatedRoute, Router } from "@angular/router"; import { ProblemService } from "@/services/problem.service"; import { ParseService } from "@/services/parse.service"; @@ -26,12 +23,14 @@ import { AppModeService } from "@/services/app-mode.service"; import { ToastService } from "@/services/toast.service"; export type ParseInputForm = FormGroup<{ + id: FormControl; premises: FormArray>; hypothesis: FormControl; kbItems: FormArray; }>; type KnowledgeBaseItemsForm = FormGroup<{ + id: FormControl; entity1: FormControl; relationship: FormControl; entity2: FormControl; @@ -64,7 +63,8 @@ export class AnnotationInputComponent implements OnInit { private toastService = inject(ToastService); public form: ParseInputForm | null = null; - public problem: ProblemResponse | null = null; + + public problem$ = this.problemService.problem$; public submit$ = new Subject(); @@ -135,11 +135,15 @@ export class AnnotationInputComponent implements OnInit { this.problemService.submit$.next(input); } - private navigateToNewProblem(problem: ProblemResponse | null): void { - if (!problem?.problem) { + public editProblem(): void { + this.router.navigate(["edit"], { relativeTo: this.route, queryParamsHandling: "preserve" }); + } + + private navigateToNewProblem(problem: Problem | null): void { + if (!problem) { return; } - const incomingProblemId = problem?.id?.toString(); + const incomingProblemId = problem.id?.toString(); const currentProblemId = this.route.snapshot.paramMap.get("problemId"); if (incomingProblemId !== currentProblemId) { @@ -149,11 +153,16 @@ export class AnnotationInputComponent implements OnInit { } } - private buildForm(response: ProblemResponse): ParseInputForm { - const premises = response.problem?.premises || []; - const hypothesis = response.problem?.hypothesis || ""; + private buildForm(problem: Problem): ParseInputForm { + const id = problem.id; + const premises = problem.premises || []; + const hypothesis = problem.hypothesis || ""; + const kbItems = this.buildKbForms(problem.kbItems); return new FormGroup({ + id: new FormControl(id, { + nonNullable: true + }), premises: new FormArray( premises.map( (premise) => @@ -167,7 +176,24 @@ export class AnnotationInputComponent implements OnInit { validators: [Validators.required], nonNullable: true, }), - kbItems: new FormArray([]), + kbItems: new FormArray(kbItems), }); } + + private buildKbForms(inputKbItems: Problem['kbItems']): KnowledgeBaseItemsForm[] { + return inputKbItems.map(item => new FormGroup({ + id: new FormControl(item.id, { + nonNullable: true + }), + entity1: new FormControl(item.entity1, { + nonNullable: true + }), + entity2: new FormControl(item.entity2, { + nonNullable: true + }), + relationship: new FormControl(item.relationship, { + nonNullable: true + }) + })); + } } diff --git a/frontend/src/app/annotate/annotation-input/knowledge-base-form/knowledge-base-form.component.ts b/frontend/src/app/annotate/annotation-input/knowledge-base-form/knowledge-base-form.component.ts index a1dacd3..5d4006c 100644 --- a/frontend/src/app/annotate/annotation-input/knowledge-base-form/knowledge-base-form.component.ts +++ b/frontend/src/app/annotate/annotation-input/knowledge-base-form/knowledge-base-form.component.ts @@ -6,13 +6,9 @@ import { FontAwesomeModule } from "@fortawesome/angular-fontawesome"; import { faPlus, faTrash } from "@fortawesome/free-solid-svg-icons"; import { ParseInputForm } from "../annotation-input.component"; import { AppModeService } from "@/services/app-mode.service"; +import { KnowledgeBaseRelationship } from "@/types"; + -export enum KnowledgeBaseRelationship { - EQUAL = "EQUAL", - NOT_EQUAL = "NOT_EQUAL", - SUBSET = "SUBSET", - SUPERSET = "SUPERSET", -} const relationshipDisplayMapping: Record = { EQUAL: "is equal to", @@ -42,6 +38,9 @@ export class KnowledgeBaseFormComponent { public addKnowledgeBaseItem(): void { const newItem = new FormGroup({ + id: new FormControl("new", { + nonNullable: true + }), entity1: new FormControl("", { validators: [Validators.required], nonNullable: true, diff --git a/frontend/src/app/app.routes.ts b/frontend/src/app/app.routes.ts index 49cc382..5373800 100644 --- a/frontend/src/app/app.routes.ts +++ b/frontend/src/app/app.routes.ts @@ -41,6 +41,11 @@ const routes: Routes = [ canActivate: [LoggedOnGuard], component: UserSettingsComponent, }, + { + path: "annotate/:problemId/edit", + canActivate: [LoggedOnGuard], + component: AnnotateComponent, + }, { path: "annotate/:problemId", canActivate: [LoggedOnGuard], diff --git a/frontend/src/app/services/problem.service.ts b/frontend/src/app/services/problem.service.ts index fa42d32..2cef86b 100644 --- a/frontend/src/app/services/problem.service.ts +++ b/frontend/src/app/services/problem.service.ts @@ -3,8 +3,13 @@ import { ProblemResponse, SaveProblemResponse, Dataset, EntailmentLabel } from " import { HttpClient, HttpParams } from "@angular/common/http"; import { Injectable, inject } from "@angular/core"; import { ParamMap } from "@angular/router"; -import { Subject, Observable, switchMap, of, shareReplay, exhaustMap, catchError, map } from "rxjs"; +import { Subject, Observable, switchMap, of, shareReplay, exhaustMap, catchError, map, BehaviorSubject, filter } from "rxjs"; +interface AllParams { + params: ParamMap; + queryParams: ParamMap; + edit: boolean; +} @Injectable({ providedIn: 'root' @@ -12,7 +17,7 @@ import { Subject, Observable, switchMap, of, shareReplay, exhaustMap, catchError export class ProblemService { private http = inject(HttpClient); - public allParams$ = new Subject<{ params: ParamMap, queryParams: ParamMap; }>(); + public allParams$ = new BehaviorSubject(null); // Submit a new problem to be saved to the database. public submit$ = new Subject(); @@ -45,7 +50,6 @@ export class ProblemService { private newProblem$(): Observable { return of({ - id: "new", index: null, firstProblemId: null, lastProblemId: null, @@ -60,6 +64,7 @@ export class ProblemService { premises: [], entailmentLabel: EntailmentLabel.UNKNOWN, extraData: null, + kbItems: [] }, }); diff --git a/frontend/src/app/types.ts b/frontend/src/app/types.ts index 5f5a7ba..ebf781b 100644 --- a/frontend/src/app/types.ts +++ b/frontend/src/app/types.ts @@ -23,11 +23,26 @@ interface SNLIData { label5: string; } +export enum KnowledgeBaseRelationship { + EQUAL = "EQUAL", + NOT_EQUAL = "NOT_EQUAL", + SUBSET = "SUBSET", + SUPERSET = "SUPERSET", +} + +interface KnowledgeBaseItem { + id: string; + entity1: string; + relationship: KnowledgeBaseRelationship; + entity2: string; +} + interface ProblemBase { id: string; premises: string[]; hypothesis: string | null; entailmentLabel: EntailmentLabel; + kbItems: KnowledgeBaseItem[]; } interface SickProblem extends ProblemBase { @@ -50,14 +65,13 @@ interface UserProblem extends ProblemBase { extraData: null; } -type Problem = SickProblem | FracasProblem | SNLIProblem | UserProblem; +export type Problem = SickProblem | FracasProblem | SNLIProblem | UserProblem; interface BaseResponse { error: string | null; } export interface ProblemResponse extends BaseResponse { - id: string | null; index: number | null; problem: Problem | null; firstProblemId: string | null; From 5d83ce68425507e48a569899901e176c4e249caf Mon Sep 17 00:00:00 2001 From: Xander Vertegaal Date: Tue, 16 Sep 2025 11:17:06 +0200 Subject: [PATCH 11/42] Use consistent str for ID --- backend/problem/models.py | 3 ++- backend/problem/problem_details.py | 2 +- backend/problem/views/problem.py | 23 ++++++++++++----------- 3 files changed, 15 insertions(+), 13 deletions(-) diff --git a/backend/problem/models.py b/backend/problem/models.py index 7650701..60bb5f5 100644 --- a/backend/problem/models.py +++ b/backend/problem/models.py @@ -76,7 +76,7 @@ def serialize(self) -> dict: kb_items = KnowledgeBase.objects.filter(problem=self) return { - "id": self.pk, + "id": str(self.pk), "dataset": self.dataset, "premises": [premise.text for premise in self.premises.all()], "hypothesis": self.hypothesis.text, @@ -111,6 +111,7 @@ class Relationship(models.TextChoices): def serialize(self) -> dict: return { + "id": str(self.pk), "entity1": self.entity1, "entity2": self.entity2, "relationship": self.relationship, diff --git a/backend/problem/problem_details.py b/backend/problem/problem_details.py index fd89ba2..703f426 100644 --- a/backend/problem/problem_details.py +++ b/backend/problem/problem_details.py @@ -19,7 +19,7 @@ class RelatedProblemIds: def get_related_problem_ids( problem_qs: QuerySet[Problem], - problem_id: Optional[int], + problem_id: Optional[str], ) -> RelatedProblemIds: """ Retrieves the IDs of surrounding problem objects diff --git a/backend/problem/views/problem.py b/backend/problem/views/problem.py index bddc19d..adcd080 100644 --- a/backend/problem/views/problem.py +++ b/backend/problem/views/problem.py @@ -47,7 +47,7 @@ def json_response(self, status=200) -> JsonResponse: class ProblemView(APIView): - def get(self, request, problem_id: int | None = None) -> JsonResponse: + def get(self, request, problem_id: str | None = None) -> JsonResponse: """ If a Problem ID is provided, attempts to retrieve the requested Problem. Otherwise, simply returns the first problem of the QS. """ @@ -83,9 +83,10 @@ def get(self, request, problem_id: int | None = None) -> JsonResponse: total=related_problem_ids.total, ).json_response(status=200) - def post(self, request) -> JsonResponse: + def post(self, request, problem_id: str) -> JsonResponse: """ - If a Problem ID is provided, attempts to update the associated Problem; else a new Problem is created. + If the Problem ID is "new", attempts to create a new Problem; + else the associated Problem is updated. """ parse_input = request.data @@ -101,7 +102,7 @@ def post(self, request) -> JsonResponse: problem: Problem | None = None error: str | None = None - if validated_input["id"] == "new": + if problem_id == "new": try: problem = create_problem_from_input(validated_input) except Exception as e: @@ -165,7 +166,7 @@ def validate_input(parse_input: dict) -> ParseInput: raise ValueError("Missing or invalid 'entity1' in kbItem") if "relationship" not in item or not isinstance(item["relationship"], str): raise ValueError("Missing or invalid 'relationship' in kbItem") - if item["relationship"].lower() not in KnowledgeBase.Relationship.values: + if item["relationship"] not in KnowledgeBase.Relationship.values: raise ValueError(f"Invalid 'relationship' in kbItem.") if "entity2" not in item or not isinstance(item["entity2"], str): raise ValueError("Missing or invalid 'entity2' in kbItem") @@ -221,7 +222,7 @@ def update_or_create_kb_items(problem: Problem, kb_items: list[KBItem]) -> None: try: kb = KnowledgeBase.objects.create( entity1=entity1, - relationship=relationship.lower(), + relationship=relationship, entity2=entity2, problem=problem, ) @@ -230,18 +231,18 @@ def update_or_create_kb_items(problem: Problem, kb_items: list[KBItem]) -> None: raise ValueError(f"Error creating knowledge base items: {e}.") else: try: - kb = KnowledgeBase.objects.get(id=id, problem=problem) + kb = KnowledgeBase.objects.get(id=id, problem_id=problem.pk) except KnowledgeBase.DoesNotExist: raise ValueError(f"Unable to find knowledge base item with id {id}.") kb.entity1 = entity1 - kb.relationship = relationship.lower() + kb.relationship = relationship kb.entity2 = entity2 kb.save() kb_ids.append(kb.pk) - # Delete existing knowledge bases associated to this problem that are - # not included in the input. - KnowledgeBase.objects.filter(problem=problem).exclude(id__in=kb_ids).delete() + # Delete existing knowledge bases associated to this problem that are + # not included in the input. + KnowledgeBase.objects.filter(problem_id=problem.pk).exclude(id__in=kb_ids).delete() def update_problem_from_input(parse_input: ParseInput) -> Problem: From 289c6228b40bc064d502c75e4b6093b8fc919343 Mon Sep 17 00:00:00 2001 From: Xander Vertegaal Date: Tue, 16 Sep 2025 11:17:34 +0200 Subject: [PATCH 12/42] Consistent casing for KB Relationship --- .../knowledge-base-form/knowledge-base-form.component.ts | 8 ++++---- frontend/src/app/types.ts | 8 ++++---- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/frontend/src/app/annotate/annotation-input/knowledge-base-form/knowledge-base-form.component.ts b/frontend/src/app/annotate/annotation-input/knowledge-base-form/knowledge-base-form.component.ts index 5d4006c..786cf94 100644 --- a/frontend/src/app/annotate/annotation-input/knowledge-base-form/knowledge-base-form.component.ts +++ b/frontend/src/app/annotate/annotation-input/knowledge-base-form/knowledge-base-form.component.ts @@ -11,10 +11,10 @@ import { KnowledgeBaseRelationship } from "@/types"; const relationshipDisplayMapping: Record = { - EQUAL: "is equal to", - NOT_EQUAL: "is not equal to", - SUBSET: "is a subset of", - SUPERSET: "is a superset of", + equal: "is equal to", + not_equal: "is not equal to", + subset: "is a subset of", + superset: "is a superset of", }; @Component({ diff --git a/frontend/src/app/types.ts b/frontend/src/app/types.ts index ebf781b..8147b75 100644 --- a/frontend/src/app/types.ts +++ b/frontend/src/app/types.ts @@ -24,10 +24,10 @@ interface SNLIData { } export enum KnowledgeBaseRelationship { - EQUAL = "EQUAL", - NOT_EQUAL = "NOT_EQUAL", - SUBSET = "SUBSET", - SUPERSET = "SUPERSET", + EQUAL = "equal", + NOT_EQUAL = "not_equal", + SUBSET = "subset", + SUPERSET = "superset", } interface KnowledgeBaseItem { From fb23d197cfe948667159d88dbf9f80ab3dfe7c3b Mon Sep 17 00:00:00 2001 From: Xander Vertegaal Date: Tue, 16 Sep 2025 11:18:06 +0200 Subject: [PATCH 13/42] Display KB items in browsing mode --- .../knowledge-base-form.component.html | 26 +++++++++---------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/frontend/src/app/annotate/annotation-input/knowledge-base-form/knowledge-base-form.component.html b/frontend/src/app/annotate/annotation-input/knowledge-base-form/knowledge-base-form.component.html index 983a6f8..57f175e 100644 --- a/frontend/src/app/annotate/annotation-input/knowledge-base-form/knowledge-base-form.component.html +++ b/frontend/src/app/annotate/annotation-input/knowledge-base-form/knowledge-base-form.component.html @@ -28,19 +28,16 @@
    @for (kbItem of kbControls; let i = $index; track $index) {
  • + @if (viewMode === 'add' || viewMode === 'edit') {
    - @@ -52,16 +49,11 @@ - @if (viewMode === 'add' || viewMode === 'edit') { - }
    + } @else { +

    + "{{ kbItem.controls.entity1.value }}" + {{ + getRelationshipTypeName(kbItem.controls.relationship.value) + }} + "{{ kbItem.controls.entity2.value }}" +

    + }
    • } @empty {
  • From 830a3d8ce29a935f950bfe3f66b605d8cc5408d1 Mon Sep 17 00:00:00 2001 From: Xander Vertegaal Date: Tue, 16 Sep 2025 11:18:34 +0200 Subject: [PATCH 14/42] Cleanup --- .../annotate/annotation-input/annotation-input.component.ts | 2 +- frontend/src/app/services/problem.service.ts | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/frontend/src/app/annotate/annotation-input/annotation-input.component.ts b/frontend/src/app/annotate/annotation-input/annotation-input.component.ts index 4321a36..db02fbb 100644 --- a/frontend/src/app/annotate/annotation-input/annotation-input.component.ts +++ b/frontend/src/app/annotate/annotation-input/annotation-input.component.ts @@ -100,6 +100,7 @@ export class AnnotationInputComponent implements OnInit { body: $localize`There was an error saving the problem: ${response.error}`, type: 'danger' }); + return; } this.toastService.show({ @@ -179,7 +180,6 @@ export class AnnotationInputComponent implements OnInit { kbItems: new FormArray(kbItems), }); } - private buildKbForms(inputKbItems: Problem['kbItems']): KnowledgeBaseItemsForm[] { return inputKbItems.map(item => new FormGroup({ id: new FormControl(item.id, { diff --git a/frontend/src/app/services/problem.service.ts b/frontend/src/app/services/problem.service.ts index 2cef86b..3a748bd 100644 --- a/frontend/src/app/services/problem.service.ts +++ b/frontend/src/app/services/problem.service.ts @@ -40,7 +40,7 @@ export class ProblemService { ); public saveProblem$ = this.submit$.pipe( - exhaustMap((problem) => this.http.post('/api/problem/new', problem).pipe( + exhaustMap((problem) => this.http.post(`/api/problem/${problem.id}`, problem).pipe( catchError((error) => { console.error('Error saving problem:', error); return of({ id: null, error: 'Failed to save problem' }); @@ -84,6 +84,7 @@ export class ProblemService { public firstProblemId$ = this.existingProblem$().pipe( map(response => response?.problem?.id ?? null), + shareReplay(1), ); private extractSearchParams(routeParams: ParamMap): HttpParams { From 49b03d40a0e25cdf9a1e01aa03d700b25fe9296f Mon Sep 17 00:00:00 2001 From: Xander Vertegaal Date: Tue, 16 Sep 2025 12:14:41 +0200 Subject: [PATCH 15/42] Reuse sentences when creating new problem --- backend/problem/views/problem.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/backend/problem/views/problem.py b/backend/problem/views/problem.py index adcd080..41c900c 100644 --- a/backend/problem/views/problem.py +++ b/backend/problem/views/problem.py @@ -185,13 +185,13 @@ def create_problem_from_input(parse_input: ParseInput) -> Problem: """ try: premise_sentences = [ - Sentence.objects.create(text=premise) for premise in parse_input["premises"] + Sentence.objects.get_or_create(text=premise)[0] for premise in parse_input["premises"] ] except Exception as e: raise ValueError(f"Error creating premise sentences: {e}") try: - hypothesis_sentence = Sentence.objects.create(text=parse_input["hypothesis"]) + hypothesis_sentence = Sentence.objects.get_or_create(text=parse_input["hypothesis"])[0] except Exception as e: raise ValueError(f"Error creating hypothesis sentence: {e}") From cf339b85a83e74f680235ba1647abfa49b5a1024 Mon Sep 17 00:00:00 2001 From: Xander Vertegaal Date: Tue, 16 Sep 2025 12:15:23 +0200 Subject: [PATCH 16/42] Fix failing tests --- .../app/annotate/annotate.component.spec.ts | 5 ++++- .../knowledge-base-form.component.spec.ts | 4 ++++ .../premises-form.component.html | 11 ++++++++-- .../src/app/services/app-mode.service.spec.ts | 20 +++++++++++-------- 4 files changed, 29 insertions(+), 11 deletions(-) diff --git a/frontend/src/app/annotate/annotate.component.spec.ts b/frontend/src/app/annotate/annotate.component.spec.ts index 93e35c4..227ca5f 100644 --- a/frontend/src/app/annotate/annotate.component.spec.ts +++ b/frontend/src/app/annotate/annotate.component.spec.ts @@ -5,6 +5,7 @@ import { ActivatedRoute } from "@angular/router"; import { of } from "rxjs"; import { provideHttpClient } from "@angular/common/http"; import { provideHttpClientTesting } from "@angular/common/http/testing"; +import { CommonModule } from "@angular/common"; describe("AnnotateComponent", () => { let component: AnnotateComponent; @@ -12,13 +13,15 @@ describe("AnnotateComponent", () => { beforeEach(async () => { await TestBed.configureTestingModule({ - imports: [AnnotateComponent], + imports: [AnnotateComponent, CommonModule], providers: [ provideHttpClient(), provideHttpClientTesting(), { provide: ActivatedRoute, useValue: { + url: of([]), + paramMap: of({}), queryParamMap: of({}) } } diff --git a/frontend/src/app/annotate/annotation-input/knowledge-base-form/knowledge-base-form.component.spec.ts b/frontend/src/app/annotate/annotation-input/knowledge-base-form/knowledge-base-form.component.spec.ts index 0c0da9c..7f1431b 100644 --- a/frontend/src/app/annotate/annotation-input/knowledge-base-form/knowledge-base-form.component.spec.ts +++ b/frontend/src/app/annotate/annotation-input/knowledge-base-form/knowledge-base-form.component.spec.ts @@ -2,6 +2,7 @@ import { ComponentFixture, TestBed } from "@angular/core/testing"; import { KnowledgeBaseFormComponent } from "./knowledge-base-form.component"; import { FormArray, FormGroup } from "@angular/forms"; +import { provideHttpClient } from "@angular/common/http"; describe("KnowledgeBaseFormComponent", () => { let component: KnowledgeBaseFormComponent; @@ -10,6 +11,9 @@ describe("KnowledgeBaseFormComponent", () => { beforeEach(async () => { await TestBed.configureTestingModule({ imports: [KnowledgeBaseFormComponent], + providers: [ + provideHttpClient(), + ] }).compileComponents(); fixture = TestBed.createComponent(KnowledgeBaseFormComponent); diff --git a/frontend/src/app/annotate/annotation-input/premises-form/premises-form.component.html b/frontend/src/app/annotate/annotation-input/premises-form/premises-form.component.html index 99599af..1baa397 100644 --- a/frontend/src/app/annotate/annotation-input/premises-form/premises-form.component.html +++ b/frontend/src/app/annotate/annotation-input/premises-form/premises-form.component.html @@ -32,7 +32,7 @@ [id]="'premise-' + i" type="text" [class]=" - (viewMode === 'add' || viewMode === 'edit') + viewMode === 'add' || viewMode === 'edit' ? 'form-control' : 'form-control-plaintext' " @@ -76,9 +76,16 @@ id="hypothesis" type="text" [class]=" - (viewMode === 'add' || viewMode === 'edit') ? 'form-control' : 'form-control-plaintext' + viewMode === 'add' || viewMode === 'edit' + ? 'form-control' + : 'form-control-plaintext' + " + [class.is-invalid]=" + form().controls.hypothesis.touched && + form().controls.hypothesis.invalid " [formControl]="form().controls.hypothesis" /> +

    Each problem must have a hypothesis.

    diff --git a/frontend/src/app/services/app-mode.service.spec.ts b/frontend/src/app/services/app-mode.service.spec.ts index 6428aa1..c2e4809 100644 --- a/frontend/src/app/services/app-mode.service.spec.ts +++ b/frontend/src/app/services/app-mode.service.spec.ts @@ -1,16 +1,20 @@ import { TestBed } from '@angular/core/testing'; import { AppModeService } from './app-mode.service'; +import { provideHttpClient } from '@angular/common/http'; +import { provideHttpClientTesting } from '@angular/common/http/testing'; describe('AppModeService', () => { - let service: AppModeService; + let service: AppModeService; - beforeEach(() => { - TestBed.configureTestingModule({}); - service = TestBed.inject(AppModeService); - }); + beforeEach(() => { + TestBed.configureTestingModule({ + providers: [provideHttpClient(), provideHttpClientTesting()], + }); + service = TestBed.inject(AppModeService); + }); - it('should be created', () => { - expect(service).toBeTruthy(); - }); + it('should be created', () => { + expect(service).toBeTruthy(); + }); }); From 9d70ffadcd6e97a2d92a2bd31cebb12e6afa82a0 Mon Sep 17 00:00:00 2001 From: Xander Vertegaal Date: Tue, 16 Sep 2025 12:15:40 +0200 Subject: [PATCH 17/42] Show error message upon omitting hypothesis --- .../app/annotate/annotation-input/annotation-input.component.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/frontend/src/app/annotate/annotation-input/annotation-input.component.ts b/frontend/src/app/annotate/annotation-input/annotation-input.component.ts index db02fbb..e6f5b45 100644 --- a/frontend/src/app/annotate/annotation-input/annotation-input.component.ts +++ b/frontend/src/app/annotate/annotation-input/annotation-input.component.ts @@ -129,6 +129,7 @@ export class AnnotationInputComponent implements OnInit { } public saveProblem(): void { + this.form?.markAllAsTouched(); if (!this.form || this.form.invalid) { return; } From 51b235d0a8b74fa0efeed952535ec4069a8732aa Mon Sep 17 00:00:00 2001 From: Xander Vertegaal Date: Tue, 16 Sep 2025 12:59:05 +0200 Subject: [PATCH 18/42] Add more tests --- .../annotation-input.component.spec.ts | 168 ++++++++++++++- .../problem-details.component.spec.ts | 106 +++++++++- .../src/app/services/app-mode.service.spec.ts | 126 +++++++++++- frontend/src/app/services/app-mode.service.ts | 6 +- .../src/app/services/problem.service.spec.ts | 194 +++++++++++++++++- 5 files changed, 577 insertions(+), 23 deletions(-) diff --git a/frontend/src/app/annotate/annotation-input/annotation-input.component.spec.ts b/frontend/src/app/annotate/annotation-input/annotation-input.component.spec.ts index 0e14e8d..702a7f6 100644 --- a/frontend/src/app/annotate/annotation-input/annotation-input.component.spec.ts +++ b/frontend/src/app/annotate/annotation-input/annotation-input.component.spec.ts @@ -1,30 +1,41 @@ import { ComponentFixture, TestBed } from "@angular/core/testing"; +import { FormArray, FormGroup } from "@angular/forms"; import { AnnotationInputComponent } from "./annotation-input.component"; import { provideHttpClientTesting } from "@angular/common/http/testing"; import { provideHttpClient } from "@angular/common/http"; -import { ActivatedRoute } from "@angular/router"; +import { ActivatedRoute, Router } from "@angular/router"; import { of } from "rxjs"; +import { Dataset, KnowledgeBaseRelationship, Problem, EntailmentLabel } from "../../types"; describe("AnnotationInputComponent", () => { let component: AnnotationInputComponent; let fixture: ComponentFixture; + let mockRouter: jasmine.SpyObj; + let mockActivatedRoute: any; beforeEach(async () => { + const routerSpy = jasmine.createSpyObj('Router', ['navigate']); + mockActivatedRoute = { + params: of({ problemId: "1" }), + snapshot: { + paramMap: { + get: jasmine.createSpy('get').and.returnValue('current-problem-id') + } + } + }; + await TestBed.configureTestingModule({ imports: [AnnotationInputComponent], providers: [ provideHttpClient(), provideHttpClientTesting(), - { - provide: ActivatedRoute, - useValue: { - params: of({ problemId: "1" }), - }, - }, + { provide: ActivatedRoute, useValue: mockActivatedRoute }, + { provide: Router, useValue: routerSpy } ], }).compileComponents(); + mockRouter = TestBed.inject(Router) as jasmine.SpyObj; fixture = TestBed.createComponent(AnnotationInputComponent); component = fixture.componentInstance; fixture.detectChanges(); @@ -33,4 +44,147 @@ describe("AnnotationInputComponent", () => { it("should create", () => { expect(component).toBeTruthy(); }); + + describe('buildForm', () => { + it('should build form with correct structure and values from problem data', () => { + const mockProblem: Problem = { + id: "test-123", + premises: ["First premise", "Second premise"], + hypothesis: "Test hypothesis", + entailmentLabel: EntailmentLabel.ENTAILMENT, + kbItems: [ + { + id: "kb1", + entity1: "cat", + entity2: "animal", + relationship: KnowledgeBaseRelationship.SUBSET + }, + { + id: "kb2", + entity1: "dog", + entity2: "pet", + relationship: KnowledgeBaseRelationship.EQUAL + } + ], + dataset: Dataset.USER, + extraData: null + }; + + // Access private method. + const form = component['buildForm'](mockProblem); + + // Test form structure + expect(form).toBeTruthy(); + expect(form.get('id')?.value).toBe('test-123'); + expect(form.get('hypothesis')?.value).toBe('Test hypothesis'); + + // Test premises array + const premisesArray = form.get('premises') as FormArray; + expect(premisesArray.length).toBe(2); + expect(premisesArray.at(0).value).toBe('First premise'); + expect(premisesArray.at(1).value).toBe('Second premise'); + + // Test knowledge base items + const kbItemsArray = form.get('kbItems') as FormArray; + expect(kbItemsArray.length).toBe(2); + + const firstKbForm = kbItemsArray.at(0) as FormGroup; + expect(firstKbForm.get('id')?.value).toBe('kb1'); + expect(firstKbForm.get('entity1')?.value).toBe('cat'); + expect(firstKbForm.get('entity2')?.value).toBe('animal'); + expect(firstKbForm.get('relationship')?.value).toBe(KnowledgeBaseRelationship.SUBSET); + + const secondKbForm = kbItemsArray.at(1) as FormGroup; + expect(secondKbForm.get('id')?.value).toBe('kb2'); + expect(secondKbForm.get('entity1')?.value).toBe('dog'); + expect(secondKbForm.get('entity2')?.value).toBe('pet'); + expect(secondKbForm.get('relationship')?.value).toBe(KnowledgeBaseRelationship.EQUAL); + }); + + it('should handle empty premises and kbItems arrays', () => { + const mockProblem: Problem = { + id: "empty-test", + premises: [], + hypothesis: "Empty test hypothesis", + entailmentLabel: EntailmentLabel.NEUTRAL, + kbItems: [], + dataset: Dataset.USER, + extraData: null + }; + + const form = component['buildForm'](mockProblem); + + expect(form.get('id')?.value).toBe('empty-test'); + expect(form.get('hypothesis')?.value).toBe('Empty test hypothesis'); + + const premisesArray = form.get('premises') as FormArray; + expect(premisesArray.length).toBe(0); + + const kbItemsArray = form.get('kbItems') as FormArray; + expect(kbItemsArray.length).toBe(0); + }); + + it('should create form controls with required validators', () => { + const mockProblem: Problem = { + id: "validator-test", + premises: ["Test premise"], + hypothesis: "Test hypothesis", + entailmentLabel: EntailmentLabel.CONTRADICTION, + kbItems: [], + dataset: Dataset.USER, + extraData: null + }; + + const form = component['buildForm'](mockProblem); + + const hypothesisControl = form.get('hypothesis'); + expect(hypothesisControl?.hasError('required')).toBeFalsy(); + hypothesisControl?.setValue(''); + expect(hypothesisControl?.hasError('required')).toBeTruthy(); + }); + }); + + describe('navigateToNewProblem', () => { + it('should navigate when problem ID is different from current route', () => { + const mockProblem: Problem = { + id: "new-problem-id", + premises: [], + hypothesis: "", + entailmentLabel: EntailmentLabel.UNKNOWN, + kbItems: [], + dataset: Dataset.USER, + extraData: null + }; + + component['navigateToNewProblem'](mockProblem); + + expect(mockRouter.navigate).toHaveBeenCalledWith( + ['/annotate', 'new-problem-id'], + { queryParamsHandling: 'preserve' } + ); + }); + + it('should not navigate when problem ID matches current route', () => { + const mockProblem: Problem = { + id: "current-problem-id", + premises: [], + hypothesis: "", + entailmentLabel: EntailmentLabel.UNKNOWN, + kbItems: [], + dataset: Dataset.USER, + extraData: null + }; + + component['navigateToNewProblem'](mockProblem); + + expect(mockRouter.navigate).not.toHaveBeenCalled(); + }); + + it('should not navigate when problem is null', () => { + // Call the private method with null + component['navigateToNewProblem'](null); + + expect(mockRouter.navigate).not.toHaveBeenCalled(); + }); + }); }); diff --git a/frontend/src/app/annotate/annotation-input/problem-details/problem-details.component.spec.ts b/frontend/src/app/annotate/annotation-input/problem-details/problem-details.component.spec.ts index 0e010ca..dbedf86 100644 --- a/frontend/src/app/annotate/annotation-input/problem-details/problem-details.component.spec.ts +++ b/frontend/src/app/annotate/annotation-input/problem-details/problem-details.component.spec.ts @@ -1,7 +1,22 @@ import { ComponentFixture, TestBed } from "@angular/core/testing"; import { ProblemDetailsComponent } from "./problem-details.component"; -import { Dataset } from "../../../types"; +import { Dataset, EntailmentLabel, Problem } from "../../../types"; + +const createMockProblem = ( + id: string, + dataset: Dataset, + entailmentLabel: EntailmentLabel, + extraData: any = {}, +): Problem => ({ + id, + dataset, + entailmentLabel, + premises: ["premise"], + hypothesis: "hypothesis", + kbItems: [], + extraData, +}); describe("ProblemDetailsComponent", () => { let component: ProblemDetailsComponent; @@ -16,9 +31,7 @@ describe("ProblemDetailsComponent", () => { component = fixture.componentInstance; const componentRef = fixture.componentRef; componentRef.setInput("problem", { - problem: { - id: 1 - }, + id: "1", dataset: Dataset.SICK, }); fixture.detectChanges(); @@ -27,4 +40,89 @@ describe("ProblemDetailsComponent", () => { it("should create", () => { expect(component).toBeTruthy(); }); + + describe("with SICK dataset problem", () => { + beforeEach(() => { + const problem = createMockProblem( + "1", + Dataset.SICK, + EntailmentLabel.ENTAILMENT, + ); + fixture.componentRef.setInput("problem", problem); + fixture.detectChanges(); + }); + + it("should extract correct problem details", () => { + expect(component.problemDetails()).toEqual({ + problemId: "1", + dataset: Dataset.SICK, + entailmentLabel: EntailmentLabel.ENTAILMENT, + section: null, + subsection: null, + comment: null, + }); + }); + + it("should compute sectionString as null", () => { + expect(component.sectionString()).toBeNull(); + }); + }); + + describe("with FRACAS dataset problem", () => { + beforeEach(() => { + const problem = createMockProblem( + "2", + Dataset.FRACAS, + EntailmentLabel.CONTRADICTION, + { + sectionName: "Quantifiers", + subsectionName: "Some", + note: "A test note", + }, + ); + fixture.componentRef.setInput("problem", problem); + fixture.detectChanges(); + }); + + it("should extract correct problem details", () => { + expect(component.problemDetails()).toEqual({ + problemId: "2", + dataset: Dataset.FRACAS, + entailmentLabel: EntailmentLabel.CONTRADICTION, + section: "Quantifiers", + subsection: "Some", + comment: "A test note", + }); + }); + + it("should compute sectionString with section and subsection", () => { + expect(component.sectionString()).toBe("Quantifiers | Some"); + }); + }); + + describe("sectionString computation", () => { + it("should show only section when subsection is null", () => { + const problem = createMockProblem( + "5", + Dataset.FRACAS, + EntailmentLabel.ENTAILMENT, + { sectionName: "SectionOnly" }, + ); + fixture.componentRef.setInput("problem", problem); + fixture.detectChanges(); + expect(component.sectionString()).toBe("SectionOnly"); + }); + + it("should show only subsection when section is null", () => { + const problem = createMockProblem( + "6", + Dataset.FRACAS, + EntailmentLabel.ENTAILMENT, + { subsectionName: "SubsectionOnly" }, + ); + fixture.componentRef.setInput("problem", problem); + fixture.detectChanges(); + expect(component.sectionString()).toBe("SubsectionOnly"); + }); + }); }); diff --git a/frontend/src/app/services/app-mode.service.spec.ts b/frontend/src/app/services/app-mode.service.spec.ts index c2e4809..4d2a91a 100644 --- a/frontend/src/app/services/app-mode.service.spec.ts +++ b/frontend/src/app/services/app-mode.service.spec.ts @@ -1,20 +1,142 @@ import { TestBed } from '@angular/core/testing'; +import { BehaviorSubject } from 'rxjs'; -import { AppModeService } from './app-mode.service'; +import { AppModeService, AppMode } from './app-mode.service'; +import { ProblemService } from './problem.service'; import { provideHttpClient } from '@angular/common/http'; import { provideHttpClientTesting } from '@angular/common/http/testing'; describe('AppModeService', () => { let service: AppModeService; + let mockProblemService: jasmine.SpyObj; + let allParamsSubject: BehaviorSubject; beforeEach(() => { + allParamsSubject = new BehaviorSubject(null); + + const problemServiceSpy = jasmine.createSpyObj('ProblemService', [], { + allParams$: allParamsSubject.asObservable() + }); + TestBed.configureTestingModule({ - providers: [provideHttpClient(), provideHttpClientTesting()], + providers: [ + provideHttpClient(), + provideHttpClientTesting(), + { provide: ProblemService, useValue: problemServiceSpy } + ], }); + service = TestBed.inject(AppModeService); + mockProblemService = TestBed.inject(ProblemService) as jasmine.SpyObj; }); it('should be created', () => { expect(service).toBeTruthy(); }); + + describe('viewMode$', () => { + it('should return BROWSE mode when problemId is set and edit is false', (done) => { + const mockParams = { + get: jasmine.createSpy('get').and.returnValue('123') + }; + const allParams = { + params: mockParams, + queryParams: jasmine.createSpy(), + edit: false + }; + + service.viewMode$.subscribe(mode => { + expect(mode).toBe(AppMode.BROWSE); + done(); + }); + + allParamsSubject.next(allParams); + }); + + it('should return EDIT mode when problemId is set and edit is true', (done) => { + const mockParams = { + get: jasmine.createSpy('get').and.returnValue('123') + }; + const allParams = { + params: mockParams, + queryParams: jasmine.createSpy(), + edit: true + }; + + service.viewMode$.subscribe(mode => { + expect(mode).toBe(AppMode.EDIT); + done(); + }); + + allParamsSubject.next(allParams); + }); + + it('should return ADD mode when problemId is "new"', (done) => { + const mockParams = { + get: jasmine.createSpy('get').and.returnValue('new') + }; + const allParams = { + params: mockParams, + queryParams: jasmine.createSpy(), + edit: false + }; + + service.viewMode$.subscribe(mode => { + expect(mode).toBe(AppMode.ADD); + done(); + }); + + allParamsSubject.next(allParams); + }); + + it('should not emit when allParams is null', () => { + const emittedValues: AppMode[] = []; + + service.viewMode$.subscribe(mode => { + emittedValues.push(mode); + }); + + allParamsSubject.next(null); + + expect(emittedValues.length).toEqual(0); + }); + }); + + describe('loading$', () => { + it('should emit true when viewMode is undefined', (done) => { + const mockParams = { + get: jasmine.createSpy('get').and.returnValue('123') + }; + const allParams = { + params: mockParams, + queryParams: jasmine.createSpy(), + edit: false + }; + + service.loading$.subscribe(loading => { + expect(loading).toBe(false); + done(); + }); + + allParamsSubject.next(allParams); + }); + + it('should emit false when viewMode is defined', (done) => { + const mockParams = { + get: jasmine.createSpy('get').and.returnValue('edit-123') + }; + const allParams = { + params: mockParams, + queryParams: jasmine.createSpy(), + edit: true + }; + + service.loading$.subscribe(loading => { + expect(loading).toBe(false); + done(); + }); + + allParamsSubject.next(allParams); + }); + }); }); diff --git a/frontend/src/app/services/app-mode.service.ts b/frontend/src/app/services/app-mode.service.ts index fab7600..93187d9 100644 --- a/frontend/src/app/services/app-mode.service.ts +++ b/frontend/src/app/services/app-mode.service.ts @@ -17,12 +17,12 @@ export class AppModeService { public viewMode$ = this.problemService.allParams$.pipe( filter(allParams => allParams !== null), map(({ params, edit }) => { - if (params.get("problemId") === "new") { - return AppMode.ADD; - } if (edit) { return AppMode.EDIT; } + if (params.get("problemId") === "new") { + return AppMode.ADD; + } return AppMode.BROWSE; }), shareReplay(1) diff --git a/frontend/src/app/services/problem.service.spec.ts b/frontend/src/app/services/problem.service.spec.ts index 0797759..a241fb4 100644 --- a/frontend/src/app/services/problem.service.spec.ts +++ b/frontend/src/app/services/problem.service.spec.ts @@ -1,19 +1,199 @@ -import { TestBed } from '@angular/core/testing'; +import { TestBed } from "@angular/core/testing"; +import { HttpTestingController, provideHttpClientTesting } from "@angular/common/http/testing"; +import { ProblemService } from "./problem.service"; +import { Dataset, EntailmentLabel, Problem, ProblemResponse, SaveProblemResponse } from "@/types"; +import { convertToParamMap } from "@angular/router"; +import { provideHttpClient } from "@angular/common/http"; +import { ParseInput } from "@/annotate/annotation-input/annotation-input.component"; -import { ProblemService } from './problem.service'; -import { provideHttpClient } from '@angular/common/http'; - -describe('ProblemService', () => { +describe("ProblemService", () => { let service: ProblemService; + let httpMock: HttpTestingController; beforeEach(() => { TestBed.configureTestingModule({ - providers: [provideHttpClient()] + providers: [ + provideHttpClient(), + provideHttpClientTesting(), + ProblemService + ], }); service = TestBed.inject(ProblemService); + httpMock = TestBed.inject(HttpTestingController); + }); + + afterEach(() => { + httpMock.verify(); }); - it('should be created', () => { + it("should be created", () => { expect(service).toBeTruthy(); }); + + describe("problemResponse$", () => { + it("should handle 'new' problem ID by returning a new problem", (done) => { + service.allParams$.next({ + params: convertToParamMap({ problemId: "new" }), + queryParams: convertToParamMap({}), + edit: false + }); + + service.problemResponse$.subscribe(response => { + expect(response?.problem?.id).toBe("new"); + expect(response?.problem?.dataset).toBe(Dataset.USER); + done(); + }); + }); + + it("should fetch an existing problem", (done) => { + const mockProblemId = "123"; + const mockResponse: ProblemResponse = { + problem: { + id: mockProblemId, + dataset: Dataset.SICK, + premises: ["a"], + hypothesis: "b", + entailmentLabel: EntailmentLabel.ENTAILMENT, + kbItems: [], + extraData: { + pairId: 1, + relatednessScore: 4.5 + } + }, + index: 1, + totalProblems: 1, + firstProblemId: "123", + lastProblemId: "123", + nextProblemId: null, + previousProblemId: null, + error: null + }; + + service.allParams$.next({ + params: convertToParamMap({ problemId: mockProblemId }), + queryParams: convertToParamMap({}), + edit: false + }); + + service.problemResponse$.subscribe(response => { + expect(response).toEqual(mockResponse); + done(); + }); + + const req = httpMock.expectOne(`/api/problem/${mockProblemId}?text=&dataset=&gold=&entailmentLabel=`); + expect(req.request.method).toBe("GET"); + req.flush(mockResponse); + }); + + it("should handle HTTP errors when fetching a problem", (done) => { + const mockProblemId = "404"; + + service.problemResponse$.subscribe(response => { + expect(response).toBeNull(); + done(); + }); + + service.allParams$.next({ + params: convertToParamMap({ problemId: mockProblemId }), + queryParams: convertToParamMap({}), + edit: false + }); + + const req = httpMock.expectOne(r => r.url.startsWith(`/api/problem/${mockProblemId}`)); + expect(req.request.method).toBe("GET"); + expect(req.request.params.get("text")).toBe(""); + expect(req.request.params.get("dataset")).toBe(""); + expect(req.request.params.get("gold")).toBe(""); + expect(req.request.params.get("entailmentLabel")).toBe(""); + req.flush("Not Found", { status: 404, statusText: "Not Found" }); + }); + + it("should include query parameters in the request", (done) => { + const queryParams = { text: "search", dataset: "FRACAS" }; + service.allParams$.next({ + params: convertToParamMap({ problemId: "abc" }), + queryParams: convertToParamMap(queryParams), + edit: false + }); + + service.problemResponse$.subscribe(() => done()); + + const req = httpMock.expectOne(r => r.url.startsWith("/api/problem/abc")); + expect(req.request.params.get("text")).toBe("search"); + expect(req.request.params.get("dataset")).toBe("FRACAS"); + expect(req.request.params.get("gold")).toBe(""); + expect(req.request.params.get("entailmentLabel")).toBe(""); + req.flush({}); + }); + }); + + + describe("saveProblem$", () => { + it("should POST the problem and return the response", (done) => { + const problemToSave: ParseInput = { + id: "1", + premises: ["a"], + hypothesis: "b", + kbItems: [], + }; + const mockResponse: SaveProblemResponse = { id: "1", error: null }; + + service.saveProblem$.subscribe(response => { + expect(response).toEqual(mockResponse); + done(); + }); + + service.submit$.next(problemToSave); + + const req = httpMock.expectOne("/api/problem/1"); + expect(req.request.method).toBe("POST"); + expect(req.request.body).toEqual(problemToSave); + req.flush(mockResponse); + }); + + it("should handle errors during save", (done) => { + const problemToSave: ParseInput = { + id: "2", + premises: ["c"], + hypothesis: "d", + kbItems: [] + }; + + service.saveProblem$.subscribe(response => { + expect(response.id).toBeNull(); + expect(response.error).toBe("Failed to save problem"); + done(); + }); + + service.submit$.next(problemToSave); + + const req = httpMock.expectOne("/api/problem/2"); + req.flush("Error", { status: 500, statusText: "Server Error" }); + }); + }); + + describe("firstProblemId$", () => { + it("should fetch the first problem and return its ID", (done) => { + const mockResponse = { problem: { id: "first" } }; + + service.firstProblemId$.subscribe(id => { + expect(id).toBe("first"); + done(); + }); + + const req = httpMock.expectOne("/api/problem/"); + expect(req.request.method).toBe("GET"); + req.flush(mockResponse); + }); + + it("should return null if fetching the first problem fails", (done) => { + service.firstProblemId$.subscribe(id => { + expect(id).toBeNull(); + done(); + }); + + const req = httpMock.expectOne("/api/problem/"); + req.flush("Error", { status: 500, statusText: "Server Error" }); + }); + }); }); From 483c0f83fde0c09a47aa01367b5941f85a848bcf Mon Sep 17 00:00:00 2001 From: Xander Vertegaal Date: Wed, 17 Sep 2025 13:41:09 +0200 Subject: [PATCH 19/42] Use int for IDs --- backend/problem/models.py | 4 +- backend/problem/problem_details.py | 20 ++++---- backend/problem/urls.py | 2 +- backend/problem/views/problem.py | 47 +++++++++++-------- .../src/app/annotate/annotate.component.ts | 4 +- .../annotation-input.component.ts | 12 ++--- .../knowledge-base-form.component.html | 2 +- .../knowledge-base-form.component.ts | 2 +- .../premises-form.component.html | 2 +- .../problem-details.component.ts | 2 +- .../annotate/navigator/navigator.component.ts | 2 +- frontend/src/app/services/problem.service.ts | 17 ++++--- frontend/src/app/types.ts | 14 +++--- 13 files changed, 71 insertions(+), 59 deletions(-) diff --git a/backend/problem/models.py b/backend/problem/models.py index 60bb5f5..d7d66c6 100644 --- a/backend/problem/models.py +++ b/backend/problem/models.py @@ -76,7 +76,7 @@ def serialize(self) -> dict: kb_items = KnowledgeBase.objects.filter(problem=self) return { - "id": str(self.pk), + "id": self.pk, "dataset": self.dataset, "premises": [premise.text for premise in self.premises.all()], "hypothesis": self.hypothesis.text, @@ -111,7 +111,7 @@ class Relationship(models.TextChoices): def serialize(self) -> dict: return { - "id": str(self.pk), + "id": self.pk, "entity1": self.entity1, "entity2": self.entity2, "relationship": self.relationship, diff --git a/backend/problem/problem_details.py b/backend/problem/problem_details.py index 703f426..3254e70 100644 --- a/backend/problem/problem_details.py +++ b/backend/problem/problem_details.py @@ -10,16 +10,16 @@ @dataclass class RelatedProblemIds: - first: Optional[str] = None - previous: Optional[str] = None - next: Optional[str] = None - last: Optional[str] = None - total: Optional[int] = None + first: int | None = None + previous: int | None = None + next: int | None = None + last: int | None = None + total: int | None = None def get_related_problem_ids( problem_qs: QuerySet[Problem], - problem_id: Optional[str], + problem_id: int | None = None, ) -> RelatedProblemIds: """ Retrieves the IDs of surrounding problem objects @@ -44,10 +44,10 @@ def get_related_problem_ids( problem = None return RelatedProblemIds( - first=str(first_problem.pk) if first_problem else None, - previous=str(previous_problem.pk) if previous_problem else None, - next=str(next_problem.pk) if next_problem else None, - last=str(last_problem.pk) if last_problem else None, + first=first_problem.pk if first_problem else None, + previous=previous_problem.pk if previous_problem else None, + next=next_problem.pk if next_problem else None, + last=last_problem.pk if last_problem else None, total=total, ) diff --git a/backend/problem/urls.py b/backend/problem/urls.py index 071bf8f..14a0a8e 100644 --- a/backend/problem/urls.py +++ b/backend/problem/urls.py @@ -5,7 +5,7 @@ urlpatterns = [ - path("", ProblemView.as_view(), name="problem_view"), + path("", ProblemView.as_view(), name="problem_view"), path("parse", ParseView.as_view(), name="parse_view"), path("", ProblemView.as_view(), name="first_problem_view"), ] diff --git a/backend/problem/views/problem.py b/backend/problem/views/problem.py index 41c900c..db5704b 100644 --- a/backend/problem/views/problem.py +++ b/backend/problem/views/problem.py @@ -15,10 +15,10 @@ class ProblemResponse: index: int | None = None error: str | None = None - first: str | None = None - previous: str | None = None - next: str | None = None - last: str | None = None + first: int | None = None + previous: int | None = None + next: int | None = None + last: int | None = None total: int | None = None def json_response(self, status=200) -> JsonResponse: @@ -39,7 +39,7 @@ def json_response(self, status=200) -> JsonResponse: @dataclass class SaveProblemResponse: - id: str | None = None + id: int | None = None error: str | None = None def json_response(self, status=200) -> JsonResponse: @@ -47,9 +47,10 @@ def json_response(self, status=200) -> JsonResponse: class ProblemView(APIView): - def get(self, request, problem_id: str | None = None) -> JsonResponse: + def get(self, request, problem_id: int | None = None) -> JsonResponse: """ - If a Problem ID is provided, attempts to retrieve the requested Problem. Otherwise, simply returns the first problem of the QS. + If a Problem ID is provided, retrieves the requested Problem. + Otherwise, simply returns the first problem of the QS. """ filters = get_filters(request.query_params) @@ -83,9 +84,9 @@ def get(self, request, problem_id: str | None = None) -> JsonResponse: total=related_problem_ids.total, ).json_response(status=200) - def post(self, request, problem_id: str) -> JsonResponse: + def post(self, request, problem_id: int | None = None) -> JsonResponse: """ - If the Problem ID is "new", attempts to create a new Problem; + If the Problem ID is None, attempts to create a new Problem; else the associated Problem is updated. """ parse_input = request.data @@ -102,7 +103,7 @@ def post(self, request, problem_id: str) -> JsonResponse: problem: Problem | None = None error: str | None = None - if problem_id == "new": + if problem_id is None: try: problem = create_problem_from_input(validated_input) except Exception as e: @@ -123,14 +124,14 @@ def post(self, request, problem_id: str) -> JsonResponse: class KBItem(TypedDict): - id: str + id: int entity1: str relationship: str entity2: str class ParseInput(TypedDict): - id: str + id: int premises: list[str] hypothesis: str kbItems: list[KBItem] @@ -143,8 +144,10 @@ def validate_input(parse_input: dict) -> ParseInput: if not isinstance(parse_input, dict): raise ValueError("Input must be a dictionary") - if "id" not in parse_input or not isinstance(parse_input["id"], str): - raise ValueError("Missing or invalid 'id' field") + if "id" not in parse_input or ( + parse_input["id"] is not None and not isinstance(parse_input["id"], int) + ): + raise ValueError("Invalid problem 'id' field") if "premises" not in parse_input or not isinstance(parse_input["premises"], list): raise ValueError("Missing or invalid 'premises' field") @@ -160,8 +163,10 @@ def validate_input(parse_input: dict) -> ParseInput: for item in parse_input["kbItems"]: if not isinstance(item, dict): raise ValueError("Each kbItem must be a dictionary") - if "id" not in item or not isinstance(item["id"], str): - raise ValueError("Missing or invalid 'id' in kbItem") + if "id" not in item or ( + item["id"] is not None and not isinstance(item["id"], int) + ): + raise ValueError("Invalid 'id' in kbItem") if "entity1" not in item or not isinstance(item["entity1"], str): raise ValueError("Missing or invalid 'entity1' in kbItem") if "relationship" not in item or not isinstance(item["relationship"], str): @@ -185,13 +190,16 @@ def create_problem_from_input(parse_input: ParseInput) -> Problem: """ try: premise_sentences = [ - Sentence.objects.get_or_create(text=premise)[0] for premise in parse_input["premises"] + Sentence.objects.get_or_create(text=premise)[0] + for premise in parse_input["premises"] ] except Exception as e: raise ValueError(f"Error creating premise sentences: {e}") try: - hypothesis_sentence = Sentence.objects.get_or_create(text=parse_input["hypothesis"])[0] + hypothesis_sentence = Sentence.objects.get_or_create( + text=parse_input["hypothesis"] + )[0] except Exception as e: raise ValueError(f"Error creating hypothesis sentence: {e}") @@ -218,7 +226,7 @@ def update_or_create_kb_items(problem: Problem, kb_items: list[KBItem]) -> None: relationship = item.get("relationship") entity2 = item.get("entity2") - if id == "new": + if id is None: try: kb = KnowledgeBase.objects.create( entity1=entity1, @@ -254,6 +262,7 @@ def update_problem_from_input(parse_input: ParseInput) -> Problem: problem.hypothesis = Sentence.objects.get_or_create(text=parse_input["hypothesis"])[ 0 ] + problem.save() premises: list[Sentence] = [] for input_premise in parse_input["premises"]: diff --git a/frontend/src/app/annotate/annotate.component.ts b/frontend/src/app/annotate/annotate.component.ts index 7dd9860..8d9b34b 100644 --- a/frontend/src/app/annotate/annotate.component.ts +++ b/frontend/src/app/annotate/annotate.component.ts @@ -62,8 +62,8 @@ export class AnnotateComponent implements OnInit { }); } - public goToProblem(problemId: string): void { - this.router.navigate(["/", "annotate", problemId]); + public goToProblem(problemId: number): void { + this.router.navigate(["/", "annotate", problemId.toString()]); } public addProblem(): void { diff --git a/frontend/src/app/annotate/annotation-input/annotation-input.component.ts b/frontend/src/app/annotate/annotation-input/annotation-input.component.ts index e6f5b45..6b21cfd 100644 --- a/frontend/src/app/annotate/annotation-input/annotation-input.component.ts +++ b/frontend/src/app/annotate/annotation-input/annotation-input.component.ts @@ -23,14 +23,14 @@ import { AppModeService } from "@/services/app-mode.service"; import { ToastService } from "@/services/toast.service"; export type ParseInputForm = FormGroup<{ - id: FormControl; + id: FormControl; premises: FormArray>; hypothesis: FormControl; kbItems: FormArray; }>; type KnowledgeBaseItemsForm = FormGroup<{ - id: FormControl; + id: FormControl; entity1: FormControl; relationship: FormControl; entity2: FormControl; @@ -142,10 +142,10 @@ export class AnnotationInputComponent implements OnInit { } private navigateToNewProblem(problem: Problem | null): void { - if (!problem) { + if (!problem || !problem.id) { return; } - const incomingProblemId = problem.id?.toString(); + const incomingProblemId = problem.id.toString(); const currentProblemId = this.route.snapshot.paramMap.get("problemId"); if (incomingProblemId !== currentProblemId) { @@ -162,7 +162,7 @@ export class AnnotationInputComponent implements OnInit { const kbItems = this.buildKbForms(problem.kbItems); return new FormGroup({ - id: new FormControl(id, { + id: new FormControl(id, { nonNullable: true }), premises: new FormArray( @@ -183,7 +183,7 @@ export class AnnotationInputComponent implements OnInit { } private buildKbForms(inputKbItems: Problem['kbItems']): KnowledgeBaseItemsForm[] { return inputKbItems.map(item => new FormGroup({ - id: new FormControl(item.id, { + id: new FormControl(item.id, { nonNullable: true }), entity1: new FormControl(item.entity1, { diff --git a/frontend/src/app/annotate/annotation-input/knowledge-base-form/knowledge-base-form.component.html b/frontend/src/app/annotate/annotation-input/knowledge-base-form/knowledge-base-form.component.html index 57f175e..e3c9e90 100644 --- a/frontend/src/app/annotate/annotation-input/knowledge-base-form/knowledge-base-form.component.html +++ b/frontend/src/app/annotate/annotation-input/knowledge-base-form/knowledge-base-form.component.html @@ -26,7 +26,7 @@
      - @for (kbItem of kbControls; let i = $index; track $index) { + @for (kbItem of kbControls; let i = $index; track `${i}-${kbItem.value.entity1}-${kbItem.value.relationship}-${kbItem.value.entity2}`) {
    • @if (viewMode === 'add' || viewMode === 'edit') {
      diff --git a/frontend/src/app/annotate/annotation-input/knowledge-base-form/knowledge-base-form.component.ts b/frontend/src/app/annotate/annotation-input/knowledge-base-form/knowledge-base-form.component.ts index 786cf94..5d78085 100644 --- a/frontend/src/app/annotate/annotation-input/knowledge-base-form/knowledge-base-form.component.ts +++ b/frontend/src/app/annotate/annotation-input/knowledge-base-form/knowledge-base-form.component.ts @@ -38,7 +38,7 @@ export class KnowledgeBaseFormComponent { public addKnowledgeBaseItem(): void { const newItem = new FormGroup({ - id: new FormControl("new", { + id: new FormControl(null, { nonNullable: true }), entity1: new FormControl("", { diff --git a/frontend/src/app/annotate/annotation-input/premises-form/premises-form.component.html b/frontend/src/app/annotate/annotation-input/premises-form/premises-form.component.html index 1baa397..b3aa888 100644 --- a/frontend/src/app/annotate/annotation-input/premises-form/premises-form.component.html +++ b/frontend/src/app/annotate/annotation-input/premises-form/premises-form.component.html @@ -25,7 +25,7 @@
        - @for (control of premiseControls; let i = $index; track $index) { + @for (control of premiseControls; let i = $index; track `${i}-${control.value}`) {
      • = { - problemId: problem.id.toString(), + problemId: problem.id?.toString() ?? $localize`new`, dataset: problem.dataset, entailmentLabel: problem.entailmentLabel, }; diff --git a/frontend/src/app/annotate/navigator/navigator.component.ts b/frontend/src/app/annotate/navigator/navigator.component.ts index 481a563..59fc445 100644 --- a/frontend/src/app/annotate/navigator/navigator.component.ts +++ b/frontend/src/app/annotate/navigator/navigator.component.ts @@ -30,7 +30,7 @@ export class NavigatorComponent { public faAngleRight = faAngleRight; public faShuffle = faShuffle; - public navigateToProblem(id: string | null | undefined): void { + public navigateToProblem(id: number | null | undefined): void { if (!id) { return; } diff --git a/frontend/src/app/services/problem.service.ts b/frontend/src/app/services/problem.service.ts index 3a748bd..e769553 100644 --- a/frontend/src/app/services/problem.service.ts +++ b/frontend/src/app/services/problem.service.ts @@ -40,12 +40,15 @@ export class ProblemService { ); public saveProblem$ = this.submit$.pipe( - exhaustMap((problem) => this.http.post(`/api/problem/${problem.id}`, problem).pipe( - catchError((error) => { - console.error('Error saving problem:', error); - return of({ id: null, error: 'Failed to save problem' }); - }) - )) + exhaustMap((problem) => { + const url = `/api/problem/${problem.id ?? ""}`; + return this.http.post(url, problem).pipe( + catchError((error) => { + console.error('Error saving problem:', error); + return of({ id: null, error: 'Failed to save problem' }); + }) + ); + }) ); private newProblem$(): Observable { @@ -58,7 +61,7 @@ export class ProblemService { totalProblems: 0, error: null, problem: { - id: "new", + id: null, hypothesis: "", dataset: Dataset.USER, premises: [], diff --git a/frontend/src/app/types.ts b/frontend/src/app/types.ts index 8147b75..0cd3c37 100644 --- a/frontend/src/app/types.ts +++ b/frontend/src/app/types.ts @@ -31,14 +31,14 @@ export enum KnowledgeBaseRelationship { } interface KnowledgeBaseItem { - id: string; + id: number; entity1: string; relationship: KnowledgeBaseRelationship; entity2: string; } interface ProblemBase { - id: string; + id: number | null; premises: string[]; hypothesis: string | null; entailmentLabel: EntailmentLabel; @@ -74,15 +74,15 @@ interface BaseResponse { export interface ProblemResponse extends BaseResponse { index: number | null; problem: Problem | null; - firstProblemId: string | null; - previousProblemId: string | null; - nextProblemId: string | null; - lastProblemId: string | null; + firstProblemId: number | null; + previousProblemId: number | null; + nextProblemId: number | null; + lastProblemId: number | null; totalProblems: number; } export interface SaveProblemResponse extends BaseResponse { - id: string | null; + id: number | null; } export enum Dataset { From d144eb682be16f9c705db8a59ee6d663e63b12b0 Mon Sep 17 00:00:00 2001 From: Xander Vertegaal Date: Wed, 17 Sep 2025 14:38:01 +0200 Subject: [PATCH 20/42] Fix failing tests --- .../annotation-input.component.spec.ts | 52 +++++++++---------- .../problem-details.component.spec.ts | 10 ++-- .../src/app/services/problem.service.spec.ts | 23 ++++---- 3 files changed, 42 insertions(+), 43 deletions(-) diff --git a/frontend/src/app/annotate/annotation-input/annotation-input.component.spec.ts b/frontend/src/app/annotate/annotation-input/annotation-input.component.spec.ts index 702a7f6..3e5bca1 100644 --- a/frontend/src/app/annotate/annotation-input/annotation-input.component.spec.ts +++ b/frontend/src/app/annotate/annotation-input/annotation-input.component.spec.ts @@ -20,7 +20,7 @@ describe("AnnotationInputComponent", () => { params: of({ problemId: "1" }), snapshot: { paramMap: { - get: jasmine.createSpy('get').and.returnValue('current-problem-id') + get: jasmine.createSpy('get').and.returnValue("17") } } }; @@ -48,19 +48,19 @@ describe("AnnotationInputComponent", () => { describe('buildForm', () => { it('should build form with correct structure and values from problem data', () => { const mockProblem: Problem = { - id: "test-123", + id: 123, premises: ["First premise", "Second premise"], hypothesis: "Test hypothesis", entailmentLabel: EntailmentLabel.ENTAILMENT, kbItems: [ { - id: "kb1", + id: 456, entity1: "cat", entity2: "animal", relationship: KnowledgeBaseRelationship.SUBSET }, { - id: "kb2", + id: 789, entity1: "dog", entity2: "pet", relationship: KnowledgeBaseRelationship.EQUAL @@ -75,35 +75,35 @@ describe("AnnotationInputComponent", () => { // Test form structure expect(form).toBeTruthy(); - expect(form.get('id')?.value).toBe('test-123'); + expect(form.get('id')?.value).toBe(123); expect(form.get('hypothesis')?.value).toBe('Test hypothesis'); // Test premises array - const premisesArray = form.get('premises') as FormArray; + const premisesArray = form.controls.premises; expect(premisesArray.length).toBe(2); - expect(premisesArray.at(0).value).toBe('First premise'); - expect(premisesArray.at(1).value).toBe('Second premise'); + expect(premisesArray.controls[0].value).toBe('First premise'); + expect(premisesArray.controls[1].value).toBe('Second premise'); // Test knowledge base items - const kbItemsArray = form.get('kbItems') as FormArray; + const kbItemsArray = form.controls.kbItems; expect(kbItemsArray.length).toBe(2); - const firstKbForm = kbItemsArray.at(0) as FormGroup; - expect(firstKbForm.get('id')?.value).toBe('kb1'); - expect(firstKbForm.get('entity1')?.value).toBe('cat'); - expect(firstKbForm.get('entity2')?.value).toBe('animal'); - expect(firstKbForm.get('relationship')?.value).toBe(KnowledgeBaseRelationship.SUBSET); - - const secondKbForm = kbItemsArray.at(1) as FormGroup; - expect(secondKbForm.get('id')?.value).toBe('kb2'); - expect(secondKbForm.get('entity1')?.value).toBe('dog'); - expect(secondKbForm.get('entity2')?.value).toBe('pet'); - expect(secondKbForm.get('relationship')?.value).toBe(KnowledgeBaseRelationship.EQUAL); + const firstKbForm = kbItemsArray.controls[0]; + expect(firstKbForm.value.id).toBe(456); + expect(firstKbForm.value.entity1).toBe('cat'); + expect(firstKbForm.value.entity2).toBe('animal'); + expect(firstKbForm.value.relationship).toBe(KnowledgeBaseRelationship.SUBSET); + + const secondKbForm = kbItemsArray.controls[1]; + expect(secondKbForm.value.id).toBe(789); + expect(secondKbForm.value.entity1).toBe('dog'); + expect(secondKbForm.value.entity2).toBe('pet'); + expect(secondKbForm.value.relationship).toBe(KnowledgeBaseRelationship.EQUAL); }); it('should handle empty premises and kbItems arrays', () => { const mockProblem: Problem = { - id: "empty-test", + id: 123, premises: [], hypothesis: "Empty test hypothesis", entailmentLabel: EntailmentLabel.NEUTRAL, @@ -114,7 +114,7 @@ describe("AnnotationInputComponent", () => { const form = component['buildForm'](mockProblem); - expect(form.get('id')?.value).toBe('empty-test'); + expect(form.get('id')?.value).toBe(123); expect(form.get('hypothesis')?.value).toBe('Empty test hypothesis'); const premisesArray = form.get('premises') as FormArray; @@ -126,7 +126,7 @@ describe("AnnotationInputComponent", () => { it('should create form controls with required validators', () => { const mockProblem: Problem = { - id: "validator-test", + id: 1, premises: ["Test premise"], hypothesis: "Test hypothesis", entailmentLabel: EntailmentLabel.CONTRADICTION, @@ -147,7 +147,7 @@ describe("AnnotationInputComponent", () => { describe('navigateToNewProblem', () => { it('should navigate when problem ID is different from current route', () => { const mockProblem: Problem = { - id: "new-problem-id", + id: 12, premises: [], hypothesis: "", entailmentLabel: EntailmentLabel.UNKNOWN, @@ -159,14 +159,14 @@ describe("AnnotationInputComponent", () => { component['navigateToNewProblem'](mockProblem); expect(mockRouter.navigate).toHaveBeenCalledWith( - ['/annotate', 'new-problem-id'], + ['/annotate', 12], { queryParamsHandling: 'preserve' } ); }); it('should not navigate when problem ID matches current route', () => { const mockProblem: Problem = { - id: "current-problem-id", + id: 17, premises: [], hypothesis: "", entailmentLabel: EntailmentLabel.UNKNOWN, diff --git a/frontend/src/app/annotate/annotation-input/problem-details/problem-details.component.spec.ts b/frontend/src/app/annotate/annotation-input/problem-details/problem-details.component.spec.ts index dbedf86..f135997 100644 --- a/frontend/src/app/annotate/annotation-input/problem-details/problem-details.component.spec.ts +++ b/frontend/src/app/annotate/annotation-input/problem-details/problem-details.component.spec.ts @@ -4,7 +4,7 @@ import { ProblemDetailsComponent } from "./problem-details.component"; import { Dataset, EntailmentLabel, Problem } from "../../../types"; const createMockProblem = ( - id: string, + id: number, dataset: Dataset, entailmentLabel: EntailmentLabel, extraData: any = {}, @@ -44,7 +44,7 @@ describe("ProblemDetailsComponent", () => { describe("with SICK dataset problem", () => { beforeEach(() => { const problem = createMockProblem( - "1", + 1, Dataset.SICK, EntailmentLabel.ENTAILMENT, ); @@ -71,7 +71,7 @@ describe("ProblemDetailsComponent", () => { describe("with FRACAS dataset problem", () => { beforeEach(() => { const problem = createMockProblem( - "2", + 2, Dataset.FRACAS, EntailmentLabel.CONTRADICTION, { @@ -103,7 +103,7 @@ describe("ProblemDetailsComponent", () => { describe("sectionString computation", () => { it("should show only section when subsection is null", () => { const problem = createMockProblem( - "5", + 5, Dataset.FRACAS, EntailmentLabel.ENTAILMENT, { sectionName: "SectionOnly" }, @@ -115,7 +115,7 @@ describe("ProblemDetailsComponent", () => { it("should show only subsection when section is null", () => { const problem = createMockProblem( - "6", + 6, Dataset.FRACAS, EntailmentLabel.ENTAILMENT, { subsectionName: "SubsectionOnly" }, diff --git a/frontend/src/app/services/problem.service.spec.ts b/frontend/src/app/services/problem.service.spec.ts index a241fb4..5648bd4 100644 --- a/frontend/src/app/services/problem.service.spec.ts +++ b/frontend/src/app/services/problem.service.spec.ts @@ -31,22 +31,21 @@ describe("ProblemService", () => { }); describe("problemResponse$", () => { - it("should handle 'new' problem ID by returning a new problem", (done) => { + it("should handle 'None' problem ID", (done) => { service.allParams$.next({ - params: convertToParamMap({ problemId: "new" }), + params: convertToParamMap({ problemId: null }), queryParams: convertToParamMap({}), edit: false }); service.problemResponse$.subscribe(response => { - expect(response?.problem?.id).toBe("new"); - expect(response?.problem?.dataset).toBe(Dataset.USER); + expect(response).toEqual(null); done(); }); }); it("should fetch an existing problem", (done) => { - const mockProblemId = "123"; + const mockProblemId = 123; const mockResponse: ProblemResponse = { problem: { id: mockProblemId, @@ -62,8 +61,8 @@ describe("ProblemService", () => { }, index: 1, totalProblems: 1, - firstProblemId: "123", - lastProblemId: "123", + firstProblemId: 123, + lastProblemId: 789, nextProblemId: null, previousProblemId: null, error: null @@ -131,12 +130,12 @@ describe("ProblemService", () => { describe("saveProblem$", () => { it("should POST the problem and return the response", (done) => { const problemToSave: ParseInput = { - id: "1", + id: 1, premises: ["a"], hypothesis: "b", kbItems: [], }; - const mockResponse: SaveProblemResponse = { id: "1", error: null }; + const mockResponse: SaveProblemResponse = { id: 1, error: null }; service.saveProblem$.subscribe(response => { expect(response).toEqual(mockResponse); @@ -153,7 +152,7 @@ describe("ProblemService", () => { it("should handle errors during save", (done) => { const problemToSave: ParseInput = { - id: "2", + id: 2, premises: ["c"], hypothesis: "d", kbItems: [] @@ -174,10 +173,10 @@ describe("ProblemService", () => { describe("firstProblemId$", () => { it("should fetch the first problem and return its ID", (done) => { - const mockResponse = { problem: { id: "first" } }; + const mockResponse = { problem: { id: 555 } }; service.firstProblemId$.subscribe(id => { - expect(id).toBe("first"); + expect(id).toBe(555); done(); }); From 619439c76b8d99e0eea1b9a46d34ec72f524b87c Mon Sep 17 00:00:00 2001 From: Xander Vertegaal Date: Mon, 22 Sep 2025 11:17:14 +0200 Subject: [PATCH 21/42] Avoid unnecessary requests --- frontend/src/app/annotate/annotate.component.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/frontend/src/app/annotate/annotate.component.ts b/frontend/src/app/annotate/annotate.component.ts index 8d9b34b..3f59fb3 100644 --- a/frontend/src/app/annotate/annotate.component.ts +++ b/frontend/src/app/annotate/annotate.component.ts @@ -7,7 +7,7 @@ import { FontAwesomeModule } from "@fortawesome/angular-fontawesome"; import { faBinoculars, faPlus } from "@fortawesome/free-solid-svg-icons"; import { ActivatedRoute, Router } from "@angular/router"; import { ProblemService } from "@/services/problem.service"; -import { combineLatest, map, tap } from "rxjs"; +import { combineLatest, distinctUntilChanged, map } from "rxjs"; import { CommonModule } from "@angular/common"; import { Dataset } from "@/types"; import { AppModeService } from "@/services/app-mode.service"; @@ -46,7 +46,8 @@ export class AnnotateComponent implements OnInit { ngOnInit(): void { const editParam$ = this.route.url.pipe( - map(segments => segments.some(segment => segment.path === "edit")) + map(segments => segments.some(segment => segment.path === "edit")), + distinctUntilChanged(), ); combineLatest([ From 52b775adf1f6780b12110f0225efddd9971f3bab Mon Sep 17 00:00:00 2001 From: Xander Vertegaal Date: Thu, 23 Oct 2025 12:47:49 +0200 Subject: [PATCH 22/42] Use serializers for input validation --- backend/problem/serializers.py | 55 ++++++++++++ backend/problem/tests.py | 138 ++++++++++++++++++++++++++++++- backend/problem/views/problem.py | 85 +++---------------- 3 files changed, 204 insertions(+), 74 deletions(-) create mode 100644 backend/problem/serializers.py diff --git a/backend/problem/serializers.py b/backend/problem/serializers.py new file mode 100644 index 0000000..5b719e7 --- /dev/null +++ b/backend/problem/serializers.py @@ -0,0 +1,55 @@ +from rest_framework import serializers +from problem.models import Problem, KnowledgeBase + + +class KnowledgeBaseSerializer(serializers.ModelSerializer): + # If an ID is not provided, it means a new KB item is to be created. + id = serializers.IntegerField(required=False, allow_null=True) + relationship = serializers.CharField(required=True) + + class Meta: + model = KnowledgeBase + fields = ["id", "entity1", "entity2", "relationship"] + + def validate_id(self, value): + """Validate that the KnowledgeBase ID exists if provided.""" + if value is not None: + if not KnowledgeBase.objects.filter(id=value).exists(): + raise serializers.ValidationError( + f"KnowledgeBase item with ID {value} does not exist." + ) + return value + + +class ProblemInputSerializer(serializers.Serializer): + """ + Serializer for validating problem input data. + This is used for both creating and updating user-created problems. + """ + + id = serializers.IntegerField(required=False, allow_null=True) + premises = serializers.ListField( + child=serializers.CharField(allow_blank=False), + allow_empty=False, + help_text="List of premise sentence texts", + ) + hypothesis = serializers.CharField( + allow_blank=False, help_text="Hypothesis sentence text" + ) + kbItems = KnowledgeBaseSerializer( + many=True, allow_empty=True, help_text="List of knowledge base items" + ) + + def validate_id(self, value): + """ + Validate that the problem ID exists and belongs to a user problem. + Users are not allowed to modify non-user problems. + """ + if value is not None: + if not Problem.objects.filter( + id=value, dataset=Problem.Dataset.USER + ).exists(): + raise serializers.ValidationError( + f"Problem with ID {value} does not exist or is not a user problem." + ) + return value diff --git a/backend/problem/tests.py b/backend/problem/tests.py index 7ce503c..67fff54 100644 --- a/backend/problem/tests.py +++ b/backend/problem/tests.py @@ -1,3 +1,139 @@ from django.test import TestCase +from rest_framework.exceptions import ValidationError -# Create your tests here. +from .serializers import ProblemInputSerializer, KnowledgeBaseSerializer +from .models import Problem, Sentence, KnowledgeBase + + +class ProblemInputSerializerTest(TestCase): + def setUp(self): + self.h_sent = Sentence.objects.create(text="Hypothesis") + self.p_sent = Sentence.objects.create(text="Premise") + self.user_problem = Problem.objects.create( + dataset=Problem.Dataset.USER, + hypothesis=self.h_sent, + extra_data={}, + ) + self.user_problem.premises.add(self.p_sent) + + self.non_user_problem = Problem.objects.create( + dataset=Problem.Dataset.SICK, + hypothesis=self.h_sent, + extra_data={}, + ) + self.non_user_problem.premises.add(self.p_sent) + + self.kb_item = KnowledgeBase.objects.create( + problem=self.user_problem, + entity1="e1", + entity2="e2", + relationship=KnowledgeBase.Relationship.EQUAL, + ) + + def test_valid_create_data(self): + """Test valid data for creating a problem.""" + data = { + "premises": ["A cat is running."], + "hypothesis": "A cat is moving.", + "kbItems": [], + } + serializer = ProblemInputSerializer(data=data) + self.assertTrue(serializer.is_valid(raise_exception=True)) + + def test_valid_update_data(self): + """Test valid data for updating a user problem.""" + data = { + "id": self.user_problem.pk, + "premises": ["The cat is on the mat."], + "hypothesis": "A cat is on a mat.", + "kbItems": [ + { + "id": self.kb_item.pk, + "entity1": "e1", + "entity2": "e2", + "relationship": "equal", + }, + {"entity1": "new_e1", "entity2": "new_e2", "relationship": "subset"}, + ], + } + serializer = ProblemInputSerializer(data=data) + self.assertTrue(serializer.is_valid(raise_exception=True)) + + def test_valid_create_data_no_id(self): + """Test valid data for creating a problem without an ID.""" + data = { + "premises": ["A dog barks."], + "hypothesis": "A dog makes noise.", + "kbItems": [], + } + serializer = ProblemInputSerializer(data=data) + self.assertTrue(serializer.is_valid(raise_exception=True)) + + def test_invalid_id_non_existent(self): + """Test that a non-existent problem ID is invalid.""" + data = { + "id": 9999, + "premises": ["premise"], + "hypothesis": "hypothesis", + "kbItems": [], + } + serializer = ProblemInputSerializer(data=data) + with self.assertRaises(ValidationError) as context: + serializer.is_valid(raise_exception=True) + self.assertIn("does not exist", str(context.exception)) + + def test_invalid_id_not_user_problem(self): + """Test that a non-user problem ID is invalid.""" + data = { + "id": self.non_user_problem.pk, + "premises": ["premise"], + "hypothesis": "hypothesis", + "kbItems": [], + } + serializer = ProblemInputSerializer(data=data) + with self.assertRaises(ValidationError) as context: + serializer.is_valid(raise_exception=True) + self.assertIn("not a user problem", str(context.exception)) + + def test_empty_premises_invalid(self): + """Test that an empty list of premises is invalid.""" + data = {"premises": [], "hypothesis": "hypothesis", "kbItems": []} + serializer = ProblemInputSerializer(data=data) + self.assertFalse(serializer.is_valid()) + self.assertIn("premises", serializer.errors) + + def test_blank_premise_invalid(self): + """Test that a blank premise string is invalid.""" + data = {"premises": [""], "hypothesis": "hypothesis", "kbItems": []} + serializer = ProblemInputSerializer(data=data) + self.assertFalse(serializer.is_valid()) + self.assertIn("premises", serializer.errors) + + def test_blank_hypothesis_invalid(self): + """Test that a blank hypothesis is invalid.""" + data = {"premises": ["premise"], "hypothesis": "", "kbItems": []} + serializer = ProblemInputSerializer(data=data) + self.assertFalse(serializer.is_valid()) + self.assertIn("hypothesis", serializer.errors) + + def test_invalid_kb_item_id(self): + """Test that a non-existent kbItem ID is invalid.""" + data = { + "premises": ["premise"], + "hypothesis": "hypothesis", + "kbItems": [{"id": 9999, "relationship": "equal"}], + } + serializer = ProblemInputSerializer(data=data) + self.assertFalse(serializer.is_valid()) + self.assertIn("kbItems", serializer.errors) + + def test_kb_item_missing_relationship(self): + """Test that a kbItem missing a relationship is invalid.""" + data = { + "premises": ["premise"], + "hypothesis": "hypothesis", + "kbItems": [{"entity1": "e1", "entity2": "e2"}], + } + serializer = ProblemInputSerializer(data=data) + self.assertFalse(serializer.is_valid()) + self.assertIn("kbItems", serializer.errors) diff --git a/backend/problem/views/problem.py b/backend/problem/views/problem.py index db5704b..1daa26d 100644 --- a/backend/problem/views/problem.py +++ b/backend/problem/views/problem.py @@ -1,5 +1,4 @@ from dataclasses import asdict, dataclass -from typing import TypedDict from django.http import JsonResponse from rest_framework.views import APIView @@ -7,6 +6,7 @@ from langpro_annotator.logger import logger from problem.problem_details import get_filters, get_related_problem_ids from problem.models import KnowledgeBase, Problem, Sentence +from problem.serializers import ProblemInputSerializer @dataclass @@ -40,7 +40,7 @@ def json_response(self, status=200) -> JsonResponse: @dataclass class SaveProblemResponse: id: int | None = None - error: str | None = None + error: dict | None = None def json_response(self, status=200) -> JsonResponse: return JsonResponse(asdict(self), status=status) @@ -89,17 +89,17 @@ def post(self, request, problem_id: int | None = None) -> JsonResponse: If the Problem ID is None, attempts to create a new Problem; else the associated Problem is updated. """ - parse_input = request.data + serializer = ProblemInputSerializer(data=request.data) - try: - validated_input = validate_input(parse_input) - except ValueError as e: - logger.error(f"Input validation error: {e}") + if not serializer.is_valid(): + logger.error(f"Input validation error: {serializer.errors}") return SaveProblemResponse( id=None, - error=str(e), + error=dict(serializer.errors), # type: ignore ).json_response(status=400) + validated_input: dict = serializer.validated_data # type: ignore + problem: Problem | None = None error: str | None = None @@ -117,74 +117,13 @@ def post(self, request, problem_id: int | None = None) -> JsonResponse: if problem is None or error is not None: return SaveProblemResponse( id=None, - error=error, + error={"message": error}, ).json_response(status=500) return SaveProblemResponse(id=problem.pk, error=None).json_response() -class KBItem(TypedDict): - id: int - entity1: str - relationship: str - entity2: str - - -class ParseInput(TypedDict): - id: int - premises: list[str] - hypothesis: str - kbItems: list[KBItem] - - -def validate_input(parse_input: dict) -> ParseInput: - """ - Validate the parse input data. - """ - if not isinstance(parse_input, dict): - raise ValueError("Input must be a dictionary") - - if "id" not in parse_input or ( - parse_input["id"] is not None and not isinstance(parse_input["id"], int) - ): - raise ValueError("Invalid problem 'id' field") - - if "premises" not in parse_input or not isinstance(parse_input["premises"], list): - raise ValueError("Missing or invalid 'premises' field") - - if "hypothesis" not in parse_input or not isinstance( - parse_input["hypothesis"], str - ): - raise ValueError("Missing or invalid 'hypothesis' field") - - if "kbItems" not in parse_input or not isinstance(parse_input["kbItems"], list): - raise ValueError("Missing or invalid 'kbItems' field") - - for item in parse_input["kbItems"]: - if not isinstance(item, dict): - raise ValueError("Each kbItem must be a dictionary") - if "id" not in item or ( - item["id"] is not None and not isinstance(item["id"], int) - ): - raise ValueError("Invalid 'id' in kbItem") - if "entity1" not in item or not isinstance(item["entity1"], str): - raise ValueError("Missing or invalid 'entity1' in kbItem") - if "relationship" not in item or not isinstance(item["relationship"], str): - raise ValueError("Missing or invalid 'relationship' in kbItem") - if item["relationship"] not in KnowledgeBase.Relationship.values: - raise ValueError(f"Invalid 'relationship' in kbItem.") - if "entity2" not in item or not isinstance(item["entity2"], str): - raise ValueError("Missing or invalid 'entity2' in kbItem") - - return ParseInput( - id=parse_input["id"], - premises=parse_input["premises"], - hypothesis=parse_input["hypothesis"], - kbItems=parse_input["kbItems"], - ) - - -def create_problem_from_input(parse_input: ParseInput) -> Problem: +def create_problem_from_input(parse_input: dict) -> Problem: """ Save a new Problem instance from the given parse input data. """ @@ -218,7 +157,7 @@ def create_problem_from_input(parse_input: ParseInput) -> Problem: return problem -def update_or_create_kb_items(problem: Problem, kb_items: list[KBItem]) -> None: +def update_or_create_kb_items(problem: Problem, kb_items: list[dict]) -> None: kb_ids: list[str] = [] for item in kb_items: id = item.get("id") @@ -253,7 +192,7 @@ def update_or_create_kb_items(problem: Problem, kb_items: list[KBItem]) -> None: KnowledgeBase.objects.filter(problem_id=problem.pk).exclude(id__in=kb_ids).delete() -def update_problem_from_input(parse_input: ParseInput) -> Problem: +def update_problem_from_input(parse_input: dict) -> Problem: try: problem = Problem.objects.get(id=parse_input["id"]) except Problem.DoesNotExist: From 4e8c4a99754984803c70baaa207c09b0f24d7357 Mon Sep 17 00:00:00 2001 From: Xander Vertegaal Date: Thu, 23 Oct 2025 12:47:58 +0200 Subject: [PATCH 23/42] Sensible naming for urls --- backend/problem/urls.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/backend/problem/urls.py b/backend/problem/urls.py index 14a0a8e..3e33b48 100644 --- a/backend/problem/urls.py +++ b/backend/problem/urls.py @@ -5,7 +5,7 @@ urlpatterns = [ - path("", ProblemView.as_view(), name="problem_view"), + path("", ProblemView.as_view(), name="problem_detail_view"), path("parse", ParseView.as_view(), name="parse_view"), - path("", ProblemView.as_view(), name="first_problem_view"), + path("", ProblemView.as_view(), name="problem_view"), ] From aa0b0098247e39cdf41405caa4b42f5b147ca432 Mon Sep 17 00:00:00 2001 From: Xander Vertegaal Date: Thu, 23 Oct 2025 20:19:54 +0200 Subject: [PATCH 24/42] Only update User problems --- backend/problem/views/problem.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/backend/problem/views/problem.py b/backend/problem/views/problem.py index 1daa26d..842f781 100644 --- a/backend/problem/views/problem.py +++ b/backend/problem/views/problem.py @@ -194,7 +194,9 @@ def update_or_create_kb_items(problem: Problem, kb_items: list[dict]) -> None: def update_problem_from_input(parse_input: dict) -> Problem: try: - problem = Problem.objects.get(id=parse_input["id"]) + problem = Problem.objects.get( + id=parse_input["id"], dataset=Problem.Dataset.USER + ) except Problem.DoesNotExist: raise ValueError(f"Cannot find Problem with ID: {parse_input["id"]}") From 4c30ea70fbad5e1a5a41e627e6d3ded93c81880f Mon Sep 17 00:00:00 2001 From: Xander Vertegaal Date: Thu, 23 Oct 2025 20:20:05 +0200 Subject: [PATCH 25/42] Regular dict access --- backend/problem/views/problem.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/backend/problem/views/problem.py b/backend/problem/views/problem.py index 842f781..ffd6b8d 100644 --- a/backend/problem/views/problem.py +++ b/backend/problem/views/problem.py @@ -160,10 +160,10 @@ def create_problem_from_input(parse_input: dict) -> Problem: def update_or_create_kb_items(problem: Problem, kb_items: list[dict]) -> None: kb_ids: list[str] = [] for item in kb_items: - id = item.get("id") - entity1 = item.get("entity1") - relationship = item.get("relationship") - entity2 = item.get("entity2") + id = item["id"] + entity1 = item["entity1"] + relationship = item["relationship"] + entity2 = item["entity2"] if id is None: try: From 8e24695fab7ae3fd6560de42d77b702a0ef22895 Mon Sep 17 00:00:00 2001 From: Xander Vertegaal Date: Thu, 23 Oct 2025 20:20:15 +0200 Subject: [PATCH 26/42] Indentation --- backend/problem/views/problem.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/backend/problem/views/problem.py b/backend/problem/views/problem.py index ffd6b8d..51694fe 100644 --- a/backend/problem/views/problem.py +++ b/backend/problem/views/problem.py @@ -200,9 +200,9 @@ def update_problem_from_input(parse_input: dict) -> Problem: except Problem.DoesNotExist: raise ValueError(f"Cannot find Problem with ID: {parse_input["id"]}") - problem.hypothesis = Sentence.objects.get_or_create(text=parse_input["hypothesis"])[ - 0 - ] + problem.hypothesis = Sentence.objects.get_or_create( + text=parse_input["hypothesis"], + )[0] problem.save() premises: list[Sentence] = [] From 4f4576ef3e9fed313f57961553d0ae89642f8cad Mon Sep 17 00:00:00 2001 From: Xander Vertegaal Date: Thu, 23 Oct 2025 20:21:02 +0200 Subject: [PATCH 27/42] More intuitive related property access --- backend/problem/models.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/backend/problem/models.py b/backend/problem/models.py index d7d66c6..2e20cee 100644 --- a/backend/problem/models.py +++ b/backend/problem/models.py @@ -73,7 +73,7 @@ def serialize(self) -> dict: case _: serialized_extra_data = {} - kb_items = KnowledgeBase.objects.filter(problem=self) + kb_items = self.knowledge_bases.all() # type: ignore return { "id": self.pk, From b7f040c973aa3bd172d27e99b20e3bc16d691128 Mon Sep 17 00:00:00 2001 From: Xander Vertegaal Date: Thu, 23 Oct 2025 20:47:34 +0200 Subject: [PATCH 28/42] Allow exceptions to bubble up to post() --- backend/problem/views/problem.py | 51 ++++++++++++++++---------------- 1 file changed, 25 insertions(+), 26 deletions(-) diff --git a/backend/problem/views/problem.py b/backend/problem/views/problem.py index 51694fe..28a606f 100644 --- a/backend/problem/views/problem.py +++ b/backend/problem/views/problem.py @@ -89,39 +89,38 @@ def post(self, request, problem_id: int | None = None) -> JsonResponse: If the Problem ID is None, attempts to create a new Problem; else the associated Problem is updated. """ - serializer = ProblemInputSerializer(data=request.data) + input_data = request.data - if not serializer.is_valid(): - logger.error(f"Input validation error: {serializer.errors}") - return SaveProblemResponse( - id=None, - error=dict(serializer.errors), # type: ignore - ).json_response(status=400) + try: + problem = save_problem(input_data, problem_id) + except Exception as e: + logger.error(f"Error saving problem: {e}") + return SaveProblemResponse(id=problem_id, error={"message": str(e)}).json_response() - validated_input: dict = serializer.validated_data # type: ignore + return SaveProblemResponse(id=problem.pk).json_response(status=200) - problem: Problem | None = None - error: str | None = None +def save_problem(input_data: dict, problem_id: int | None) -> Problem: + serializer = ProblemInputSerializer(data=input_data) - if problem_id is None: - try: - problem = create_problem_from_input(validated_input) - except Exception as e: - error = f"Error creating problem: {str(e)}" - else: - try: - problem = update_problem_from_input(validated_input) - except Exception as e: - error = f"Error updating problem: {str(e)}" + if not serializer.is_valid(): + logger.error(f"Input validation error: {serializer.errors}") + return SaveProblemResponse( + id=None, + error=dict(serializer.errors), # type: ignore + ).json_response(status=400) + + validated_input: dict = serializer.validated_data # type: ignore - if problem is None or error is not None: - return SaveProblemResponse( - id=None, - error={"message": error}, - ).json_response(status=500) + problem: Problem | None = None + if problem_id is None: + problem = create_problem_from_input(validated_input) + else: + problem = update_problem_from_input(validated_input) - return SaveProblemResponse(id=problem.pk, error=None).json_response() + if problem is None: + raise ValueError("Problem could not be saved.") + return problem def create_problem_from_input(parse_input: dict) -> Problem: """ From 83dbee68ed1fbbcf47002256de0bc480c096dbe2 Mon Sep 17 00:00:00 2001 From: Xander Vertegaal Date: Thu, 23 Oct 2025 22:45:53 +0200 Subject: [PATCH 29/42] Let Django crash hard if no KB items can be created --- backend/problem/views/problem.py | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/backend/problem/views/problem.py b/backend/problem/views/problem.py index 28a606f..5ffb259 100644 --- a/backend/problem/views/problem.py +++ b/backend/problem/views/problem.py @@ -165,16 +165,13 @@ def update_or_create_kb_items(problem: Problem, kb_items: list[dict]) -> None: entity2 = item["entity2"] if id is None: - try: - kb = KnowledgeBase.objects.create( - entity1=entity1, - relationship=relationship, - entity2=entity2, - problem=problem, - ) - kb_ids.append(kb.pk) - except Exception as e: - raise ValueError(f"Error creating knowledge base items: {e}.") + kb = KnowledgeBase.objects.create( + entity1=entity1, + relationship=relationship, + entity2=entity2, + problem=problem, + ) + kb_ids.append(kb.pk) else: try: kb = KnowledgeBase.objects.get(id=id, problem_id=problem.pk) From 2c39be65c3a3cd9f68c8c905f6a48aae936eff81 Mon Sep 17 00:00:00 2001 From: Xander Vertegaal Date: Thu, 23 Oct 2025 22:48:32 +0200 Subject: [PATCH 30/42] Remove appModeService, integrate logic into ProblemService --- .../src/app/annotate/annotate.component.html | 8 +- .../src/app/annotate/annotate.component.ts | 4 +- .../annotation-input.component.html | 4 +- .../annotation-input.component.ts | 4 +- .../knowledge-base-form.component.html | 6 +- .../knowledge-base-form.component.ts | 7 +- .../premises-form.component.html | 10 +- .../premises-form/premises-form.component.ts | 6 +- .../src/app/services/app-mode.service.spec.ts | 142 ------------------ frontend/src/app/services/app-mode.service.ts | 32 ---- frontend/src/app/services/problem.service.ts | 22 +++ 11 files changed, 44 insertions(+), 201 deletions(-) delete mode 100644 frontend/src/app/services/app-mode.service.spec.ts delete mode 100644 frontend/src/app/services/app-mode.service.ts diff --git a/frontend/src/app/annotate/annotate.component.html b/frontend/src/app/annotate/annotate.component.html index 5c72471..7a9cf9b 100644 --- a/frontend/src/app/annotate/annotate.component.html +++ b/frontend/src/app/annotate/annotate.component.html @@ -1,8 +1,8 @@ @let firstProblemId = firstProblemId$ | async; -@let viewMode = viewMode$ | async; -@let browsing = viewMode === 'browse'; -@let adding = viewMode === 'add'; -@let editing = viewMode === 'edit'; +@let appMode = appMode$ | async; +@let browsing = appMode === 'browse'; +@let adding = appMode === 'add'; +@let editing = appMode === 'edit';
        diff --git a/frontend/src/app/annotate/annotate.component.ts b/frontend/src/app/annotate/annotate.component.ts index 3f59fb3..7aa85f6 100644 --- a/frontend/src/app/annotate/annotate.component.ts +++ b/frontend/src/app/annotate/annotate.component.ts @@ -10,7 +10,6 @@ import { ProblemService } from "@/services/problem.service"; import { combineLatest, distinctUntilChanged, map } from "rxjs"; import { CommonModule } from "@angular/common"; import { Dataset } from "@/types"; -import { AppModeService } from "@/services/app-mode.service"; import { takeUntilDestroyed } from "@angular/core/rxjs-interop"; @Component({ @@ -31,13 +30,12 @@ export class AnnotateComponent implements OnInit { private router = inject(Router); private route = inject(ActivatedRoute); private problemService = inject(ProblemService); - private appModeService = inject(AppModeService); private destroyRef = inject(DestroyRef); public faPlus = faPlus; public faBinoculars = faBinoculars; - public viewMode$ = this.appModeService.viewMode$; + public appMode$ = this.problemService.appMode$; public firstProblemId$ = this.problemService.firstProblemId$; public isUserProblem$ = this.problemService.problem$.pipe( diff --git a/frontend/src/app/annotate/annotation-input/annotation-input.component.html b/frontend/src/app/annotate/annotation-input/annotation-input.component.html index fbb434e..61890ac 100644 --- a/frontend/src/app/annotate/annotation-input/annotation-input.component.html +++ b/frontend/src/app/annotate/annotation-input/annotation-input.component.html @@ -1,5 +1,5 @@ @let problem = problem$ | async; -@let viewMode = viewMode$ | async; +@let appMode = appMode$ | async; @let userProblem = isUserProblem$ | async; @if (problem) {
        @@ -25,7 +25,7 @@ /> Start parse - @if ((viewMode === 'edit' || viewMode === 'add') && userProblem) { + @if ((appMode === 'edit' || appMode === 'add') && userProblem) {
        @@ -55,8 +55,8 @@ - } @else { - + }
        diff --git a/frontend/src/app/annotate/annotate.component.ts b/frontend/src/app/annotate/annotate.component.ts index 7aa85f6..05f2840 100644 --- a/frontend/src/app/annotate/annotate.component.ts +++ b/frontend/src/app/annotate/annotate.component.ts @@ -5,7 +5,7 @@ import { AnnotationInputComponent } from "./annotation-input/annotation-input.co import { SearchComponent } from "./search/search.component"; import { FontAwesomeModule } from "@fortawesome/angular-fontawesome"; import { faBinoculars, faPlus } from "@fortawesome/free-solid-svg-icons"; -import { ActivatedRoute, Router } from "@angular/router"; +import { ActivatedRoute, RouterLinkWithHref } from "@angular/router"; import { ProblemService } from "@/services/problem.service"; import { combineLatest, distinctUntilChanged, map } from "rxjs"; import { CommonModule } from "@angular/common"; @@ -22,12 +22,12 @@ import { takeUntilDestroyed } from "@angular/core/rxjs-interop"; SearchComponent, FontAwesomeModule, CommonModule, + RouterLinkWithHref, ], templateUrl: "./annotate.component.html", styleUrl: "./annotate.component.scss", }) export class AnnotateComponent implements OnInit { - private router = inject(Router); private route = inject(ActivatedRoute); private problemService = inject(ProblemService); private destroyRef = inject(DestroyRef); @@ -60,12 +60,4 @@ export class AnnotateComponent implements OnInit { this.problemService.allParams$.next({ params, queryParams, edit }); }); } - - public goToProblem(problemId: number): void { - this.router.navigate(["/", "annotate", problemId.toString()]); - } - - public addProblem(): void { - this.router.navigate(["/", "annotate", "new"]); - } } diff --git a/frontend/src/app/annotate/annotation-input/annotation-input.component.html b/frontend/src/app/annotate/annotation-input/annotation-input.component.html index 61890ac..5a1789d 100644 --- a/frontend/src/app/annotate/annotation-input/annotation-input.component.html +++ b/frontend/src/app/annotate/annotation-input/annotation-input.component.html @@ -39,18 +39,18 @@ Save problem } @else if (userProblem) { - + + + Edit problem + }
      } diff --git a/frontend/src/app/annotate/annotation-input/annotation-input.component.ts b/frontend/src/app/annotate/annotation-input/annotation-input.component.ts index b72b59b..04dd8fa 100644 --- a/frontend/src/app/annotate/annotation-input/annotation-input.component.ts +++ b/frontend/src/app/annotate/annotation-input/annotation-input.component.ts @@ -15,7 +15,7 @@ import { Dataset, KnowledgeBaseRelationship, Problem } from "../../types"; import { faCheck, faExclamationCircle, faFloppyDisk, faTrash, faTree, faWrench } from "@fortawesome/free-solid-svg-icons"; import { ProblemDetailsComponent } from "./problem-details/problem-details.component"; import { map, Subject } from "rxjs"; -import { ActivatedRoute, Router } from "@angular/router"; +import { ActivatedRoute, Router, RouterLinkWithHref } from "@angular/router"; import { ProblemService } from "@/services/problem.service"; import { ParseService } from "@/services/parse.service"; import { FontAwesomeModule } from "@fortawesome/angular-fontawesome"; @@ -48,6 +48,7 @@ export type ParseInput = ReturnType; ReactiveFormsModule, ProblemDetailsComponent, FontAwesomeModule, + RouterLinkWithHref ], templateUrl: "./annotation-input.component.html", styleUrl: "./annotation-input.component.scss", @@ -135,10 +136,6 @@ export class AnnotationInputComponent implements OnInit { this.problemService.submit$.next(input); } - public editProblem(): void { - this.router.navigate(["edit"], { relativeTo: this.route, queryParamsHandling: "preserve" }); - } - private navigateToNewProblem(problem: Problem | null): void { if (!problem || !problem.id) { return; diff --git a/frontend/src/app/annotate/navigator/navigator.component.html b/frontend/src/app/annotate/navigator/navigator.component.html index bb833c6..0b28368 100644 --- a/frontend/src/app/annotate/navigator/navigator.component.html +++ b/frontend/src/app/annotate/navigator/navigator.component.html @@ -3,11 +3,11 @@
      - - +
      @@ -52,11 +52,12 @@
      - - +
      diff --git a/frontend/src/app/annotate/navigator/navigator.component.ts b/frontend/src/app/annotate/navigator/navigator.component.ts index 59fc445..6ed1e45 100644 --- a/frontend/src/app/annotate/navigator/navigator.component.ts +++ b/frontend/src/app/annotate/navigator/navigator.component.ts @@ -8,18 +8,17 @@ import { faShuffle, } from "@fortawesome/free-solid-svg-icons"; import { CommonModule } from "@angular/common"; -import { Router } from "@angular/router"; +import { RouterLinkWithHref } from "@angular/router"; import { ProblemService } from "@/services/problem.service"; @Component({ selector: "la-navigator", standalone: true, - imports: [FontAwesomeModule, CommonModule], + imports: [FontAwesomeModule, CommonModule, RouterLinkWithHref], templateUrl: "./navigator.component.html", styleUrl: "./navigator.component.scss", }) export class NavigatorComponent { - private router = inject(Router); private problemService = inject(ProblemService); public problemResponse$ = this.problemService.problemResponse$; @@ -29,13 +28,4 @@ export class NavigatorComponent { public faAngleLeft = faAngleLeft; public faAngleRight = faAngleRight; public faShuffle = faShuffle; - - public navigateToProblem(id: number | null | undefined): void { - if (!id) { - return; - } - this.router.navigate(["/annotate", id], { - queryParamsHandling: "preserve", - }); - } } From a7b09f394c5714dc5c3a00d240e48cbb3a5c6315 Mon Sep 17 00:00:00 2001 From: Xander Vertegaal Date: Thu, 4 Dec 2025 22:47:09 +0100 Subject: [PATCH 40/42] Add template veriable for shorthand --- .../premises-form/premises-form.component.html | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/frontend/src/app/annotate/annotation-input/premises-form/premises-form.component.html b/frontend/src/app/annotate/annotation-input/premises-form/premises-form.component.html index e1b9780..2f91624 100644 --- a/frontend/src/app/annotate/annotation-input/premises-form/premises-form.component.html +++ b/frontend/src/app/annotate/annotation-input/premises-form/premises-form.component.html @@ -1,5 +1,6 @@ @let premiseControls = form().controls.premises.controls; @let appMode = appMode$ | async; +@let editingOrAdding = appMode === 'add' || appMode === 'edit';
      @@ -7,7 +8,7 @@ class="form-label h6 fw-bold d-flex align-items-center justify-content-between" > Premises - @if (appMode === 'add' || appMode === 'edit') { + @if (editingOrAdding) {