Skip to content

Improve promptTypes Documentation#29

Merged
rappm merged 9 commits into
mainfrom
improve-prompt-types-documentation
Nov 3, 2025
Merged

Improve promptTypes Documentation#29
rappm merged 9 commits into
mainfrom
improve-prompt-types-documentation

Conversation

@rappm

@rappm rappm commented Jul 21, 2025

Copy link
Copy Markdown
Member

Improve the prompt types documentation.

First draft, open for suggestions. Closes #30, Closes #27

Summary by CodeRabbit

  • New Features

    • Added a gender option: "Prefer Not To Say" for improved inclusivity.
  • Refactor

    • Student now reuses a Person type for identity fields.
  • Chores

    • Relaxed strict UUID binding on several ID fields to reduce validation failures.
  • Documentation

    • Expanded inline comments and docs across many types (answers, participation, metadata, degrees, phase copy, student, team, resolution).

@coderabbitai

coderabbitai Bot commented Jul 21, 2025

Copy link
Copy Markdown

Walkthrough

Adds extensive inline documentation across many promptTypes files, introduces a new gender constant, refactors Student to embed Person, and removes UUID binding validation from several ID fields. No core algorithmic logic was added; most edits are comments and small struct/tag adjustments.

Changes

Cohort / File(s) Change Summary
Application answers
promptTypes/applicationAnswer.go
Added detailed comments to constants, structs, and the JSON-polymorphic reader; no logic changes.
Course phase participation
promptTypes/coursePhaseParticipation.go
Added type-level and field-level comments to CoursePhaseParticipationWithStudent.
Gender
promptTypes/gender.go
Added comments for Gender type and constants; introduced GenderPreferNotToSay.
MetaData
promptTypes/metaData.go
Added descriptive comment explaining MetaData purpose and value shapes.
Person
promptTypes/person.go
Added Person type file with documentation (used by Student).
Phase copy API docs
promptTypes/phaseCopy.go
Expanded documentation for PhaseCopyRequest, PhaseCopyHandler, and RegisterCopyEndpoint; no signature changes.
Student
promptTypes/student.go
Refactored Student to embed Person; removed explicit uuid import; added detailed field comments.
Study degree
promptTypes/studyDegree.go
Added comments to StudyDegree type and constants.
Team
promptTypes/team.go
Added Team documentation; removed binding:"uuid" tag from ID field.
Resolution
resolution.go
Changed binding tag on Resolution.CoursePhaseID from binding:"required,uuid" to binding:"required" (removed UUID format validation).

Sequence Diagram(s)

(The changes are documentation-focused and do not introduce new control-flow or runtime interactions; no sequence diagram provided.)

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Potential points needing attention:

  • Verify serialization/deserialization behavior after Student now embeds Person.
  • Confirm removal of binding:"uuid" on Person.ID, Team.ID, and change on Resolution.CoursePhaseID is intentional (validation impact).
  • Ensure consumers expecting the removed validation or tag-based binding are updated.
  • Check usage of new GenderPreferNotToSay across codebase for handling/compatibility.

Poem

I nibble on comments, tidy and bright,
Docstrings in burrows, line by line light.
Person hops in — Student snuggles near,
Gender choices whispered, now clear.
Code blossoms neatly — a rabbit’s delight. 🐇

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Out of Scope Changes check ⚠️ Warning While the primary objective is documentation improvement, the PR includes several out-of-scope functional changes beyond documentation: removal of UUID validation bindings from Person.ID, Team.ID, and Resolution.CoursePhaseID fields; introduction of a new GenderPreferNotToSay constant; and structural refactoring of Student to embed Person instead of declaring fields directly. These changes alter input validation semantics and data structures beyond the stated documentation objective. To resolve this, separate the functional changes from the documentation improvements into distinct pull requests. Create one PR focusing exclusively on documentation additions (comments, type descriptions, field explanations) and another PR for structural changes (removing validation bindings, adding new constants, refactoring structs). This approach ensures each change can be reviewed, tested, and justified independently according to its specific requirements.
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The pull request title 'Improve promptTypes Documentation' accurately reflects the main objective of the changeset. The summary confirms this PR is specifically focused on improving documentation for promptTypes by adding detailed comments and explanatory text across multiple files in the promptTypes package without altering functional logic. The title is concise, clear, and directly related to the primary changes.
Linked Issues check ✅ Passed The pull request successfully addresses the linked issue #30 'Add documentation to promptTypes' by adding comprehensive documentation comments across all promptTypes files. The changes include type-level comments, field-level comments for structs, and descriptive comments for constants and functions throughout the package, directly fulfilling the requirement to document promptTypes.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch improve-prompt-types-documentation

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

