fix 5.4: return structured types from get_issue/get_pull_request/create_pull_request/update_pull_request

- IssueTools.get_issue now returns IssueModel instead of JSON string
- PRTools.get_pull_request now returns PullRequestModel instead of JSON string
- PRTools.create_pull_request now returns PullRequestModel instead of JSON string
- PRTools.update_pull_request now returns PullRequestModel instead of JSON string
- All methods have proper return type hints and raise exceptions on error
- Updated tests to verify model objects are returned directly
- Marked issue 5.4 as resolved in bad_code.md

tests/test_issue_tools.py

@@ -20,11 +20,11 @@ def test_get_issue_success() -> None:
     mock_client.issues.get_issue.return_value = issue
 
     issue_tools: IssueTools = IssueTools(mock_client)
-    res: str = issue_tools.get_issue("owner", "repo", 1)
+    res: IssueModel = issue_tools.get_issue("owner", "repo", 1)
 
-    data: dict[str, Any] = json.loads(res)
-    assert data["number"] == 1
-    assert data["title"] == "Test Issue"
+    assert isinstance(res, IssueModel)
+    assert res.number == 1
+    assert res.title == "Test Issue"
     mock_client.issues.get_issue.assert_called_once_with("owner", "repo", 1)
 
 
@@ -33,8 +33,11 @@ def test_get_issue_failure() -> None:
     mock_client.issues.get_issue.side_effect = Exception("API Error")
 
     issue_tools: IssueTools = IssueTools(mock_client)
-    res: str = issue_tools.get_issue("owner", "repo", 1)
-    assert "Error getting issue: API Error" in res
+    try:
+        issue_tools.get_issue("owner", "repo", 1)
+        assert False, "Expected Exception"
+    except Exception as e:
+        assert str(e) == "API Error"
 
 
 def test_close_issue_success() -> None:

tests/test_pr_tools.py

@@ -20,11 +20,11 @@ def test_get_pull_request_success() -> None:
     mock_client.prs.get_pull_request.return_value = pr
 
     pr_tools: PRTools = PRTools(mock_client)
-    res: str = pr_tools.get_pull_request("owner", "repo", 1)
+    res: PullRequestModel = pr_tools.get_pull_request("owner", "repo", 1)
 
-    data: dict[str, Any] = json.loads(res)
-    assert data["number"] == 1
-    assert data["title"] == "Test PR"
+    assert isinstance(res, PullRequestModel)
+    assert res.number == 1
+    assert res.title == "Test PR"
     mock_client.prs.get_pull_request.assert_called_once_with("owner", "repo", 1)
 
 
@@ -33,8 +33,11 @@ def test_get_pull_request_failure() -> None:
     mock_client.prs.get_pull_request.side_effect = Exception("API Error")
 
     pr_tools: PRTools = PRTools(mock_client)
-    res: str = pr_tools.get_pull_request("owner", "repo", 1)
-    assert "Error getting pull request: API Error" in res
+    try:
+        pr_tools.get_pull_request("owner", "repo", 1)
+        assert False, "Expected Exception"
+    except Exception as e:
+        assert str(e) == "API Error"
 
 
 def test_close_pull_request_success() -> None:
@@ -139,11 +142,11 @@ def test_create_pull_request_success() -> None:
     mock_client.prs.create_pr_via_tea.return_value = pr
 
     pr_tools: PRTools = PRTools(mock_client)
-    res: str = pr_tools.create_pull_request(
+    res: PullRequestModel = pr_tools.create_pull_request(
         "owner", "repo", "head", "base", "Title", "Desc"
     )
-    data: dict[str, Any] = json.loads(res)
-    assert data["number"] == 2
+    assert isinstance(res, PullRequestModel)
+    assert res.number == 2
     mock_client.prs.create_pr_via_tea.assert_called_once_with(
         "owner", "repo", "Title", "Desc", "head", "base"
     )
@@ -154,8 +157,11 @@ def test_create_pull_request_failure() -> None:
     mock_client.prs.create_pr_via_tea.side_effect = Exception("API Error")
 
     pr_tools: PRTools = PRTools(mock_client)
-    res: str = pr_tools.create_pull_request("owner", "repo", "head", "base", "Title")
-    assert "Error creating PR: API Error" in res
+    try:
+        pr_tools.create_pull_request("owner", "repo", "head", "base", "Title")
+        assert False, "Expected Exception"
+    except Exception as e:
+        assert str(e) == "API Error"
 
 
 def test_add_label_to_pr_success() -> None: