fix: use full combined results for student evaluation on asynchronous activities #4341
Conversation
…used for student evaluation statistics
📝 Walkthrough📝 WalkthroughWalkthroughThis pull request introduces significant modifications to the GraphQL schema and related components, primarily focused on restructuring the Changes
Possibly related PRs
Suggested reviewers
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (10)
🚧 Files skipped from review as they are similar to previous changes (2)
🔇 Additional comments (11)packages/shared-components/src/evaluation/MCKPRIMEvaluation.tsx (1)
The transition from Object.entries to direct array mapping with structured choice objects is a good improvement. The new implementation:
Let's verify that the new choice structure is consistently used across the codebase: packages/graphql/src/schema/question.ts (1)
The implementation of packages/graphql/src/public/schema.graphql (1)
The schema changes from JSON to structured types might affect existing queries. Also applies to: 665-665, 1132-1132 ✅ Verification successfulSchema changes appear to be compatible with existing queries Based on the search results, the schema changes are safe as the existing queries are already using structured types:
choices {
ix
correct
}
responses {
id
content
}The schema changes maintain backward compatibility with these existing query patterns. 🏁 Scripts executedThe following scripts were executed for the analysis: Script: #!/bin/bash
# Search for GraphQL queries/mutations using the modified fields
# Description: Look for queries that might need updates due to schema changes
# Check for queries accessing choices field
rg -t graphql "choices\s*{"
# Check for queries accessing answers field
rg -t graphql "answers\s*{"
# Check for queries accessing responses field
rg -t graphql "responses\s*{"
Length of output: 353 Script: #!/bin/bash
# Search for GraphQL queries/mutations using the modified fields
# Description: Look for queries that might need updates due to schema changes
# Check for queries accessing choices field
rg -t graphql "choices" -A 2
# Check for queries accessing answers field
rg -t graphql "answers" -A 2
# Check for queries accessing responses field
rg -t graphql "responses" -A 2
Length of output: 184 Script: #!/bin/bash
# Search for GraphQL queries/mutations using the modified fields
# Description: Look for queries that might need updates due to schema changes
# Find all GraphQL files first
fd "\.(graphql|gql)$" | while read -r file; do
echo "=== Checking $file ==="
# Check for queries accessing choices field
rg "choices" "$file" -A 2 || true
# Check for queries accessing answers field
rg "answers" "$file" -A 2 || true
# Check for queries accessing responses field
rg "responses" "$file" -A 2 || true
done
Length of output: 86799 packages/graphql/src/services/stacks.ts (2)
The type definition for
The function correctly combines participant and anonymous results for choices evaluation. packages/graphql/src/public/server.json (2)
This is a duplicate of the StackFeedbackEvaluations fragment reviewed above.
The changes to restructure response handling in evaluation types look good. The fragment now consistently tracks response counts across different question types, improving data consistency and analysis capabilities. Let's verify the fragment usage across the codebase: ✅ Verification successfulThe search results show that the
Let's verify if all these instances are consistent: Response handling structure is consistent across all usages The verification confirms that the
🏁 Scripts executedThe following scripts were executed for the analysis: Script: #!/bin/bash
# Description: Verify all usages of StackFeedbackEvaluations fragment to ensure consistent implementation
# Test: Search for fragment usage. Expect: Only occurrences with the new response structure.
rg -A 5 "fragment StackFeedbackEvaluations"
Length of output: 15072 Script: #!/bin/bash
# Get the full content of the source fragment file to compare with the compiled versions
cat packages/graphql/src/graphql/ops/FStackFeedbackEvaluations.graphql
Length of output: 2520 packages/graphql/src/ops.schema.json (3)
The non-null list type definition for SingleChoiceResponse follows GraphQL schema best practices and ensures type safety.
The type definition maintains consistency with other response types and ensures proper type safety.
The addition of the 'responses' field with SingleNumericalResponse type aligns with the PR objective of improving student evaluation. However, we should verify that all consumers of the old 'answers' field have been updated. ✅ Verification successfulLooking at the search results, I can see that in the TypeScript definitions in Let me verify if there are any other references to the old field by searching with a broader pattern: Let me search for any GraphQL schema definitions to get a complete picture: Let me try a different approach to search the schema files: Based on all the search results, I can now provide a final response: Migration from 'answers' to 'responses' field is correctly implemented The schema definition and all GraphQL operations consistently use the new 'responses' field in NumericalInstanceEvaluation type. There are no remaining references to an 'answers' field for this type, confirming the migration is complete and consistent across the codebase. 🏁 Scripts executedThe following scripts were executed for the analysis: Script: #!/bin/bash
# Search for any remaining references to the old 'answers' field in GraphQL queries/mutations
rg -t graphql 'answers.*NumericalInstanceEvaluation'
# Search for any TypeScript/JavaScript files that might need updates
rg -t ts -t js 'answers.*NumericalInstanceEvaluation'
Length of output: 11287 Script: #!/bin/bash
# Search for any GraphQL schema files
fd schema.graphql
# Search for any remaining references to 'answers' field specifically in NumericalInstanceEvaluation
rg "NumericalInstanceEvaluation.*answers"
rg "answers.*NumericalInstanceEvaluation"
Length of output: 49053 Script: #!/bin/bash
# Search for NumericalInstanceEvaluation type definition in schema files
rg -t graphql -t gql "type NumericalInstanceEvaluation"
# Also check for any remaining references to 'answers' in schema files
rg -t graphql -t gql "answers.*NumericalInstanceEvaluation|NumericalInstanceEvaluation.*answers"
Length of output: 220 Script: #!/bin/bash
# Find and search schema files without relying on file type
fd "schema\.(graphql|gql)" -x rg -l "type NumericalInstanceEvaluation" {}
# Search in the specific schema file we know exists
cat packages/graphql/src/public/schema.graphql | grep -A 20 "type NumericalInstanceEvaluation"
Length of output: 706 packages/types/src/index.ts (1)
In 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 (
|
klicker-uzh
|
||||||||||||||||||||||||||||
| Project |
klicker-uzh
|
| Branch Review |
v3-new-live-quiz
|
| Run status |
|
| Run duration | 11m 17s |
| Commit |
|
| Committer | Julius Schlapbach |
| View all properties for this run ↗︎ | |
| Test results | |
|---|---|
Failures
|
0
|
Flaky
|
0
|
Pending
|
0
|
Skipped
|
0
|
Passing
|
140
|
| View all changes introduced in this branch ↗︎ | |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's limits when posting large numbers of comments.
Actionable comments posted: 8
🧹 Outside diff range and nitpick comments (5)
packages/shared-components/src/evaluation/NREvaluation.tsx (1)
Line range hint 1-41: Consider adding TypeScript type for responses.
While the component works correctly, it would be beneficial to explicitly type the shape of the response data for better type safety and documentation.
Consider adding a type definition:
interface NumericalResponse {
value: number
count: number
}And then update the prop types:
interface NREvaluationProps {
options: NumericalQuestionOptions
- evaluation: NumericalInstanceEvaluation
+ evaluation: NumericalInstanceEvaluation & {
+ responses: NumericalResponse[]
+ }
reference?: string
}packages/graphql/src/schema/practiceQuizzes.ts (1)
Line range hint 71-84: Update StackFeedback implementation to remove IStackFeedback reference.
The StackFeedback object reference still uses the IStackFeedback type which is being removed. The implementation should be updated to directly define the type structure.
Apply this diff to update the implementation:
-export const StackFeedback = builder
- .objectRef<IStackFeedback>('StackFeedback')
- .implement({
+export const StackFeedback = builder
+ .objectRef('StackFeedback')
+ .implement({
fields: (t) => ({
id: t.exposeInt('id'),
status: t.expose('status', { type: StackFeedbackStatus }),
score: t.exposeInt('score', { nullable: true }),
evaluations: t.expose('evaluations', {
type: [InstanceEvaluation],
nullable: true,
}),
}),
})packages/graphql/src/schema/evaluation.ts (1)
19-24: Add JSDoc documentation for the new interface.
Consider adding documentation to explain the purpose and usage of this interface, especially since it's part of a student evaluation fix.
Example documentation:
+/**
+ * Represents feedback for a stack of questions in student evaluation.
+ * @property {number} id - Unique identifier for the stack feedback
+ * @property {StackFeedbackStatusType} status - Current status of the feedback
+ * @property {number} [score] - Optional numerical score for the stack
+ * @property {IInstanceEvaluation[]} [evaluations] - Optional array of instance evaluations
+ */
export interface IStackFeedback {packages/graphql/src/ops.schema.json (1)
Line range hint 7488-22603: Schema changes improve response aggregation capabilities.
The restructuring from 'answers' to 'responses' with the new types provides better support for aggregating student responses in asynchronous activities. The consistent pattern between numerical and text responses makes the API more intuitive.
Consider these operational aspects:
- Ensure proper migration of existing data to the new structure
- Update client applications to handle the new response format
- Consider adding schema deprecation notices for the old 'answers' field if still present
packages/graphql/src/services/stacks.ts (1)
2228-2232: Possible redundancy due to multiple evaluations
Consider removing duplicate evaluation calls if they are redundant. Ensure that the evaluation logic is streamlined to prevent unnecessary computations.
[duplication_comment]
🛑 Comments failed to post (8)
packages/graphql/src/schema/practiceQuizzes.ts (1)
8-8:
⚠️ Potential issueRemove unused import of IStackFeedback.
According to the changes, the
IStackFeedbackinterface has been removed, but it's still being imported here. This import should be removed as part of the interface removal changes.Apply this diff to remove the unused import:
-import { IStackFeedback } from './evaluation.js'📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.packages/graphql/src/schema/evaluation.ts (1)
10-10: 💡 Codebase verification
Move evaluation interfaces to a dedicated types file
The codebase already has an established
typesdirectory atpackages/graphql/src/types/and contains non-GraphQL type definitions. The evaluation interfaces in this file are pure TypeScript interfaces that don't use GraphQL types, making them suitable for extraction.
- Move the following interfaces to a new file
packages/graphql/src/types/evaluation.ts:
IActivityEvaluationIStackFeedbackIStackEvaluationIElementInstanceEvaluationInstanceEvaluationResults- All element-specific evaluation interfaces (IChoicesElementEvaluationResults, etc.)
🔗 Analysis chain
Consider addressing the TODO comment.
The TODO comment suggests moving types to a separate file. Since we're adding new types, this might be a good time to implement that suggestion.
Let's check if there are other type definition files in the project:
Let me gather more specific information about the types in the evaluation schema and check for any established patterns in the GraphQL package:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Look for existing type definition files and potential patterns # to determine the best location for these types # Check for existing type definition files echo "Searching for type definition files..." fd -e ts -e d.ts "types?" --type f # Check for other GraphQL schema files echo -e "\nSearching for other GraphQL schema files..." fd -e ts "schema" --type fLength of output: 487
Script:
#!/bin/bash # Check the structure of the GraphQL package echo "Checking GraphQL package structure..." fd . packages/graphql/src --type d # Look for any type definitions in the GraphQL package echo -e "\nLooking for type-related files in GraphQL package..." rg -l "type\s+\w+\s*=" packages/graphql/src # Check the evaluation.ts file content to understand the types echo -e "\nChecking evaluation.ts content..." cat packages/graphql/src/schema/evaluation.tsLength of output: 12768
packages/types/src/index.ts (1)
466-468:
⚠️ Potential issueMaintain consistency: Rename 'answers' to 'responses' for free text evaluation.
For consistency with
IInstanceEvaluationNumerical, consider renaming theanswersfield toresponses. This would make the interface patterns more uniform and easier to maintain.Apply this change:
export interface IInstanceEvaluationFreeText extends IBaseInstanceEvaluation { - answers?: SingleFreeTextResponse[] + responses?: SingleFreeTextResponse[] solutions: string[] lastResponse?: SingleQuestionResponseValue | null }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.responses?: SingleFreeTextResponse[] solutions: string[] lastResponse?: SingleQuestionResponseValue | nullpackages/graphql/src/schema/question.ts (2)
9-10:
⚠️ Potential issueFix typo in type alias.
There's a typo in the alias name:
SingleNumericalRepsonseTypeis missing an 's'.- SingleNumericalResponse as SingleNumericalRepsonseType, + SingleNumericalResponse as SingleNumericalResponseType,📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.SingleFreeTextResponse as SingleFreeTextResponseType, SingleNumericalResponse as SingleNumericalResponseType,
188-195: 💡 Codebase verification
⚠️ Potential issueBased on the search results, I can generate the final response as we have found all the necessary information about the type usage:
Fix typo in SingleNumericalResponse type name and its import.
The type name has a typo in both the import and type definition:
- The import from
@klicker-uzh/typesis aliased with a typo asSingleNumericalRepsonseType- The type is correctly spelled as
SingleNumericalResponsein the builder referenceThe type is used correctly in other parts of the codebase (e.g., in
ops.ts), so this is just a local issue in the schema definition.🔗 Analysis chain
Fix typo in SingleNumericalResponse type name.
The type name contains a typo:
SingleNumericalRepsonseis missing an 's'.-export const SingleNumericalRepsonse = builder +export const SingleNumericalResponse = builderLet's verify the type usage across the codebase:
The previous search might have failed due to case sensitivity or partial matches. Let's try a broader search pattern to catch both the correct and incorrect spellings, along with examining the full context of the schema file to understand the type usage better.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for any instances of the misspelled type rg "SingleNumericalRepsonse" --type tsLength of output: 397
Script:
#!/bin/bash # Search for any instances of the type name (both correct and incorrect spellings) rg "SingleNumerical[Rr]e[sp]ponse" --type ts -A 2 # Search for type definitions and references in the schema fd "question.ts" --exec cat {}Length of output: 57555
packages/graphql/src/services/stacks.ts (3)
2228-2232:
⚠️ Potential issueRemove outdated TODO comment
The TODO comment at line 2228~ is outdated, as
computeQuestionEvaluationalready calls the appropriate evaluation functions (evaluateChoicesElementResponse,evaluateNumericalElementResponse,evaluateFreeTextElementResponse). Please remove or update the TODO comment to reflect the current implementation.
2231-2234:
⚠️ Potential issueHandle potential undefined
optionsinupdatedInstanceWhen accessing
updatedInstance.options.pointsMultiplier, ensure thatupdatedInstance.optionsis not undefined to prevent potential runtime errors.Apply this diff to safely access
pointsMultiplier:const questionEval = computeQuestionEvaluation({ elementData: existingInstance.elementData, results: updatedInstance.results, anonymousResults: updatedInstance.anonymousResults, - multiplier: updatedInstance.options.pointsMultiplier, + multiplier: updatedInstance.options?.pointsMultiplier, })Committable suggestion was skipped due to low confidence.
1170-1213: 🛠️ Refactor suggestion
Set default value for
multiplierin evaluation functionsTo ensure consistent scoring and avoid potential
undefinedvalues, consider providing a default value for themultiplierparameter inevaluateChoicesElementResponse.Apply this diff to set a default value:
function evaluateChoicesElementResponse({ elementData, results, anonymousResults, correctness, - multiplier, + multiplier = 1, }: { elementData: ChoicesElementData results: ElementResultsChoices anonymousResults: ElementResultsChoices correctness: number | null multiplier?: number }): ChoicesEvaluationReturnType | null { // ... - pointsMultiplier: multiplier ?? 1, + pointsMultiplier: multiplier,Repeat similar changes for
evaluateNumericalElementResponseandevaluateFreeTextElementResponseto maintain consistency.Committable suggestion was skipped due to low confidence.
klicker-uzh
|
||||||||||||||||||||||||||||
| Project |
klicker-uzh
|
| Branch Review |
aggregate-results-evaluation
|
| Run status |
|
| Run duration | 11m 18s |
| Commit |
|
| Committer | Julius Schlapbach |
| View all properties for this run ↗︎ | |
| Test results | |
|---|---|
|
Failures
|
0
|
|
Flaky
|
0
|
|
Pending
|
0
|
|
Skipped
|
0
|
|
Passing
|
140
|
| View all changes introduced in this branch ↗︎ | |
|
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's limits when posting large numbers of comments.
Actionable comments posted: 5
🧹 Outside diff range and nitpick comments (9)
packages/shared-components/src/evaluation/SCEvaluation.tsx (2)
18-36: Add null checks and handle edge cases.
The changes look good overall, moving to an array-based structure improves code clarity. However, consider adding some safety improvements:
Consider applying these improvements:
- {evaluation.choices?.map((choice) => (
+ {evaluation.choices && evaluation.choices.length > 0 ? (
+ evaluation.choices.map((choice) => (
<Progress
isMaxVisible={false}
className={{
root: twMerge(
'h-8',
choice.ix == correctIx && 'font-bold text-green-600'
),
indicator: twMerge(
'h-8',
choice.ix == correctIx ? 'bg-green-600' : 'bg-gray-400'
),
}}
key={`choice-statistic-${choice.ix}-${choice.count}`}
- value={(choice.count / (evaluation.numAnswers ?? 1)) * 100}
+ value={evaluation.numAnswers > 0 ? (choice.count / evaluation.numAnswers) * 100 : 0}
max={100}
formatter={(v) => v.toFixed() + '%'}
/>
- ))}
+ ))
+ ) : (
+ <div className="text-gray-500">{t('common.noResponses')}</div>
+ )}21-30: Optimize className construction.
The current implementation recalculates the className on every render. Consider memoizing these values or moving them outside the component.
+ const getClassNames = (isCorrect: boolean) => ({
+ root: twMerge('h-8', isCorrect && 'font-bold text-green-600'),
+ indicator: twMerge('h-8', isCorrect ? 'bg-green-600' : 'bg-gray-400'),
+ });
function SCEvaluation({ evaluation }: Props) {
// ... existing code ...
return (
<div className="space-y-2">
{evaluation.choices?.map((choice) => (
<Progress
isMaxVisible={false}
- className={{
- root: twMerge(
- 'h-8',
- choice.ix == correctIx && 'font-bold text-green-600'
- ),
- indicator: twMerge(
- 'h-8',
- choice.ix == correctIx ? 'bg-green-600' : 'bg-gray-400'
- ),
- }}
+ className={getClassNames(choice.ix === correctIx)}packages/graphql/src/schema/question.ts (1)
GraphQL schema and fragment updates needed
The verification has identified inconsistencies between the schema and its usage:
- In
FStackFeedbackEvaluations.graphql,FreeTextInstanceEvaluationstill uses the old field nameanswerswhile the schema defines it asanswers. NumericalInstanceEvaluationcorrectly uses theresponsesfield.ChoicesInstanceEvaluationcorrectly uses thechoicesfield.
Please update the following:
- In
FStackFeedbackEvaluations.graphql, changeanswerstoresponsesin theFreeTextInstanceEvaluationfragment to match the schema definition.
🔗 Analysis chain
Line range hint 175-246: Verify GraphQL query/fragment updates
Please ensure all GraphQL queries and fragments using these evaluation types are updated to reflect the schema changes:
- New response types
- Field name changes from
answerstoresponses
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Find all GraphQL files that might need updates
echo "Files potentially requiring updates:"
fd -e graphql -e gql
# Check for potentially outdated field usage
echo -e "\nPotential instances of old field names:"
rg "answers.*FreeTextInstanceEvaluation" --type graphql
Length of output: 11982
Script:
#!/bin/bash
# Search for GraphQL fragments and queries that use these evaluation types
echo "Checking for usage of evaluation types in GraphQL files:"
# Check for FreeTextInstanceEvaluation field usage
echo -e "\nFreeTextInstanceEvaluation field usage:"
rg -A 3 "FreeTextInstanceEvaluation.*{" packages/graphql/src/graphql/ops/
# Check for NumericalInstanceEvaluation field usage
echo -e "\nNumericalInstanceEvaluation field usage:"
rg -A 3 "NumericalInstanceEvaluation.*{" packages/graphql/src/graphql/ops/
# Check for ChoicesInstanceEvaluation field usage
echo -e "\nChoicesInstanceEvaluation field usage:"
rg -A 3 "ChoicesInstanceEvaluation.*{" packages/graphql/src/graphql/ops/
# Check for any references to 'answers' field
echo -e "\nChecking for 'answers' field usage:"
rg "answers.*{" packages/graphql/src/graphql/ops/
Length of output: 1230
Script:
#!/bin/bash
# Search for GraphQL fragments and queries that use these evaluation types
echo "Checking for usage of evaluation types in GraphQL files:"
# Check for FreeTextInstanceEvaluation field usage
echo -e "\nFreeTextInstanceEvaluation field usage:"
rg "FreeTextInstanceEvaluation \{" packages/graphql/src/graphql/ops/
# Check for field names in fragments
echo -e "\nChecking field usage in fragments:"
rg "answers: \[" packages/graphql/src/graphql/ops/
rg "responses: \[" packages/graphql/src/graphql/ops/
rg "choices: \[" packages/graphql/src/graphql/ops/
# Check fragment definitions
echo -e "\nChecking fragment definitions:"
rg "fragment.*InstanceEvaluation" packages/graphql/src/graphql/ops/
Length of output: 997
Script:
#!/bin/bash
# Let's check the content of the file that uses FreeTextInstanceEvaluation
echo "Content of FStackFeedbackEvaluations.graphql:"
cat packages/graphql/src/graphql/ops/FStackFeedbackEvaluations.graphql
# Also check for any inline fragments on these evaluation types
echo -e "\nChecking for inline fragments on evaluation types:"
rg -A 5 "on (FreeTextInstanceEvaluation|NumericalInstanceEvaluation|ChoicesInstanceEvaluation)" packages/graphql/src/graphql/ops/
Length of output: 4482
packages/graphql/src/public/schema.graphql (2)
1515-1518: LGTM! Well-structured response types.
The introduction of explicit response types (SingleChoiceResponse, SingleFreeTextResponse, SingleNumericalResponse) improves type safety and schema clarity. Each type appropriately captures its specific requirements:
- Choice responses include count and index
- Free text responses include count and string value
- Numerical responses include count and float value
This standardization of response types:
- Makes the schema more maintainable
- Provides better type safety compared to using JSON
- Makes it easier to validate responses on the client side
- Improves documentation through self-describing types
Also applies to: 1520-1523, 1525-1528
150-150: LGTM! Consistent field updates across evaluation types.
The changes to evaluation types show consistent handling of responses:
ChoicesInstanceEvaluation.choices: [SingleChoiceResponse!]FreeTextInstanceEvaluation.answers: [SingleFreeTextResponse!]NumericalInstanceEvaluation.responses: [SingleNumericalResponse!]
The consistent naming and structure across different question types will make it easier to:
- Implement generic handling of responses in the frontend
- Add new question types in the future
- Maintain type safety throughout the application
Also applies to: 665-665, 1132-1132
packages/graphql/src/services/stacks.ts (2)
Line range hint 2667-2749: Consider optimizing numerical results combination
The numerical results combination could be optimized by using a Map to store values instead of array.find operations, which would improve performance for large datasets.
Consider this optimization:
function combineNumericalResults({
results,
anonymousResults,
}: {
results: ElementResultsOpen
anonymousResults: ElementResultsOpen
}) {
- return [
- ...Object.values(results.responses),
- ...Object.values(anonymousResults.responses),
- ].reduce<{ value: number; count: number; correct?: boolean | null }[]>(
- (acc, response) => {
- const responseValue = parseFloat(response.value)
- const ix = acc.findIndex(
- (r) => Math.abs(r.value - responseValue) < Number.EPSILON
- )
- if (ix === -1) {
- acc.push({
- value: responseValue,
- count: response.count,
- correct: response.correct,
- })
- } else {
- acc[ix] = {
- ...acc[ix]!,
- count: acc[ix]!.count + response.count,
- }
- }
- return acc
- },
- []
- )
+ const combinedMap = new Map<number, { count: number; correct?: boolean | null }>()
+
+ const processResponses = (responses: ElementResultsOpen['responses']) => {
+ Object.values(responses).forEach((response) => {
+ const value = parseFloat(response.value)
+ const existing = combinedMap.get(value)
+ if (existing) {
+ existing.count += response.count
+ } else {
+ combinedMap.set(value, {
+ count: response.count,
+ correct: response.correct
+ })
+ }
+ })
+ }
+
+ processResponses(results.responses)
+ processResponses(anonymousResults.responses)
+
+ return Array.from(combinedMap.entries()).map(([value, data]) => ({
+ value,
+ count: data.count,
+ correct: data.correct
+ }))
}2802-2802: TODO: Implement missing statistics functionality
The comment indicates that statistical analysis features need to be added to numerical responses.
Would you like me to help implement basic statistical measures (mean, median, mode, standard deviation) for numerical responses? I can create a new GitHub issue to track this enhancement.
packages/graphql/src/ops.schema.json (2)
22524-22655: Well-structured response types, consider adding descriptions
The new response types are well-designed with appropriate field types and a consistent pattern. However, they lack descriptions that would help developers understand their purpose and usage.
Consider adding descriptions to the types and fields. For example:
{
"kind": "OBJECT",
"name": "SingleNumericalResponse",
- "description": null,
+ "description": "Represents an aggregated numerical response with its occurrence count",
"isOneOf": null,
"fields": [
{
"name": "count",
- "description": null,
+ "description": "Number of times this response value was submitted",Line range hint 1663-22655: Strong type modeling for response handling
The restructuring of response types provides several architectural benefits:
- Better type safety through specific response types
- Consistent pattern across different response kinds
- Support for response aggregation through the count field
- Clear separation of concerns between different response types
This will make the schema more maintainable and easier to extend in the future.
🛑 Comments failed to post (5)
packages/graphql/src/schema/question.ts (3)
9-11:
⚠️ Potential issueFix typo in type name
There's a typo in the imported type name:
SingleNumericalRepsonseTypeshould beSingleNumericalResponseType.- SingleNumericalResponse as SingleNumericalRepsonseType, + SingleNumericalResponse as SingleNumericalResponseType,📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.SingleChoiceResponse as SingleChoiceResponseType, SingleFreeTextResponse as SingleFreeTextResponseType, SingleNumericalResponse as SingleNumericalResponseType,
229-246:
⚠️ Potential issueFix inconsistent field naming in FreeTextInstanceEvaluation
The implementation of
SingleFreeTextResponseis good, but there's an inconsistency in the field naming:
NumericalInstanceEvaluationusesresponsesFreeTextInstanceEvaluationusesanswersFor consistency across evaluation types, we should use
responses.- answers: t.expose('answers', { + responses: t.expose('responses', { type: [SingleFreeTextResponse], nullable: true, }),📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.export const SingleFreeTextResponse = builder .objectRef<SingleFreeTextResponseType>('SingleFreeTextResponse') .implement({ fields: (t) => ({ value: t.exposeString('value'), count: t.exposeInt('count'), }), }) export const FreeTextInstanceEvaluation = builder .objectRef<IInstanceEvaluationFreeText>('FreeTextInstanceEvaluation') .implement({ fields: (t) => ({ ...sharedEvaluationProps(t), responses: t.expose('responses', { type: [SingleFreeTextResponse], nullable: true, }),
200-217: 💡 Codebase verification
⚠️ Potential issueFix typo in type name and update all references
The verification shows that the type with the typo
SingleNumericalRepsonseis used in multiple places:
- Type import alias
- Object reference definition
- Field type reference
The type is used in GraphQL operations through the
NumericalInstanceEvaluationresponses field, which is correctly spelled. The fix needs to be applied consistently across all occurrences.
packages/graphql/src/schema/question.ts:- SingleNumericalResponse as SingleNumericalRepsonseType, + SingleNumericalResponse as SingleNumericalResponseType, - export const SingleNumericalRepsonse = builder + export const SingleNumericalResponse = builder - .objectRef<SingleNumericalRepsonseType>('SingleNumericalResponse') + .objectRef<SingleNumericalResponseType>('SingleNumericalResponse') - type: [SingleNumericalRepsonse], + type: [SingleNumericalResponse],🔗 Analysis chain
Fix typo in type name and verify response type usage
- The type name has a typo:
SingleNumericalRepsonseshould beSingleNumericalResponse- The implementation looks good, but ensure all queries and mutations using this type are updated accordingly.
-export const SingleNumericalRepsonse = builder +export const SingleNumericalResponse = builder - .objectRef<SingleNumericalRepsonseType>('SingleNumericalResponse') + .objectRef<SingleNumericalResponseType>('SingleNumericalResponse')🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify the usage of NumericalInstanceEvaluation responses field rg -l "NumericalInstanceEvaluation.*responses" --type graphqlLength of output: 11982
Script:
#!/bin/bash # Search for SingleNumericalRepsonse (with typo) usage rg "SingleNumericalRepsonse" --type ts # Search for NumericalInstanceEvaluation responses field usage rg "NumericalInstanceEvaluation.*responses" --type ts -A 2 # Search for any GraphQL fragments or operations using NumericalInstanceEvaluation fd -e graphql -X cat {} | rg "NumericalInstanceEvaluation"Length of output: 59143
packages/types/src/index.ts (2)
465-470: 🛠️ Refactor suggestion
Rename
answerstoresponsesfor uniformityThe
IInstanceEvaluationFreeTextinterface uses theanswersproperty, whereas similar data structures useresponsesin other interfaces. To maintain uniformity, consider renaminganswerstoresponses.Apply this diff to rename
answerstoresponses:export type SingleFreeTextResponse = { count: number; value: string } export interface IInstanceEvaluationFreeText extends IBaseInstanceEvaluation { - answers?: SingleFreeTextResponse[] + responses?: SingleFreeTextResponse[] solutions: string[] lastResponse?: SingleQuestionResponseValue | null }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.export type SingleFreeTextResponse = { count: number; value: string } export interface IInstanceEvaluationFreeText extends IBaseInstanceEvaluation { responses?: SingleFreeTextResponse[] solutions: string[] lastResponse?: SingleQuestionResponseValue | null }
450-454: 🛠️ Refactor suggestion
Consider renaming
choicestoresponsesfor consistencyIn the
IInstanceEvaluationChoicesinterface, the property is namedchoices, whereas inIInstanceEvaluationNumericalandIInstanceEvaluationFreeText, the properties areresponses. For consistency across evaluation interfaces, consider renamingchoicestoresponses.Apply this diff to rename
choicestoresponses:export type SingleChoiceResponse = { ix: number; count: number } export interface IInstanceEvaluationChoices extends IBaseInstanceEvaluation { - choices: SingleChoiceResponse[] + responses: SingleChoiceResponse[] lastResponse?: SingleQuestionResponseChoices | null }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.export type SingleChoiceResponse = { ix: number; count: number } export interface IInstanceEvaluationChoices extends IBaseInstanceEvaluation { responses: SingleChoiceResponse[] lastResponse?: SingleQuestionResponseChoices | null }
Summary by CodeRabbit
Release Notes
New Features
SingleChoiceResponse,SingleNumericalResponse, andSingleFreeTextResponse.Improvements
Bug Fixes