Skip to content

Conversation

@joaovitoralvarenga
Copy link
Collaborator

Sorry for the long wait to open this pull request, this last week was kinda busy and I wanted to make sure the choices that I made in the implementation were the right ones, since this issue covers a lot of methods of the grading process. First of all, as I have written in the comment section, for the main purpose of storing the subjects and or categories as individual tests I developed a new class inside ../core/models called result_tree. The class contains the representation of a tree structure that stores each part of the grading process:

class NodeType(str, Enum):
    """Represents the types of nodes in the result tree"""
    ROOT = "root"
    CATEGORY = "category" #base, bonus, penalty 
    SUBJECT = "subject"

class ResultNode(BaseModel):
    """This class represents a node in the result tree"""

    model_config = ConfigDict(arbitrary_types_allowed=True)
    
    node_type : NodeType
    name: str
    unweighted_score: Optional[float] = None
    weighted_score: Optional[float] = None
    weight: float = 0.0
    max_score: Optional[float] = None
    # Hierarchical structure 
    children: List['ResultNode'] = Field(default_factory=list)
    test_results: List[TestResult] = Field(default_factory=list)
    total_tests: int = 0

After that, to make sure all the other parts of the system properly accessed the new method of storing results I made some changes and implementations in other files, for example the result.py, that now contains more methods that serve the purpose of processing the tree structure and navigate through it:

 def get_subject_tests(self, subject_name: str) ->  List[TestResult]:
        node = self.get_subject_node(subject_name)
        return node.get_all_tests_results() if node else []

 def get_navigation_path(self, node_name: str) -> Optional[List[ResultNode]]: 
        # Main method for navigation on the tree
        if not self.result_tree:
            return None
        return self.result_tree.get_node_path(node_name)

I made, as well, changes in the grader.py to implement the tree building and in the autograder_response.py to adapt the way the main method returned the response, now including the result_tree. Now in the grading and simulation process, the output includes the building of the result_tree and returns a tree structure. However, I did not change the playroom.py to print the returned tree, because I wasn't sure if that was needed, but if you need me to do it, and show the structure in the simulations logs, please tell me, as well as any other change that you think it's needed!

Copilot AI review requested due to automatic review settings November 25, 2025 02:40
@joaovitoralvarenga joaovitoralvarenga requested a review from a team as a code owner November 25, 2025 02:40
@joaovitoralvarenga joaovitoralvarenga linked an issue Nov 25, 2025 that may be closed by this pull request
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This pull request implements a hierarchical tree structure for storing and organizing grading results. The new ResultNode class provides a tree-based representation of the grading process, organizing tests into subjects and categories (base, bonus, penalty), enabling better navigation and reporting of test results.

Key Changes

  • Introduced ResultNode and NodeType classes in a new result_tree.py module to represent hierarchical grading structure
  • Extended Result class with tree navigation methods (get_subject_node, get_category_tests, print_tree, etc.)
  • Modified Grader._run() to build and return the result tree alongside the final score
  • Updated AutograderResponse to include the result_tree field in all responses

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 17 comments.

Show a summary per file
File Description
autograder/core/models/result_tree.py New file introducing ResultNode and NodeType classes for hierarchical result representation
autograder/core/models/result.py Added tree navigation and query methods, tree printing utilities, and result_tree field
autograder/core/grading/grader.py Modified grading workflow to build result tree after scoring, added _build_result_node method
autograder/core/models/autograder_response.py Added result_tree field to response model with appropriate configuration
autograder/autograder_facade.py Updated response creation to include result_tree in all code paths
tests/playroom.py Minor whitespace cleanup

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

return None
return self.result_tree.find_node(subject_name, NodeType.SUBJECT)

def get_subject_tests(self, subject_name: str) -> List[TestResult]:
Copy link

Copilot AI Nov 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Extra space before arrow in return type annotation. Should be -> List[TestResult]: instead of -> List[TestResult]: to follow consistent formatting.

Suggested change
def get_subject_tests(self, subject_name: str) -> List[TestResult]:
def get_subject_tests(self, subject_name: str) -> List[TestResult]:

Copilot uses AI. Check for mistakes.
result_node.test_results = subject_tests
result_node.total_tests = len(subject_tests)

# Calculate score(tests averages)
Copy link

Copilot AI Nov 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing space after opening parenthesis in comment. Should be # Calculate score (tests averages) instead of # Calculate score(tests averages).

Suggested change
# Calculate score(tests averages)
# Calculate score (tests averages)

Copilot uses AI. Check for mistakes.
return self.base_results + self.bonus_results + self.penalty_results


def get_category_node(self,category: str) -> Optional[ResultNode]:
Copy link

Copilot AI Nov 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing space after comma in function parameter list. Should be get_category_node(self, category: str) instead of get_category_node(self,category: str) to follow PEP 8 style guidelines.

Suggested change
def get_category_node(self,category: str) -> Optional[ResultNode]:
def get_category_node(self, category: str) -> Optional[ResultNode]:

Copilot uses AI. Check for mistakes.
author: str
submission_files: Dict[str,str] = Field(default_factory=dict, alias="submission_files")

""" The hierarchical result tree structure """
Copy link

Copilot AI Nov 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Improper placement of docstring comment. The comment """ The hierarchical result tree structure """ should either be a regular inline comment using # or be moved to be part of the class docstring. Multi-line string literals in this position are not standard Python docstrings.

Suggested change
""" The hierarchical result tree structure """
# The hierarchical result tree structure

Copilot uses AI. Check for mistakes.
feedback = feedback_report,
test_report = result.get_test_report()
test_report = result.get_test_report(),
result_tree= result.result_tree
Copy link

Copilot AI Nov 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing space after equals sign in parameter assignment. Should be result_tree=result.result_tree instead of result_tree= result.result_tree to follow PEP 8 style guidelines.

Copilot uses AI. Check for mistakes.
feedback="",
test_report=result.get_test_report()
test_report=result.get_test_report(),
result_tree= result.result_tree
Copy link

Copilot AI Nov 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing space after equals sign in parameter assignment. Should be result_tree=result.result_tree instead of result_tree= result.result_tree to follow PEP 8 style guidelines.

Suggested change
result_tree= result.result_tree
result_tree=result.result_tree

Copilot uses AI. Check for mistakes.
depth + 1
)

if child_result_node: # Only add a children if it has tests
Copy link

Copilot AI Nov 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Grammatical error in comment: "children" should be singular "child" to match the context. Should be # Only add a child if it has tests instead of # Only add a children if it has tests.

Suggested change
if child_result_node: # Only add a children if it has tests
if child_result_node: # Only add a child if it has tests

Copilot uses AI. Check for mistakes.
from autograder.core.models.test_result import TestResult

from pydantic import BaseModel, Field
from pydantic import BaseModel, Field, ConfigDict
Copy link

Copilot AI Nov 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Import of 'ConfigDict' is not used.

Suggested change
from pydantic import BaseModel, Field, ConfigDict
from pydantic import BaseModel, Field

Copilot uses AI. Check for mistakes.
@@ -0,0 +1,97 @@
from typing import List, Dict, Optional
Copy link

Copilot AI Nov 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Import of 'Dict' is not used.

Suggested change
from typing import List, Dict, Optional
from typing import List, Optional

Copilot uses AI. Check for mistakes.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Copy link
Member

@ArthurCRodrigues ArthurCRodrigues left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks reeeally good. Can you just update playroom.py and test if it works correctly?
Here are some key parts to look at:

  • How does the tree structure get returned via API?
  • Does the Reporter access the tree structure? (I guess not)
  • Is it possible to build the result tree at the same process of grading the criteria tree or these two have two be different processes? (Making the first one possible would save us a lot of resources)

The most important ones are the first and last. If everything is right, then we'll get to the best part of this entire feature: improving the feedback system.

I'll tag you up for a quick call today, are you free at 9?

@joaovitoralvarenga
Copy link
Collaborator Author

joaovitoralvarenga commented Nov 26, 2025

Yo! Just had time to push my changes right now!. So, about the way the result_tree is returned in the output, just wanted to show how it is being shown and how I could apply to all the others tests that will present the structure:

--- BUILDING RESULT TREE ---
        Built leaf node: Bootstrap Integration with 1 tests (avg: 100.00)
        Built leaf node: Bootstrap Grid Classes with 1 tests (avg: 100.00)
    Built branch node: HTML Structure with 2 children, 2 total tests
        Built leaf node: Card Components with 1 tests (avg: 100.00)
        Built leaf node: Custom Styling with 1 tests (avg: 100.00)
    Built branch node: Components with 2 children, 2 total tests
Built branch node: base with 2 children, 4 total tests
    Built leaf node: Best Practices with 1 tests (avg: 100.00)
Built branch node: bonus with 1 children, 1 total tests
✓ Added base_node: base (4 tests)
✓ Added bonus_node: bonus (1 tests)

This being the result of the finalized result tree:

======================================================================
          HIERARCHICAL RESULT TREE
======================================================================

🌳 Assignment Result: 100.00
  📁 base: 100.00
    📘 HTML Structure (w=50.0): 100.00
      📘 Bootstrap Integration (w=40.0): 100.00
      📘 Bootstrap Grid Classes (w=60.0): 100.00
    📘 Components (w=50.0): 100.00
      📘 Card Components (w=50.0): 100.00
      📘 Custom Styling (w=50.0): 100.00
  📁 bonus: 100.00
    📘 Best Practices (w=100.0): 100.00

And about the other questions:

  • I wasn't able to test the return of the API, since it reports a permission error, however, I imagine is going to be returned the same way as the examples above!
  • No, the Reporter can not access the structure, but I can change that!
  • And yes, I could build the result_tree while the criteria grading process is being executed, I would just have to refactor the grader.py as well!

Please, let me know about any other suggestions or improvements that could be made!

@ArthurCRodrigues
Copy link
Member

I'm really impressed with your implementation, we'll be able to come up with some really good features with this result tree now.
Could you try to refactor grader.py to build the ResultTree at the same time that it traverses the CriteriaTree to grade the submission?
I'll text you on whatsapp to check the permission error issue.

@joaovitoralvarenga
Copy link
Collaborator Author

Yo! I'm just starting putting my hands on the tasks this week! However, I just wanted to know how I should approach the grader.py refactor to change the way the Result Tree is being built, making sure the process of generating it occurs at the same time as the criteria_tree building. Should I already refactor the grader in this PR, or we should merge these changes and link a sub issue to change the grading system?

@ArthurCRodrigues
Copy link
Member

ArthurCRodrigues commented Dec 1, 2025

@joaovitoralvarenga it's easier to apply these changes in this branch.

Notice that you'll have to refactor grader.py, not criteria_tree.py. What i mean is, you'll only change the process of grading, not the criteria_tree building as you mentioned.
The Grader should build the ResultTree at the same time it traverses the CriteriaTree.

obs: I'll be kinda distant this week for college motives

@joaovitoralvarenga
Copy link
Collaborator Author

Indeed! I meant to say that I would refactor the grader.py to make sure the result_tree is being built at the same time the grading process of the criteria tree ocurs.

@ArthurCRodrigues
Copy link
Member

Okay! If you need anything, just tag me up.

@ArthurCRodrigues ArthurCRodrigues merged commit 894d9e6 into main Jan 27, 2026
1 check failed
@ArthurCRodrigues ArthurCRodrigues deleted the 95-return-tree-like-report-structure branch January 27, 2026 10:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Return Tree-Like Report Structure

3 participants