…riptions for clarity in ApplicationAnswer, CoursePhaseParticipation, Gender, MetaData, Person, PhaseCopy, Student, StudyDegree, and Team types.
@rappm rappm requested a review from mathildeshagl July 21, 2025 07:04
@rappm rappm assigned rappm and unassigned rappm Jul 21, 2025
@rappm rappm requested a review from Mtze July 21, 2025 07:04
@rappm rappm added the schau mi o Translation: ready for review label Jul 21, 2025

@mathildeshagl mathildeshagl left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

@mathildeshagl mathildeshagl left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

just a few small inline comments

Comment thread promptTypes/coursePhaseParticipation.go Outdated
Comment thread promptTypes/phaseCopy.go
Comment thread promptTypes/student.go Outdated

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 3

🧹 Nitpick comments (3)
promptTypes/metaData.go (1)

3-12: Prefer any over interface{} for modern Go style

Since Go 1.18 the predeclared identifier any is the canonical alias for interface{}. Migrating to map[string]any keeps the semantics identical while matching current language idioms.

-type MetaData map[string]interface{}
+type MetaData map[string]any
promptTypes/student.go (1)

30-33: Consider typed constants for gender

Hard-coding the allowed literals twice (code + tag) risks drift. Defining const ( GenderMale Gender = "male" … ) and re-using those values in both the tag and business logic improves maintainability.

promptTypes/coursePhaseParticipation.go (1)

12-15: Enforce PassStatus values

Since the doc lists the allowed statuses, add binding:"oneof=passed failed not_assessed" or make PassStatus a dedicated enum-type string to prevent invalid inputs slipping through.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 066db14 and 18c3603.

📒 Files selected for processing (9)
  • promptTypes/applicationAnswer.go (1 hunks)
  • promptTypes/coursePhaseParticipation.go (1 hunks)
  • promptTypes/gender.go (1 hunks)
  • promptTypes/metaData.go (1 hunks)
  • promptTypes/person.go (1 hunks)
  • promptTypes/phaseCopy.go (1 hunks)
  • promptTypes/student.go (1 hunks)
  • promptTypes/studyDegree.go (1 hunks)
  • promptTypes/team.go (1 hunks)
🧰 Additional context used
🧠 Learnings (10)
📓 Common learnings
Learnt from: mathildeshagl
PR: ls1intum/Prompt-SDK#24
File: promptTypes/copyRequest.go:5-8
Timestamp: 2025-07-06T12:18:51.959Z
Learning: The Prompt-SDK project does not follow the Go convention of adding GoDoc comments to exported types. Most exported types in the codebase (Student, MetaData, Gender, StudyDegree, Resolution, etc.) lack GoDoc comments, with only TokenUser being documented. The project maintains consistency by not requiring GoDoc comments for exported types.
Learnt from: mathildeshagl
PR: ls1intum/Prompt-SDK#22
File: resolution.go:26-29
Timestamp: 2025-06-21T11:14:56.130Z
Learning: In resolution.go, the struct rename from CoursePhaseParticipationWithResolution to CoursePhaseParticipationWithResolutions and field change from Resolution to Resolutions []Resolution was a consistency fix, not a breaking change. Everything using this endpoint already had the correct structure with multiple resolutions, but this particular struct definition was outdated and didn't match existing usage patterns.
promptTypes/studyDegree.go (2)

Learnt from: mathildeshagl
PR: #24
File: promptTypes/copyRequest.go:5-8
Timestamp: 2025-07-06T12:18:51.959Z
Learning: The Prompt-SDK project does not follow the Go convention of adding GoDoc comments to exported types. Most exported types in the codebase (Student, MetaData, Gender, StudyDegree, Resolution, etc.) lack GoDoc comments, with only TokenUser being documented. The project maintains consistency by not requiring GoDoc comments for exported types.

Learnt from: mathildeshagl
PR: #22
File: resolution.go:26-29
Timestamp: 2025-06-21T11:14:56.130Z
Learning: In resolution.go, the struct rename from CoursePhaseParticipationWithResolution to CoursePhaseParticipationWithResolutions and field change from Resolution to Resolutions []Resolution was a consistency fix, not a breaking change. Everything using this endpoint already had the correct structure with multiple resolutions, but this particular struct definition was outdated and didn't match existing usage patterns.

