feat: add possibility to specify exact solutions for numerical questions #4419
Conversation
…s of numerical question
Current Aviator status
This PR was merged manually (without Aviator). Merging manually can negatively impact the performance of the queue. Consider using Aviator next time.
See the real-time status of this PR on the
Aviator webapp.
Use the Aviator Chrome Extension
to see the status of your PR within GitHub.
|
📝 WalkthroughWalkthroughThis pull request introduces comprehensive enhancements to the handling of numerical questions across multiple components and services. The primary focus is on adding support for "exact solutions" in numerical questions, which allows instructors to specify precise answer values in addition to solution ranges. The changes span frontend, backend, and infrastructure components, introducing new input methods, validation logic, evaluation processes, and GraphQL schema modifications to support this feature. Changes
Possibly Related PRs
Suggested Reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (37)
apps/analytics/src/notebooks/archive/daily_participant_analytics.ipynb (13)
46-48: Consider parameterizing the date
If you’d like this script to be run for arbitrary dates, consider passing the date as a function parameter or via a CLI argument rather than hardcoding it.
59-64: Potential large output
Printing all participant details for debugging might be unwieldy in production. Consider logging only a summary of the relevant information or limiting the output.
82-87: Consolidate or rename the function
The lines identifyingcourseIdfrom eitherpracticeQuizormicroLearningis correct. However, consider a more expressive function name or docstring formap_detailsto document its merging logic.
101-107: Naming and clarity
convert_to_dfis descriptive, but consider clarifying that it specifically handles participant response details. You might rename it toconvert_participant_responses_to_dffor clarity.
146-152: Graceful handling of missingoptions
You have a check for"options"inelementData, but be mindful of missing or malformed data. Consider adding error handling to avoid runtime exceptions ifoptionsis absent.
201-201: Minor note
Line 201 is a direct reference toelement_instance["type"] == "MC". Make sure you handle any newly introduced question types in the future.
204-208: Positive logic for correctness
Usingcorrectness == 1andcorrectness == 0is fine, but consider using a small threshold for floating-point comparisons if that’s a possibility.
209-214: Condition simplification
For KPRIM, the nested ternary might be less readable. Consider using if/elif blocks to enhance clarity.
216-225: Handle multiple numerical solution ranges
This chunk properly checks if any solution range matches. If you plan to handle exact solutions here, consider also checking for an “exactSolutions” field if that’s part of the PR’s objective.
231-234: TODO for FREE_TEXT
Implementation is not done yet. If FREE_TEXT usage is possible in production, consider raising a more descriptive error.
239-241: Column addition
df_details["correctness"] = ...is properly appended. Just ensure you handle NaN or None values in a standardized manner across the data pipeline.
371-375: Upsert usage
Upserting with a statictimestampis fine for daily analytics. Confirm that you don’t inadvertently overwrite earlier or future daily entries if the data updates happen multiple times for the same day.
379-391: Field name consistency
ThemeanPartialCorrectCountfield is derived frommeanPartialCount. The naming might confuse end users—consider a shorter or more consistent attribute name.apps/frontend-manage/src/components/questions/manipulation/useValidationSchema.ts (1)
167-288: Consider extracting sub-tests for improved maintainability.
This code segment includes numerous chained.testvalidations, making it lengthy and somewhat repetitive. While functionally sound, extracting these checks into helper functions or separate schemas could improve readability and maintainability.🧰 Tools
🪛 Biome (1.9.4)
[error] 179-179: Do not add then to an object.
(lint/suspicious/noThenProperty)
apps/frontend-manage/src/components/questions/manipulation/options/NumericalSolutionTypeSwitch.tsx (1)
1-46: Component design is clear; consider a radio group for scalability.
Having two buttons is effective. If more solution types are introduced in the future, a radio group might provide greater extensibility. Current code is otherwise well-organized.apps/frontend-manage/src/components/questions/manipulation/options/NumericalOptions.tsx (1)
4-6: Use dynamic imports only if needed.
The added imports forNumericalExactSolutionsInput,NumericalSolutionRangesInput, andNumericalSolutionTypeSwitchlook good. However, consider lazy-loading them if these components are heavy and not always necessary. This can improve initial load performance.apps/frontend-manage/src/components/questions/manipulation/options/NumericalExactSolutionsInput.tsx (3)
23-44: Ensure unique keys and correct array manipulation.
Usingindexfor the React key is acceptable for a static list. However, if items can reorder, consider using a unique ID to avoid key collisions.
45-52: Encourage user confirmation before deletion (optional).
The delete button works well. Consider a confirmation mechanism for users to avoid unintentional removals.
55-65: Consider setting default values on push.
When pushing new solutions, you might set a default value (e.g.,0) for clarity. This can prevent confusion if a user accidentally saves an unspecified exact solution.apps/frontend-manage/src/components/questions/manipulation/options/NumericalSolutionRangesInput.tsx (2)
21-61: Handle overlapping or incorrect range inputs.
When users input multiple ranges, consider validating that they do not overlap or specify an inverted range. This will improve the integrity of solution data.
62-74: Initialize new ranges with domain knowledge.
You might set[min, max] = [0, 0]or some other default to accelerate user entry. Blank fields could lead to incomplete data.apps/frontend-manage/src/components/questions/manipulation/useElementFormInitialValues.ts (2)
93-93: Use a fallback forsolutionTypewhenexactSolutionsarray is empty.
Currently,solutionTypefalls back to'range'ifoptions.exactSolutionsis falsy. If there's a scenario whereexactSolutionsexists but is empty—thus implying no valid solutions—consider enforcing a stricter type instead of defaulting to'range'. This would ensure accurate reflection of the solution state.
108-108: Consider initializingexactSolutionswith an empty array.
Storingundefinedmight complicate downstream usage, as it requires consumers to handle multiple cases (undefinedvs. empty array). Initializing it to an empty array ([]) offers a more consistent data structure for iteration and display logic.apps/frontend-manage/src/components/evaluation/elements/NRSidebar.tsx (2)
112-112: Use stable keys to prevent potential re-render issues.
Using the array index in the key can cause unexpected behaviors when the order ofsolutionRangeschanges. Consider deriving a stable identifier from the contents of therangeobject (e.g.,range.minorrange.max) to maintain React’s reconciliation process accurately.
118-129: Conditionally render the exact solutions block
IfexactSolutionsis an empty array, consider either hiding this block or displaying a hint to the user (e.g., "No exact solutions specified"). This helps avoid displaying an empty list in the UI.packages/grading/src/index.ts (1)
89-118: Provide partial credit for near-exact matches.
The code returnsnullif bothsolutionRangesandexactSolutionsare empty and returns0if the response doesn’t match. Consider a future enhancement to award partial credit when a response is near an exact solution (within a small delta).apps/analytics/src/modules/participant_analytics/compute_correctness.py (2)
72-80: Consider merging the range check logic more cleanly.Lines 73-80 introduce a new list comprehension to verify if the response value is within any of the provided solution ranges. The approach is correct but you could avoid multiple conversions of
range["min"]andrange["max"]to floats if performance becomes a concern. For large datasets, caching them outside the lambda or converting them once could be beneficial.
97-100: Double-check floating-point tolerance.The ±1e-10 tolerance is minimal and might be too strict for certain contexts (e.g., currency calculations). If your domain requires a broader tolerance or user-configurable margin, consider making it adjustable.
apps/frontend-manage/src/components/questions/manipulation/ElementFormErrors.tsx (1)
120-127: Check for array-based exactSolutions validation.Currently, the code only shows an error if
exactSolutionsis a string. IfexactSolutionsis expected to be an array as well, consider how to display multiple errors for different indices—similar to the approach used for solutionRanges.packages/graphql/src/schema/elementData.ts (1)
88-88: Make sure to document the new property.A short comment (or docstring) explaining how
exactSolutionsis intended to be used can help future maintainers. For instance, mention the expected tolerance logic on the consumer side.packages/shared-components/src/charts/ElementHistogram.tsx (1)
246-264: Add safety checks for solution parsing.When using
parseFloat(String(solution)), malformed or non-numeric strings might silently parse toNaN. If user input is allowed, consider gracefully handling invalid data or logging warnings to ensure the reference lines are drawn correctly.packages/graphql/src/services/questions.ts (1)
40-40: Ensure consistent default handling.Defaulting
exactSolutionstoundefinedis generally safe. If you intend to treat no solutions as an empty list, consider defaulting to[]for clarity in downstream checks.packages/graphql/src/services/liveQuizzes.ts (1)
963-967: Ensure consistent storage format in Redis.
StoringexactSolutionsvs.solutionRangeshere is correct, yet consider clarifying in the code docstring or comment how these solutions get retrieved and parsed later.packages/graphql/src/services/stacks.ts (1)
3218-3222: Ternary logic is valid but consider consolidating for easier reading.
You could unify into a single assignment with the fallback to an empty array. Current form is correct but slightly verbose.packages/graphql/src/graphql/ops/FStackFeedbackEvaluations.graphql (1)
67-67: Schema changes look consistent and well-structuredThe addition of the
exactSolutionsfield is consistently implemented across all relevant GraphQL fragments. The placement alongside existing solution-related fields is logical and maintains good schema organization.Consider adding:
- Documentation comments to describe the field's purpose and type
- Examples in the schema documentation showing how exact solutions work alongside solution ranges
- Deprecation strategy if this field is intended to eventually replace solution ranges
Also applies to: 76-76, 65-65
packages/graphql/src/public/schema.graphql (1)
1132-1132: Schema changes look good, consider adding field descriptions.The addition of
exactSolutionsfield to numerical-related types is consistent with the PR objective. Consider adding field descriptions to improve schema documentation.type NumericalElementResults { + """List of exact numerical values that are considered correct answers.""" exactSolutions: [Float!] } type NumericalInstanceEvaluation { + """List of exact numerical values that are considered correct answers for this instance.""" exactSolutions: [Float!] } type NumericalQuestionOptions { + """List of exact numerical values that are considered correct answers for this question.""" exactSolutions: [Float!] } input OptionsNumericalInput { + """List of exact numerical values to be considered as correct answers.""" exactSolutions: [Float!] }Also applies to: 1148-1148, 1168-1168, 1212-1212
packages/graphql/src/ops.schema.json (1)
17284-17303: Add descriptions for the newexactSolutionsfieldsThe new
exactSolutionsfields lack descriptions. Adding clear descriptions would help API consumers understand:
- The purpose of exact solutions vs other solution types
- The expected format and constraints of the values
- Any validation rules that apply
Add descriptions to each field. Example:
{ "name": "exactSolutions", - "description": null, + "description": "List of precise numerical values that are considered correct answers. Values are treated as exact matches during evaluation.", "args": [], ... }Also applies to: 17464-17483, 17724-17743, 18141-18160
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (40)
apps/analytics/src/modules/participant_analytics/compute_correctness.py(1 hunks)apps/analytics/src/notebooks/archive/daily_participant_analytics.ipynb(8 hunks)apps/frontend-manage/src/components/evaluation/ElementChart.tsx(1 hunks)apps/frontend-manage/src/components/evaluation/elements/NRSidebar.tsx(1 hunks)apps/frontend-manage/src/components/questions/manipulation/ElementFormErrors.tsx(1 hunks)apps/frontend-manage/src/components/questions/manipulation/helpers.ts(1 hunks)apps/frontend-manage/src/components/questions/manipulation/options/NumericalExactSolutionsInput.tsx(1 hunks)apps/frontend-manage/src/components/questions/manipulation/options/NumericalOptions.tsx(2 hunks)apps/frontend-manage/src/components/questions/manipulation/options/NumericalSolutionRangesInput.tsx(1 hunks)apps/frontend-manage/src/components/questions/manipulation/options/NumericalSolutionTypeSwitch.tsx(1 hunks)apps/frontend-manage/src/components/questions/manipulation/types.ts(1 hunks)apps/frontend-manage/src/components/questions/manipulation/useElementFormInitialValues.ts(2 hunks)apps/frontend-manage/src/components/questions/manipulation/useValidationSchema.ts(1 hunks)apps/func-response-processor/src/index.ts(1 hunks)cypress/cypress/e2e/D-questions-workflow.cy.ts(1 hunks)cypress/cypress/support/commands.ts(1 hunks)packages/grading/src/index.ts(1 hunks)packages/grading/test/index.test.ts(1 hunks)packages/graphql/src/graphql/ops/FElementData.graphql(1 hunks)packages/graphql/src/graphql/ops/FEvaluationResults.graphql(1 hunks)packages/graphql/src/graphql/ops/FStackFeedbackEvaluations.graphql(1 hunks)packages/graphql/src/graphql/ops/MManipulateNumericalQuestion.graphql(1 hunks)packages/graphql/src/graphql/ops/QGetSingleQuestion.graphql(1 hunks)packages/graphql/src/graphql/ops/QGetUserQuestions.graphql(1 hunks)packages/graphql/src/ops.schema.json(4 hunks)packages/graphql/src/public/client.json(6 hunks)packages/graphql/src/public/schema.graphql(4 hunks)packages/graphql/src/schema/elementData.ts(2 hunks)packages/graphql/src/schema/evaluation.ts(2 hunks)packages/graphql/src/schema/question.ts(2 hunks)packages/graphql/src/services/liveQuizzes.ts(2 hunks)packages/graphql/src/services/questions.ts(2 hunks)packages/graphql/src/services/stacks.ts(5 hunks)packages/i18n/messages/de.ts(4 hunks)packages/i18n/messages/en.ts(4 hunks)packages/prisma/src/data/data/TEST.ts(1 hunks)packages/shared-components/src/NumericalQuestion.tsx(1 hunks)packages/shared-components/src/charts/ElementHistogram.tsx(3 hunks)packages/shared-components/src/evaluation/NREvaluation.tsx(1 hunks)packages/types/src/index.ts(2 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
apps/frontend-manage/src/components/questions/manipulation/useValidationSchema.ts
[error] 163-163: Do not add then to an object.
(lint/suspicious/noThenProperty)
[error] 179-179: Do not add then to an object.
(lint/suspicious/noThenProperty)
[error] 301-301: Do not add then to an object.
(lint/suspicious/noThenProperty)
[error] 304-304: Do not add then to an object.
(lint/suspicious/noThenProperty)
🪛 Gitleaks (8.21.2)
packages/graphql/src/public/client.json
139-139: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🔇 Additional comments (83)
apps/analytics/src/notebooks/archive/daily_participant_analytics.ipynb (14)
51-52: Verify date filters
Ensure that filtering by createdAt is correct and aligns with user or system timezones. This can impact queries and data accuracy.
92-97: Potential memory overhead
map_participants creates lists and dictionaries. For large datasets, confirm that these transformations won’t lead to excessive memory usage. Consider using generators or chunked processing if necessary.
111-122: Selective columns
Extracting a subset of columns is good to keep the dataframe clean. Just ensure that any newly added columns in the DB are also included if needed.
136-138: Querying large datasets
Ensure that the list of elementInstanceId values is not excessively large. For extremely large sets, consider pagination or a staged approach to avoid potential performance bottlenecks.
164-168: Use caution on single row extraction
When you do .iloc[0], ensure that your dataset always has exactly one matching row. Otherwise, an IndexError could occur if df_element_instances is empty for some reason.
173-179: Single-choice logic
The SC-type correctness logic is straightforward. Just confirm that an empty choices array would not raise exceptions when referencing [0].
180-186: Confirm multi-choice structure
For “MC” and “KPRIM,” the structure of options["choices"] must have valid ix fields. If any are missing or inconsistent, it might cause misalignment when building selected_choices_array.
187-199: Hamming distance approach
The approach is sound for measuring differences. Just confirm that the range indexing will not break if ix in choices is out of range.
[approve]
237-237: Unknown element type
Raising a ValueError is correct. Ensure you have logging or monitoring in place to detect if this is thrown in production.
260-275: Aggregated columns
The groupby-agg structure looks clear. Keep in mind that count vs. nunique can yield different insights; ensure that’s the intended logic.
286-293: Pivoting correctness
Using .unstack(fill_value=0) for correctness categories is a solid approach. Confirm that no new correctness labels (like “EXACT”) are introduced in future expansions, or adapt the logic accordingly.
334-344: Flattened columns
Renaming flattened columns is correct. Just keep track of your naming conventions if you add new ones.
348-350: Possible mismatch
When merging dataframes, confirm that both dataframes have consistent participant/course pairs. Rows might disappear if they don’t match.
393-394: No update on upsert
Currently, the update block is empty. If you run this code repeatedly on the same day, it won’t update the existing record. Confirm this is intentional or add explicit update logic.
apps/frontend-manage/src/components/questions/manipulation/useValidationSchema.ts (2)
158-165: Confirm static analysis rule for ".when(...).then".
The code here is standard Yup usage, but static analysis flags line 163 for "noThenProperty". Verify whether that rule is appropriate or a false positive in your configuration. Functionally, this logic is correct for conditional validation.
🧰 Tools
🪛 Biome (1.9.4)
[error] 163-163: Do not add then to an object.
(lint/suspicious/noThenProperty)
290-338: Confirm static analysis rule for nested ".when(...).then".
Much like line 163, lines 301 and 304 are flagged by static analysis. This usage is a typical Yup pattern, so please verify if "noThenProperty" is indeed intended for your code style. Functionally, the nested condition checks are valid for handling "exact" solutions.
🧰 Tools
🪛 Biome (1.9.4)
[error] 301-301: Do not add then to an object.
(lint/suspicious/noThenProperty)
[error] 304-304: Do not add then to an object.
(lint/suspicious/noThenProperty)
cypress/cypress/e2e/D-questions-workflow.cy.ts (1)
Line range hint 827-844: Add coverage for 'exact' solution type.
Here, the "range" solution type is tested thoroughly. To ensure complete coverage, consider including a scenario where "exact" solution type is selected and validated.
packages/shared-components/src/evaluation/NREvaluation.tsx (1)
26-26: Prop introduction looks good.
Passing exactSolutions to ElementHistogram extends the histogram to handle exact solutions. Implementation appears correct, with minimal risk.
apps/frontend-manage/src/components/questions/manipulation/types.ts (1)
44-46: Verify need for string-based solutions.
The union (number | string) might introduce parsing complexities if the backend expects numeric data exclusively. Ensure that accepting a string type is intentional or revise to strictly accept numbers.
Also applies to: 51-51
apps/frontend-manage/src/components/questions/manipulation/options/NumericalOptions.tsx (2)
49-51: Consider default or fallback solutionType.
While setting up the NumericalSolutionTypeSwitch, make sure the solutionType has a valid default to avoid potential undefined states.
59-65: Check that precision is respected.
Passing String(values.options.accuracy) to NumericalExactSolutionsInput is correct as long as the receiving component properly parses it. Verify that users cannot input fractional decimals if the precision is set to 0.
apps/frontend-manage/src/components/questions/manipulation/options/NumericalExactSolutionsInput.tsx (2)
6-12: Support usability for an empty array.
The component expects exactSolutions by default. Consider gracefully handling scenarios where exactSolutions might be null or undefined to avoid runtime errors when the user has not specified any exact solutions yet.
15-22: Label and tooltip usage looks good.
Reusing localized strings for the label and tooltip ensures consistent translations. Great use of FormLabel and t().
packages/shared-components/src/NumericalQuestion.tsx (1)
93-93: Validate existence of exactSolutions.
Adding exactSolutions is beneficial. Ensure the absence of exactSolutions in older data or incomplete states is handled gracefully to avoid runtime errors in NREvaluation.
apps/frontend-manage/src/components/questions/manipulation/options/NumericalSolutionRangesInput.tsx (1)
15-20: Label and tooltip usage is consistent.
Reusing the design system’s FormLabel with an appropriate tooltip improves usability.
apps/frontend-manage/src/components/questions/manipulation/helpers.ts (1)
132-139: Handle edge cases for solution ranges.
The logic filters and maps solution ranges only if hasSampleSolution and solutionType === 'range'. If solutionRanges is empty (e.g., no valid ranges), ensure downstream usage can handle an empty array without unexpected side effects.
packages/grading/src/index.ts (1)
77-83: Optional fields should be consistently typed.
The updated interface allows solutionRanges and exactSolutions to be null. Downstream code needs to consistently handle these as null | undefined. Ensure that the rest of the codebase is aware of this optionality to avoid TypeError issues.
apps/analytics/src/modules/participant_analytics/compute_correctness.py (2)
82-86: Use consistent nomenclature between solution ranges and result states.
When the code finds any values within the range, it returns "CORRECT" immediately, else "INCORRECT". This is logically sound. Just ensure that external labeling or logging references “ranges” consistently as well, so there's no confusion about the term in future expansions.
87-96: Evaluate fallback logic when both solutionRanges & exactSolutions coexist.
Here, the code checks elif "exactSolutions" in options: only if solutionRanges isn't present, which might cause unexpected fallback if both solutionRanges and exactSolutions are defined. Consider whether you need to check both conditions or apply a priority in the same block.
apps/frontend-manage/src/components/questions/manipulation/ElementFormErrors.tsx (2)
93-100: Ensure translation alignment for solutionType errors.
Adding solutionType error display is good. Double-check that the “solutionTypeNumerical” key always points to a relevant localized string. If future solution types are added, you might need dynamic key resolution or additional i18n mapping.
105-119: Sanitize data when handling solutionRanges as string vs. array.
Lines 105-119 handle solutionRanges as either a single string or an array. Ensure the upstream form logic sets the correct data type. If user input is invalid JSON or inconsistent format, the UI might break or messages might not map properly. Consider adding a fallback or more robust type checks.
packages/graphql/src/schema/elementData.ts (1)
111-111: Validate float list usage.
By exposing exactSolutions as a float list, ensure clients passing integer-based solutions (or strings) handle the type conversion properly. Any mismatch might cause build- or runtime-level issues in strict schema environments.
packages/shared-components/src/charts/ElementHistogram.tsx (2)
Line range hint 24-47: Optional property handling for exactSolutions.
Defining exactSolutions?: (number | string)[] | null is flexible. Make sure any logic that consumes this property can handle mixed numeric and string data gracefully (e.g., verifying or parsing string-based user inputs).
237-237: Localization of the label.
Replacing hardcoded "Korrekt" with t('manage.evaluation.correctLabel') ensures consistent localization. Confirm that the translation key manage.evaluation.correctLabel exists in all relevant languages.
packages/types/src/index.ts (2)
174-174: Looks good!
Adding exactSolutions?: number[] aligns well with the existing solutionRanges. This optional array of numbers will allow more granular or even discrete solution checks. Ensure that any usage sites handle undefined or empty arrays properly.
341-341: Maintain consistency with ElementOptionsNumerical.
The new exactSolutions?: number[] property consistently mirrors the definition in ElementOptionsNumerical. Verify that the rest of the code consistently handles null/undefined scenarios and type coercions for numeric fields.
packages/graphql/src/schema/evaluation.ts (2)
82-82: Optional chaining for consistent usage.
Defining exactSolutions?: number[] | null ensures flexibility in scenarios without exact solutions. Double-check any downstream usage to gracefully handle null or empty arrays, especially when enumerating or comparing solution values.
283-283: Exposing float list matches typed property.
Using t.exposeFloatList('exactSolutions', { nullable: true }) matches the optional numeric array declared in the interface. This is a clean, consistent approach to GraphQL schema exposure.
apps/func-response-processor/src/index.ts (2)
244-249: Condition detection for exact solutions.
Here you check if the parsed solutions array has valid entries, including numeric or string data. This logic is correct for toggling between solutionRanges and exactSolutions. However, you may want to enforce numeric-only validation to avoid unexpected behavior when a string is not parseable into a number.
252-253: Conditional usage of solutionRanges vs. exactSolutions.
Setting solutionRanges to undefined when exactSolutions is present prevents ambiguity in the grading logic. Confirm that this is consistent with the downstream grading function. Otherwise, combining the two could lead to unexpected logic merges.
packages/grading/test/index.test.ts (8)
160-166: Test with empty solutionRanges.
The test assures the function returns null when no solution ranges are provided. This scenario effectively checks the absence of valid solutions, confirming that no correctness points can be awarded. Good coverage here!
167-172: Testing single exact solution with a matching response.
This test ensures a perfect match yields a score of 1. Very straightforward coverage that confirms the “exact match” logic works.
173-178: Testing single exact solution with a non-matching response.
Properly verifies the zero score scenario. Confirms that a response outside the list yields 0.
179-184: Multiple exact solutions handled correctly.
Including both 0 and 100 as valid answers ensures that a response matching any one of them scores 1. Good expansion of test coverage.
185-190: Confirm large boundary handling.
Allows for verifying extremes (e.g., 100) to ensure no floating-point edge cases break the logic.
191-196: Rejecting midpoint when only extremes are exact.
A response of 50 is rightly scored 0 in this scenario. Great test for partial-range confusion.
197-202: Handling minuscule variations.
Ensuring that 1e-30 matches 0 within the logic is a strong check for floating-point comparison behavior. Proceed with caution if stricter equality checks might be needed.
203-208: Empty exact solutions array returns null.
This confirms that an empty array is effectively “no exact solutions,” returning null for no match scenario. Good coverage.
packages/prisma/src/data/data/TEST.ts (1)
174-191: Potential duplication and type mismatch concern.
You're duplicating a question with originalId: '2' and adding exactSolutions as string values. Since this question is of type NUMERICAL, ensure that consuming code expects these solutions as strings rather than numbers. Also verify that having two entries with the same originalId is intended, as it may cause confusion in lookups or references.
packages/graphql/src/schema/question.ts (2)
93-93: Flexibility for optional exact solutions looks good.
Introducing exactSolutions as a float list is a straightforward way to handle precise answers for numerical questions. If needed, consider adding validation to ensure values fall within permitted ranges.
257-257: Expose float list for exact solutions.
Exposing exactSolutions in NumericalInstanceEvaluation is consistent with the input type changes. Please verify the handling of empty or null lists to avoid unexpected evaluation logic.
packages/graphql/src/services/questions.ts (1)
166-166: String vs number mismatch.
Your exactSolutions property is typed as number[], but in the seed data (TEST.ts), it's an array of strings. Verify that the consumer code correctly handles numeric parsing or that the source data should indeed be numeric values.
cypress/cypress/support/commands.ts (1)
374-374: Integration test coverage for solution ranges.
Clicking [data-cy="set-solution-type-range"] provides a new solution type for numerical questions. Ensure you have end-to-end tests that confirm correct behavior of both solution ranges and exact solutions in subsequent steps (e.g., validating user submissions against these constraints).
packages/graphql/src/services/liveQuizzes.ts (2)
159-161: Consider verifying that solutions only contains homogenous solution types.
If the array can mix objects or other types, relying solely on the first element's type might be fragile. You may want to ensure consistently typed solutions or at least validate all elements for numeric/string types.
165-166: Looks good, but ensure downstream code handles this new logic properly.
When exactSolutionsDefined is true, solutionRanges is set to undefined. Verify all usage sites expect an undefined instead of an empty array.
packages/graphql/src/services/stacks.ts (3)
1156-1156: Approach to add 'exactSolutions' property looks solid.
This properly extends NumericalEvaluationReturnType to cover exact solutions.
1257-1257: Make sure an empty array is acceptable when no exact solutions are provided.
This default fallback can prevent errors, but confirm whether undefined was previously used to indicate no exact solutions.
2881-2881: Verify dependent logic now that exactSolutions is part of the returned object.
If calculation or UI code previously expected only solutionRanges, confirm that the presence of exactSolutions doesn't break existing features.
packages/i18n/messages/en.ts (9)
993-995: New strings for exact solutions are clearly described.
Ensure translations exist for other supported languages, if applicable.
996-998: Useful clarity in the tooltip about choosing solution type.
No issues spotted—just confirm these labels match any updated UI elements.
1000-1000: "Add Exact Solution" label is consistent.
Looks good for a user-facing hint.
1323-1323: Clear message about having at least one solution or disabling sample solutions.
This helps avoid confusion for free-text or numerical questions if no solutions are defined.
1328-1329: Validation message clarifies boundary values for solution ranges.
No problems noted—straightforward guidance for numeric question authors.
1331-1332: Provides instructive advice for choosing solution type.
The text is concise and consistent with the new logic.
1334-1334: "exactSolutionRequired" offers straightforward error guidance.
Ensures question authors do not forget to add at least one exact solution.
1344-1347: Messages for ensuring solutions lie within restrictions are correct.
This consistency check is crucial for robust numeric question validation.
1472-1473: Refined labeling for "correct" status.
Good for clarifying how correct values are displayed, especially in a chart legend.
packages/i18n/messages/de.ts (7)
992-997: Ensure consistency of newly added tooltips for exact solutions.
The added fields for exact solutions and solution types are clearly named and localized. Ensure the tooltip style and phrasing align with existing tooltips for numerical question fields, keeping the messages succinct to prevent UI layout issues.
999-999: Translation addition looks good.
“Neue exakte Lösung hinzufügen” is consistent with the rest of the translations.
1337-1338: Confirm that placeholders match the design system.
The error message texts for numerical solution requirements look consistent. Verify that placeholders (like “NROneValueRequired”) are used correctly in the UI.
1340-1341: Clear messaging for solution requirements.
“Bitte geben Sie mindestens einen Lösungsbereich an.” and “Bitte geben Sie mindestens eine exakte Lösung an.” effectively guide the user. No issues found.
1344-1345: Exact solution requirement text is concise.
The newly added translation properly conveys that an exakte Lösung is mandatory.
1356-1359: Further clarifies solution restrictions.
The new lines enforce that both solution ranges and exact solutions must lie within the stated restrictions, helping avoid user confusion.
1485-1486: No functional issues with “Korrekt” labels.
The additional localized label for correctness matches the existing style.
packages/graphql/src/graphql/ops/MManipulateNumericalQuestion.graphql (1)
43-43: Field addition for exact solutions in GraphQL mutation.
Adding exactSolutions to the mutation output ensures the UI can retrieve and display these values. This maintains consistency with the broader introduction of exact solutions across the application.
packages/graphql/src/graphql/ops/FElementData.graphql (1)
46-46: “exactSolutions” field addition aligns with updated numerical model.
The new field in the NumericalElementData fragment correctly expands the options for numerical questions.
packages/graphql/src/graphql/ops/QGetSingleQuestion.graphql (1)
58-58: Inclusion of “exactSolutions” in the query response.
This change ensures that when fetching a single numerical question, exact solutions are accessible to the client. Good practice to keep this in sync with the rest of the GraphQL schema changes.
packages/graphql/src/graphql/ops/QGetUserQuestions.graphql (1)
67-67: Please specify the type of exactSolutions field
The exactSolutions field has been added, but its type is not immediately clear from the schema. Based on the feature's purpose, this should likely be an array of numbers.
Let's verify the type definition and its consistency across the schema:
✅ Verification successful
Based on the search results, I can now provide a definitive response about the type of exactSolutions:
exactSolutions is an array of floating-point numbers
The GraphQL schema in packages/graphql/src/public/schema.graphql clearly defines the field type as [Float!], and this is consistently used across the codebase:
- In the schema:
exactSolutions: [Float!] - In TypeScript interfaces:
exactSolutions?: number[] - In test data: Used with numeric values like
[0],[0, 100]
The field is correctly positioned in the schema between other solution-related fields and is used consistently across numerical question implementations.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for exactSolutions type definition across schema files
rg -A 1 "exactSolutions" packages/graphql/src/graphql/schema/
Length of output: 138
Script:
#!/bin/bash
# Let's try searching in the entire repository for exactSolutions type definitions
rg -A 3 "exactSolutions.*:"
# Also search for any GraphQL type definitions containing exactSolutions
ast-grep --pattern 'type $_ {
$$$
exactSolutions
$$$
}'
# Search for GraphQL input type definitions containing exactSolutions
ast-grep --pattern 'input $_ {
$$$
exactSolutions
$$$
}'
Length of output: 74868
packages/graphql/src/graphql/ops/FEvaluationResults.graphql (1)
76-76: Verify integration with statistics calculation
The exactSolutions field has been added to the evaluation results. Please ensure that:
- The statistics calculation (max, mean, median, etc.) properly handles cases with exact solutions
- The
responseValues.correctfield correctly evaluates responses against both solution ranges and exact solutions
Let's verify the evaluation logic:
✅ Verification successful
Let's gather more information about the evaluation logic by examining the relevant files.
Let's check one more thing to understand how the correctness is evaluated with exact solutions.
Integration with statistics and evaluation is properly handled
Based on the code analysis, the exactSolutions field is well integrated into the evaluation system:
- The evaluation logic in
evaluateNumericalAnswerCorrectnessconsiders both solution ranges and exact solutions - The correctness value (0, 1, or partial) is properly computed and used to:
- Update response values with correct/incorrect status
- Calculate statistics through
computeNumericalStatistics - Set appropriate feedback status (CORRECT/INCORRECT/PARTIAL)
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for files handling numerical evaluation logic
rg -l "NumericalElement.*[Ee]valuation|compute.*correctness" packages/
Length of output: 618
Script:
#!/bin/bash
# Check evaluation logic in schema and implementation
echo "=== Schema Definition ==="
rg -A 10 "type NumericalElementInstanceEvaluation" packages/graphql/src/public/schema.graphql
echo -e "\n=== Evaluation Implementation ==="
rg -A 10 "computeCorrectness|evaluateNumerical" packages/graphql/src/services/stacks.ts
echo -e "\n=== Response Processing ==="
rg -A 5 "responseValues.*correct|exactSolutions" packages/graphql/src/schema/evaluation.ts
Length of output: 2479
Script:
#!/bin/bash
# Look for exact solution evaluation logic
rg -A 10 "evaluateNumericalAnswerCorrectness|exactSolutions" packages/graphql/src/services/stacks.ts
Length of output: 2261
packages/graphql/src/graphql/ops/FStackFeedbackEvaluations.graphql (1)
65-65: Verify feedback handling for exact solutions
The exactSolutions field has been added to the feedback structure. Please ensure that:
- The feedback generation logic properly handles exact solutions
- The UI components correctly display feedback for exact solutions
- The
correctnessfield calculation considers both solution ranges and exact solutions
Let's verify the feedback handling:
✅ Verification successful
Based on the code analysis, I can now provide a final response:
Feedback handling for exact solutions is properly implemented
The codebase shows complete implementation of exact solutions handling:
-
The grading logic in
packages/grading/src/index.tsproperly handles exact solutions:- Checks for existence of exact solutions
- Uses epsilon comparison for floating-point numbers
- Returns correct score (1 for match, 0 for no match)
-
The UI components correctly display feedback:
NREvaluationcomponent in shared components handles both solution ranges and exact solutions- Evaluation results are properly passed through the component hierarchy
- The histogram visualization includes exact solutions markers
-
The correctness calculation considers both solution types:
- Prioritizes solution ranges if present
- Falls back to exact solutions comparison
- Properly handles null/undefined cases
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for feedback-related components and logic
rg -l "Numerical.*Feedback|feedback.*numerical" apps/frontend-*/src/
Length of output: 125
Script:
#!/bin/bash
# Let's try a broader search for numerical evaluation and feedback handling
rg -l "NumericalInstance|exactSolutions?" --type ts --type tsx
# Also search for any feedback evaluation components
rg -l "FeedbackEvaluation" --type ts --type tsx
# Search for numerical feedback patterns in GraphQL files
rg "type Numerical.*Evaluation|exactSolutions?" --type graphql -A 3
Length of output: 270
Script:
#!/bin/bash
# Search for numerical evaluation and feedback handling without file type restrictions
rg -l "NumericalInstance|exactSolutions?"
# Search for feedback evaluation components
rg -l "FeedbackEvaluation"
# Search for numerical feedback patterns in all files
rg "type Numerical.*Evaluation|exactSolutions?" -A 3
Length of output: 84413
Script:
#!/bin/bash
# Let's check for feedback handling implementation in the codebase
rg "gradeQuestionNumerical|NumericalInstanceEvaluation" packages/grading/src/
# Also check for numerical evaluation components
rg "NumericalEvaluation|exactSolutions.*correctness" packages/shared-components/src/
Length of output: 237
Script:
#!/bin/bash
# Let's examine the grading implementation for numerical questions
cat packages/grading/src/index.ts
# Also check for evaluation components in shared components
rg -A 5 "NREvaluation" packages/shared-components/src/
Length of output: 8421
packages/graphql/src/ops.schema.json (1)
17284-17303: Verify schema structure and consider adding validation constraints
The schema changes look correct for adding exact solutions support. However, let's verify the complete schema structure and consider any validation needs.
Run this script to analyze the schema structure:
Consider adding:
- Input validation directives if you need to enforce constraints (e.g., non-empty list, value ranges)
- Clear error messages for validation failures
- Documentation about how exact solutions interact with existing solution ranges
Also applies to: 17464-17483, 17724-17743, 18141-18160
✅ Verification successful
Let me generate additional scripts to check for any validation-related directives and input types that might be relevant:
Let me check for any numerical validation types and restrictions:
Schema structure is consistent with existing numerical validation patterns
The schema already has appropriate validation structures in place:
NumericalRestrictionsInputtype exists for handling numerical constraintsSolutionRangeInputwith min/max fields is used for validating ranges- The
exactSolutionsfield is consistently defined as a non-null list of Float values
The schema changes follow the existing patterns for numerical validation and don't require additional validation directives since:
- The type is properly non-nullable (
NON_NULL) - It's contained within a list type (
LIST) - It uses the same
Floatscalar type as other numerical fields - It integrates with existing numerical validation structures
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Analyze the schema structure and related types
# Check if exactSolutions is consistently defined across related types
rg -A 5 '"name": "exactSolutions",' packages/graphql/src/
# Look for any existing validation patterns we should follow
rg -B 5 '"name": "Float",' packages/graphql/src/
# Check for any existing numerical validation directives
ast-grep --pattern 'directive @validateNumber'
Length of output: 45893
Script:
#!/bin/bash
# Check for any validation directives in the schema
rg -B 2 -A 2 'directive @' packages/graphql/src/
# Look for input types related to exact solutions
rg -B 5 -A 5 '"kind": "INPUT_OBJECT"' packages/graphql/src/
# Check for any min/max validation patterns
rg -A 5 '"name": "(min|max)",' packages/graphql/src/
Length of output: 37949
Script:
#!/bin/bash
# Check for numerical validation types and restrictions
rg -B 5 -A 10 '"name": "NumericalRestrictionsInput",' packages/graphql/src/
# Look for solution range input type definition
rg -B 5 -A 10 '"name": "SolutionRangeInput",' packages/graphql/src/
# Check for any validation-related types
rg -B 2 -A 5 '"name": ".*Validation.*",' packages/graphql/src/
Length of output: 4337
apps/frontend-manage/src/components/evaluation/ElementChart.tsx (1)
53-53: LGTM: Properly handles exact solutions for numerical questions
The addition of exactSolutions prop with a null coalescing operator follows the existing pattern and safely handles undefined values.
packages/graphql/src/public/client.json (1)
Line range hint 19-166: Hash updates look consistent with schema changes
The updated operation hashes reflect the schema changes needed to support exact solutions for numerical questions.
However, please note:
Let's verify the GraphQL schema changes are consistent:
✅ Verification successful
Hash updates correctly reflect schema changes for numerical questions
The hash updates in client.json are consistent with the GraphQL schema changes. The schema shows:
NumericalElementResultstype includes the newexactSolutionsfieldManipulateNumericalQuestionmutation includesexactSolutionsin its options- Evaluation fragments properly handle the new
exactSolutionsfield in numerical question results
The updated operation hashes in client.json correspond to these schema changes, particularly:
- ManipulateNumericalQuestion
- GetLiveQuizEvaluation
- GetMicroLearningEvaluation
- Other related evaluation queries
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for related GraphQL schema changes
# Look for exact solutions in schema
rg -l "exactSolutions?" --type graphql
# Look for numerical question type definitions
ast-grep --pattern 'type NumericalElement'
Length of output: 117
Script:
#!/bin/bash
# Description: Check for related GraphQL schema changes with different approaches
# Look for exact solutions in schema files (trying different extensions)
rg -l "exactSolutions?" -g "*.graphql" -g "*.gql"
# Look for schema definitions in common locations
fd -e graphql -e gql
# Try finding numerical question related content
rg -l "NumericalElement|NumericalQuestion"
# Look for schema files with common naming patterns
fd "schema|types" -e graphql -e gql
Length of output: 14395
Script:
#!/bin/bash
# Let's examine the schema and operations for exact solutions implementation
# Check the schema definition for numerical questions
rg -A 10 "type NumericalElement" packages/graphql/src/public/schema.graphql
# Look at the numerical question mutation
cat packages/graphql/src/graphql/ops/MManipulateNumericalQuestion.graphql
# Examine evaluation related changes
cat packages/graphql/src/graphql/ops/FEvaluationResults.graphql
Length of output: 4575
🧰 Tools
🪛 Gitleaks (8.21.2)
139-139: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
apps/frontend-manage/src/components/questions/manipulation/options/NumericalOptions.tsx
Show resolved
Hide resolved
...ontend-manage/src/components/questions/manipulation/options/NumericalSolutionRangesInput.tsx
Show resolved
Hide resolved
klicker-uzh
|
||||||||||||||||||||||||||||
| Project |
klicker-uzh
|
| Branch Review |
exact-numerical-solutions
|
| Run status |
|
| Run duration | 13m 20s |
| Commit |
|
| Committer | Julius Schlapbach |
| View all properties for this run ↗︎ | |
| Test results | |
|---|---|
Failures
|
0
|
Flaky
|
0
|
Pending
|
0
|
Skipped
|
0
|
Passing
|
174
|
| View all changes introduced in this branch ↗︎ | |
…r numerical questions
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
cypress/cypress/e2e/D-questions-workflow.cy.ts (1)
921-964: LGTM with suggestions for additional test coverage.The test cases thoroughly validate the exact solutions functionality. Consider adding tests for:
- Duplicate exact solutions (e.g., adding the same value twice)
- Decimal/fractional exact solutions
Example test cases to add:
it('Should handle duplicate exact solutions', () => { // ... existing setup code ... // Try to add the same solution twice const duplicateValue = 25 cy.get('[data-cy="add-exact-solution"]').click() cy.get(`[data-cy="set-exact-solution-0"]`).click().type(String(duplicateValue)) cy.get('[data-cy="add-exact-solution"]').click() cy.get(`[data-cy="set-exact-solution-1"]`).click().type(String(duplicateValue)) cy.get('[data-cy="save-new-question"]').should('be.disabled') // or handle according to requirements }) it('Should handle decimal exact solutions', () => { // ... existing setup code ... // Add decimal solutions const decimalValue = 25.5 cy.get('[data-cy="add-exact-solution"]').click() cy.get(`[data-cy="set-exact-solution-0"]`).click().type(String(decimalValue)) cy.get('[data-cy="save-new-question"]').should('not.be.disabled') })
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/frontend-manage/src/components/evaluation/elements/Statistic.tsx(1 hunks)cypress/cypress/e2e/D-questions-workflow.cy.ts(4 hunks)
✅ Files skipped from review due to trivial changes (1)
- apps/frontend-manage/src/components/evaluation/elements/Statistic.tsx
🔇 Additional comments (3)
cypress/cypress/e2e/D-questions-workflow.cy.ts (3)
54-54: LGTM: Test data constant follows conventions.
The constant NRSolutionsExact is well-defined with appropriate test values.
826-829: LGTM: Proper validation for solution type selection.
The test correctly verifies that:
- The save button is disabled until a solution type is selected
- The user must explicitly choose between range and exact solutions
845-871: LGTM: Comprehensive boundary testing for solution ranges.
The test suite effectively validates that:
- Solution ranges cannot extend below the minimum restriction
- Solution ranges cannot extend above the maximum restriction
- The UI properly handles invalid inputs and cleanup
|
klicker-uzh
|
||||||||||||||||||||||||||||
| Project |
klicker-uzh
|
| Branch Review |
v3-new-elements
|
| Run status |
|
| Run duration | 13m 20s |
| Commit |
|
| Committer | Julius Schlapbach |
| View all properties for this run ↗︎ | |
| Test results | |
|---|---|
|
Failures
|
0
|
|
Flaky
|
0
|
|
Pending
|
0
|
|
Skipped
|
0
|
|
Passing
|
174
|
| View all changes introduced in this branch ↗︎ | |
Summary by CodeRabbit
Release Notes
New Features
Improvements
User Interface
Internationalization