promptTypes/metaData.go (1)

Learnt from: mathildeshagl
PR: #24
File: promptTypes/copyRequest.go:5-8
Timestamp: 2025-07-06T12:18:51.959Z
Learning: The Prompt-SDK project does not follow the Go convention of adding GoDoc comments to exported types. Most exported types in the codebase (Student, MetaData, Gender, StudyDegree, Resolution, etc.) lack GoDoc comments, with only TokenUser being documented. The project maintains consistency by not requiring GoDoc comments for exported types.

promptTypes/applicationAnswer.go (2)

Learnt from: mathildeshagl
PR: #24
File: promptTypes/copyRequest.go:5-8
Timestamp: 2025-07-06T12:18:51.959Z
Learning: The Prompt-SDK project does not follow the Go convention of adding GoDoc comments to exported types. Most exported types in the codebase (Student, MetaData, Gender, StudyDegree, Resolution, etc.) lack GoDoc comments, with only TokenUser being documented. The project maintains consistency by not requiring GoDoc comments for exported types.

Learnt from: mathildeshagl
PR: #22
File: resolution.go:26-29
Timestamp: 2025-06-21T11:14:56.130Z
Learning: In resolution.go, the struct rename from CoursePhaseParticipationWithResolution to CoursePhaseParticipationWithResolutions and field change from Resolution to Resolutions []Resolution was a consistency fix, not a breaking change. Everything using this endpoint already had the correct structure with multiple resolutions, but this particular struct definition was outdated and didn't match existing usage patterns.

promptTypes/gender.go (1)

Learnt from: mathildeshagl
PR: #24
File: promptTypes/copyRequest.go:5-8
Timestamp: 2025-07-06T12:18:51.959Z
Learning: The Prompt-SDK project does not follow the Go convention of adding GoDoc comments to exported types. Most exported types in the codebase (Student, MetaData, Gender, StudyDegree, Resolution, etc.) lack GoDoc comments, with only TokenUser being documented. The project maintains consistency by not requiring GoDoc comments for exported types.

promptTypes/student.go (2)

Learnt from: mathildeshagl
PR: #24
File: promptTypes/copyRequest.go:5-8
Timestamp: 2025-07-06T12:18:51.959Z
Learning: The Prompt-SDK project does not follow the Go convention of adding GoDoc comments to exported types. Most exported types in the codebase (Student, MetaData, Gender, StudyDegree, Resolution, etc.) lack GoDoc comments, with only TokenUser being documented. The project maintains consistency by not requiring GoDoc comments for exported types.

Learnt from: mathildeshagl
PR: #22
File: resolution.go:26-29
Timestamp: 2025-06-21T11:14:56.130Z
Learning: In resolution.go, the struct rename from CoursePhaseParticipationWithResolution to CoursePhaseParticipationWithResolutions and field change from Resolution to Resolutions []Resolution was a consistency fix, not a breaking change. Everything using this endpoint already had the correct structure with multiple resolutions, but this particular struct definition was outdated and didn't match existing usage patterns.

promptTypes/coursePhaseParticipation.go (2)

Learnt from: mathildeshagl
PR: #22
File: resolution.go:26-29
Timestamp: 2025-06-21T11:14:56.130Z
Learning: In resolution.go, the struct rename from CoursePhaseParticipationWithResolution to CoursePhaseParticipationWithResolutions and field change from Resolution to Resolutions []Resolution was a consistency fix, not a breaking change. Everything using this endpoint already had the correct structure with multiple resolutions, but this particular struct definition was outdated and didn't match existing usage patterns.

Learnt from: mathildeshagl
PR: #24
File: promptTypes/copyRequest.go:5-8
Timestamp: 2025-07-06T12:18:51.959Z
Learning: The Prompt-SDK project does not follow the Go convention of adding GoDoc comments to exported types. Most exported types in the codebase (Student, MetaData, Gender, StudyDegree, Resolution, etc.) lack GoDoc comments, with only TokenUser being documented. The project maintains consistency by not requiring GoDoc comments for exported types.

promptTypes/team.go (2)

Learnt from: mathildeshagl
PR: #24
File: promptTypes/copyRequest.go:5-8
Timestamp: 2025-07-06T12:18:51.959Z
Learning: The Prompt-SDK project does not follow the Go convention of adding GoDoc comments to exported types. Most exported types in the codebase (Student, MetaData, Gender, StudyDegree, Resolution, etc.) lack GoDoc comments, with only TokenUser being documented. The project maintains consistency by not requiring GoDoc comments for exported types.

Learnt from: mathildeshagl
PR: #22
File: resolution.go:26-29
Timestamp: 2025-06-21T11:14:56.130Z
Learning: In resolution.go, the struct rename from CoursePhaseParticipationWithResolution to CoursePhaseParticipationWithResolutions and field change from Resolution to Resolutions []Resolution was a consistency fix, not a breaking change. Everything using this endpoint already had the correct structure with multiple resolutions, but this particular struct definition was outdated and didn't match existing usage patterns.

promptTypes/person.go (1)

Learnt from: mathildeshagl
PR: #24
File: promptTypes/copyRequest.go:5-8
Timestamp: 2025-07-06T12:18:51.959Z
Learning: The Prompt-SDK project does not follow the Go convention of adding GoDoc comments to exported types. Most exported types in the codebase (Student, MetaData, Gender, StudyDegree, Resolution, etc.) lack GoDoc comments, with only TokenUser being documented. The project maintains consistency by not requiring GoDoc comments for exported types.

promptTypes/phaseCopy.go (2)

Learnt from: mathildeshagl
PR: #22
File: resolution.go:26-29
Timestamp: 2025-06-21T11:14:56.130Z
Learning: In resolution.go, the struct rename from CoursePhaseParticipationWithResolution to CoursePhaseParticipationWithResolutions and field change from Resolution to Resolutions []Resolution was a consistency fix, not a breaking change. Everything using this endpoint already had the correct structure with multiple resolutions, but this particular struct definition was outdated and didn't match existing usage patterns.

Learnt from: mathildeshagl
PR: #24
File: promptTypes/copyRequest.go:5-8
Timestamp: 2025-07-06T12:18:51.959Z
Learning: The Prompt-SDK project does not follow the Go convention of adding GoDoc comments to exported types. Most exported types in the codebase (Student, MetaData, Gender, StudyDegree, Resolution, etc.) lack GoDoc comments, with only TokenUser being documented. The project maintains consistency by not requiring GoDoc comments for exported types.

🧬 Code Graph Analysis (3)
promptTypes/student.go (3)
promptTypes/person.go (1)
  • Person (8-18)
promptTypes/gender.go (1)
  • Gender (6-6)
promptTypes/studyDegree.go (1)
  • StudyDegree (6-6)
promptTypes/coursePhaseParticipation.go (2)
promptTypes/metaData.go (1)
  • MetaData (13-13)
promptTypes/student.go (1)
  • Student (10-49)
promptTypes/team.go (1)
promptTypes/person.go (1)
  • Person (8-18)
🔇 Additional comments (6)
promptTypes/studyDegree.go (1)

3-16: Documentation addition is consistent and helpful – no issues found
The new comments follow GoDoc conventions and improve discoverability without altering behaviour.

promptTypes/gender.go (1)

3-22: LGTM – new constant and explanatory comments improve clarity
No functional changes, the additional GenderPreferNotToSay option strengthens inclusivity.

promptTypes/student.go (2)

11-13: Embedding looks good

Switching to an embedded Person cleanly flattens the identity fields in JSON and avoids duplicate tags. Nice improvement.


18-21: Confirm custom validation tag is registered

binding:"matriculationNumber" is not one of the built-in Gin/validator tags. Make sure a custom rule with this name is registered, otherwise the field will silently skip validation.

promptTypes/coursePhaseParticipation.go (1)

30-31: Payload size warning

Embedding the full Student object may be overkill if the endpoint only needs a subset (e.g., ID + name). Consider a lean view model to keep responses small and avoid leaking data inadvertently.

promptTypes/applicationAnswer.go (1)

8-14: Docs-only change – LGTM

The new comments greatly clarify answer types without touching logic.

Comment thread promptTypes/person.go
Comment thread promptTypes/phaseCopy.go
Comment thread promptTypes/team.go Outdated
@Mtze

Mtze commented Jul 22, 2025

Copy link
Copy Markdown
Member

We have some conflicts after the last merge

@Mtze Mtze left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Please fix/close the Rabbit comments

@github-project-automation github-project-automation Bot moved this to In Progress in PROMPT Jul 22, 2025
@Mtze Mtze moved this from In Progress to In Review in PROMPT Oct 31, 2025
@rappm rappm requested a review from mathildeshagl November 3, 2025 12:00
@rappm rappm requested a review from Mtze November 3, 2025 12:07
@rappm

rappm commented Nov 3, 2025

Copy link
Copy Markdown
Member Author

Done - pls review agin

@rappm rappm self-assigned this Nov 3, 2025

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (1)
resolution.go (1)

18-18: UUID binding removed—required validation retained. Consider standardizing binding patterns across similar uuid.UUID fields.

This change correctly removes the non-functional uuid binding constraint while preserving binding:"required". The uuid validator only works on string types, not uuid.UUID types.

However, promptTypes/person.go line 11 uses a different approach—no binding tag at all on its ID field. If both fields serve similar request validation purposes, standardizing the binding approach across the codebase would improve consistency. If person.go is intentionally a domain model without Gin validation, the difference is justified.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 18c3603 and b8f1dd6.

📒 Files selected for processing (3)
  • promptTypes/person.go (1 hunks)
  • promptTypes/team.go (1 hunks)
  • resolution.go (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • promptTypes/team.go
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: mathildeshagl
Repo: ls1intum/Prompt-SDK PR: 24
File: promptTypes/copyRequest.go:5-8
Timestamp: 2025-07-06T12:18:51.959Z
Learning: The Prompt-SDK project does not follow the Go convention of adding GoDoc comments to exported types. Most exported types in the codebase (Student, MetaData, Gender, StudyDegree, Resolution, etc.) lack GoDoc comments, with only TokenUser being documented. The project maintains consistency by not requiring GoDoc comments for exported types.
📚 Learning: 2025-06-21T11:14:56.130Z
Learnt from: mathildeshagl
Repo: ls1intum/Prompt-SDK PR: 22
File: resolution.go:26-29
Timestamp: 2025-06-21T11:14:56.130Z
Learning: In resolution.go, the struct rename from CoursePhaseParticipationWithResolution to CoursePhaseParticipationWithResolutions and field change from Resolution to Resolutions []Resolution was a consistency fix, not a breaking change. Everything using this endpoint already had the correct structure with multiple resolutions, but this particular struct definition was outdated and didn't match existing usage patterns.

Applied to files:

  • resolution.go
📚 Learning: 2025-07-06T12:18:51.959Z
Learnt from: mathildeshagl
Repo: ls1intum/Prompt-SDK PR: 24
File: promptTypes/copyRequest.go:5-8
Timestamp: 2025-07-06T12:18:51.959Z
Learning: The Prompt-SDK project does not follow the Go convention of adding GoDoc comments to exported types. Most exported types in the codebase (Student, MetaData, Gender, StudyDegree, Resolution, etc.) lack GoDoc comments, with only TokenUser being documented. The project maintains consistency by not requiring GoDoc comments for exported types.

Applied to files:

  • promptTypes/person.go
🔇 Additional comments (2)
promptTypes/person.go (2)

5-17: Comprehensive documentation added to Person type.

The documentation is clear, well-structured, and follows Go conventions. This represents a deliberate shift from the project's previous practice of not documenting exported types.

Based on learnings: The project historically did not add GoDoc comments to exported types, but this PR explicitly aims to improve documentation across promptTypes, marking a positive change in practice.


11-11: UUID binding tag correctly removed.

This change properly addresses the past review comment by removing the non-functional binding:"uuid" tag. Since Person is a data model rather than a request body, removing all binding validation is appropriate—the uuid.UUID type itself provides type safety.

This differs from resolution.go line 18, which retains binding:"required" because Resolution is used in request validation contexts.

@Mtze

Mtze commented Nov 3, 2025

Copy link
Copy Markdown
Member

Looking good now :)

@mathildeshagl

Copy link
Copy Markdown
Member

👍🏻

@rappm rappm merged commit 790b3eb into main Nov 3, 2025
4 checks passed
@rappm rappm deleted the improve-prompt-types-documentation branch November 3, 2025 14:37
@github-project-automation github-project-automation Bot moved this from In Review to Done in PROMPT Nov 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

schau mi o Translation: ready for review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add documentation to promptTypes Add documentation and readme for PROMPT SDK

3 participants