🔨 Added JsDoc, WIP metadata reconciliation
This commit is contained in:
379
.claude/skills/jsdoc/SKILL.md
Normal file
379
.claude/skills/jsdoc/SKILL.md
Normal file
@@ -0,0 +1,379 @@
|
||||
---
|
||||
name: jsdoc
|
||||
description: Commenting and documentation guidelines. Auto-activate when the user discusses comments, documentation, docstrings, code clarity, API docs, JSDoc, or asks about commenting strategies.
|
||||
---
|
||||
|
||||
Auto-activate when: User discusses comments, documentation, docstrings, code clarity, code quality, API docs, JSDoc, Python docstrings, or asks about commenting strategies.
|
||||
Core Principle
|
||||
|
||||
Write code that speaks for itself. Comment only when necessary to explain WHY, not WHAT.
|
||||
|
||||
Most code does not need comments. Well-written code with clear naming and structure is self-documenting.
|
||||
|
||||
The best comment is the one you don't need to write because the code is already obvious.
|
||||
The Commenting Philosophy
|
||||
When to Comment
|
||||
|
||||
✅ DO comment when explaining:
|
||||
|
||||
WHY something is done (business logic, design decisions)
|
||||
Complex algorithms and their reasoning
|
||||
Non-obvious trade-offs or constraints
|
||||
Workarounds for bugs or limitations
|
||||
API contracts and public interfaces
|
||||
Regex patterns and what they match
|
||||
Performance considerations or optimizations
|
||||
Constants and magic numbers
|
||||
Gotchas or surprising behaviors
|
||||
|
||||
❌ DON'T comment when:
|
||||
|
||||
The code is obvious and self-explanatory
|
||||
The comment repeats the code (redundant)
|
||||
Better naming would eliminate the need
|
||||
The comment would become outdated quickly
|
||||
It's decorative or organizational noise
|
||||
It states what a standard language construct does
|
||||
|
||||
Comment Anti-Patterns
|
||||
❌ 1. Obvious Comments
|
||||
|
||||
BAD:
|
||||
|
||||
counter = 0 # Initialize counter to zero
|
||||
counter += 1 # Increment counter by one
|
||||
user_name = input("Enter name: ") # Get user name from input
|
||||
|
||||
Better: No comment needed - the code is self-explanatory.
|
||||
❌ 2. Redundant Comments
|
||||
|
||||
BAD:
|
||||
|
||||
def get_user_name(user):
|
||||
return user.name # Return the user's name
|
||||
|
||||
def calculate_total(items):
|
||||
# Loop through items and sum the prices
|
||||
total = 0
|
||||
for item in items:
|
||||
total += item.price
|
||||
return total
|
||||
|
||||
Better:
|
||||
|
||||
def get_user_name(user):
|
||||
return user.name
|
||||
|
||||
def calculate_total(items):
|
||||
return sum(item.price for item in items)
|
||||
|
||||
❌ 3. Outdated Comments
|
||||
|
||||
BAD:
|
||||
|
||||
# Calculate tax at 5% rate
|
||||
tax = price * 0.08 # Actually 8%, comment is wrong
|
||||
|
||||
# DEPRECATED: Use new_api_function() instead
|
||||
def old_function(): # Still being used, comment is misleading
|
||||
pass
|
||||
|
||||
Better: Keep comments in sync with code, or remove them entirely.
|
||||
❌ 4. Noise Comments
|
||||
|
||||
BAD:
|
||||
|
||||
# Start of function
|
||||
def calculate():
|
||||
# Declare variable
|
||||
result = 0
|
||||
# Return result
|
||||
return result
|
||||
# End of function
|
||||
|
||||
Better: Remove all of these comments.
|
||||
❌ 5. Dead Code & Changelog Comments
|
||||
|
||||
BAD:
|
||||
|
||||
# Don't comment out code - use version control
|
||||
# def old_function():
|
||||
# return "deprecated"
|
||||
|
||||
# Don't maintain history in comments
|
||||
# Modified by John on 2023-01-15
|
||||
# Fixed bug reported by Sarah on 2023-02-03
|
||||
|
||||
Better: Delete the code. Git has the history.
|
||||
Good Comment Examples
|
||||
✅ Complex Business Logic
|
||||
|
||||
# Apply progressive tax brackets: 10% up to $10k, 20% above
|
||||
# This matches IRS publication 501 for 2024
|
||||
def calculate_progressive_tax(income):
|
||||
if income <= 10000:
|
||||
return income * 0.10
|
||||
else:
|
||||
return 1000 + (income - 10000) * 0.20
|
||||
|
||||
✅ Non-obvious Algorithms
|
||||
|
||||
# Using Floyd-Warshall for all-pairs shortest paths
|
||||
# because we need distances between all nodes.
|
||||
# Time: O(n³), Space: O(n²)
|
||||
for k in range(vertices):
|
||||
for i in range(vertices):
|
||||
for j in range(vertices):
|
||||
dist[i][j] = min(dist[i][j], dist[i][k] + dist[k][j])
|
||||
|
||||
✅ Regex Patterns
|
||||
|
||||
# Match email format: username@domain.extension
|
||||
# Allows letters, numbers, dots, hyphens in username
|
||||
# Requires valid domain and 2+ char extension
|
||||
email_pattern = r'^[a-zA-Z0-9._%+-]+@[a-zA-Z0-9.-]+\.[a-zA-Z]{2,}$'
|
||||
|
||||
✅ API Constraints or Gotchas
|
||||
|
||||
# GitHub API rate limit: 5000 requests/hour for authenticated users
|
||||
# We implement exponential backoff to handle rate limiting
|
||||
await rate_limiter.wait()
|
||||
response = await fetch(github_api_url)
|
||||
|
||||
✅ Workarounds for Bugs
|
||||
|
||||
# HACK: Workaround for bug in library v2.1.0
|
||||
# Remove after upgrading to v2.2.0
|
||||
# See: https://github.com/library/issues/123
|
||||
if library_version == "2.1.0":
|
||||
apply_workaround()
|
||||
|
||||
Decision Framework
|
||||
|
||||
Before writing a comment, ask yourself:
|
||||
Step 1: Is the code self-explanatory?
|
||||
|
||||
If YES → No comment needed
|
||||
If NO → Continue to step 2
|
||||
|
||||
Step 2: Would a better variable/function name eliminate the need?
|
||||
|
||||
If YES → Refactor the code instead
|
||||
If NO → Continue to step 3
|
||||
|
||||
Step 3: Does this explain WHY, not WHAT?
|
||||
|
||||
If explaining WHAT → Refactor code to be clearer
|
||||
If explaining WHY → Good comment candidate
|
||||
|
||||
Step 4: Will this help future maintainers?
|
||||
|
||||
If YES → Write the comment
|
||||
If NO → Skip it
|
||||
|
||||
Special Cases for Comments
|
||||
Public APIs and Docstrings
|
||||
Python Docstrings
|
||||
|
||||
def calculate_compound_interest(
|
||||
principal: float,
|
||||
rate: float,
|
||||
time: int,
|
||||
compound_frequency: int = 1
|
||||
) -> float:
|
||||
"""
|
||||
Calculate compound interest using the standard formula.
|
||||
|
||||
Args:
|
||||
principal: Initial amount invested
|
||||
rate: Annual interest rate as decimal (e.g., 0.05 for 5%)
|
||||
time: Time period in years
|
||||
compound_frequency: Times per year interest compounds (default: 1)
|
||||
|
||||
Returns:
|
||||
Final amount after compound interest
|
||||
|
||||
Raises:
|
||||
ValueError: If any parameter is negative
|
||||
|
||||
Example:
|
||||
>>> calculate_compound_interest(1000, 0.05, 10)
|
||||
1628.89
|
||||
"""
|
||||
if principal < 0 or rate < 0 or time < 0:
|
||||
raise ValueError("Parameters must be non-negative")
|
||||
|
||||
# Compound interest formula: A = P(1 + r/n)^(nt)
|
||||
return principal * (1 + rate / compound_frequency) ** (compound_frequency * time)
|
||||
|
||||
JavaScript/TypeScript JSDoc
|
||||
|
||||
/**
|
||||
* Fetch user data from the API.
|
||||
*
|
||||
* @param {string} userId - The unique user identifier
|
||||
* @param {Object} options - Configuration options
|
||||
* @param {boolean} options.includeProfile - Include profile data (default: true)
|
||||
* @param {number} options.timeout - Request timeout in ms (default: 5000)
|
||||
*
|
||||
* @returns {Promise<User>} User object with requested fields
|
||||
*
|
||||
* @throws {Error} If userId is invalid or request fails
|
||||
*
|
||||
* @example
|
||||
* const user = await fetchUser('123', { includeProfile: true });
|
||||
*/
|
||||
async function fetchUser(userId, options = {}) {
|
||||
// Implementation
|
||||
}
|
||||
|
||||
Constants and Configuration
|
||||
|
||||
# Based on network reliability studies (95th percentile)
|
||||
MAX_RETRIES = 3
|
||||
|
||||
# AWS Lambda timeout is 15s, leaving 5s buffer for cleanup
|
||||
API_TIMEOUT = 10000 # milliseconds
|
||||
|
||||
# Cache duration optimized for balance between freshness and load
|
||||
# See: docs/performance-tuning.md
|
||||
CACHE_TTL = 300 # 5 minutes
|
||||
|
||||
Annotations for TODOs and Warnings
|
||||
|
||||
# TODO: Replace with proper authentication after security review
|
||||
# Issue: #456
|
||||
def temporary_auth(user):
|
||||
return True
|
||||
|
||||
# WARNING: This function modifies the original array instead of creating a copy
|
||||
def sort_in_place(arr):
|
||||
arr.sort()
|
||||
return arr
|
||||
|
||||
# FIXME: Memory leak in production - investigate connection pooling
|
||||
# Ticket: JIRA-789
|
||||
def get_connection():
|
||||
return create_connection()
|
||||
|
||||
# PERF: Consider caching this result if called frequently in hot path
|
||||
def expensive_calculation(data):
|
||||
return complex_algorithm(data)
|
||||
|
||||
# SECURITY: Validate input to prevent SQL injection before using in query
|
||||
def build_query(user_input):
|
||||
sanitized = escape_sql(user_input)
|
||||
return f"SELECT * FROM users WHERE name = '{sanitized}'"
|
||||
|
||||
Common Annotation Keywords
|
||||
|
||||
TODO: - Work that needs to be done
|
||||
FIXME: - Known bugs that need fixing
|
||||
HACK: - Temporary workarounds
|
||||
NOTE: - Important information or context
|
||||
WARNING: - Critical information about usage
|
||||
PERF: - Performance considerations
|
||||
SECURITY: - Security-related notes
|
||||
BUG: - Known bug documentation
|
||||
REFACTOR: - Code that needs refactoring
|
||||
DEPRECATED: - Soon-to-be-removed code
|
||||
|
||||
Refactoring Over Commenting
|
||||
Instead of Commenting Complex Code...
|
||||
|
||||
BAD: Complex code with comment
|
||||
|
||||
# Check if user is admin or has special permissions
|
||||
if user.role == "admin" or (user.permissions and "special" in user.permissions):
|
||||
grant_access()
|
||||
|
||||
...Extract to Named Function
|
||||
|
||||
GOOD: Self-explanatory through naming
|
||||
|
||||
def user_has_admin_access(user):
|
||||
return user.role == "admin" or has_special_permission(user)
|
||||
|
||||
def has_special_permission(user):
|
||||
return user.permissions and "special" in user.permissions
|
||||
|
||||
if user_has_admin_access(user):
|
||||
grant_access()
|
||||
|
||||
Language-Specific Examples
|
||||
JavaScript
|
||||
|
||||
// Good: Explains WHY we debounce
|
||||
// Debounce search to reduce API calls (500ms wait after last keystroke)
|
||||
const debouncedSearch = debounce(searchAPI, 500);
|
||||
|
||||
// Bad: Obvious
|
||||
let count = 0; // Initialize count to zero
|
||||
count++; // Increment count
|
||||
|
||||
// Good: Explains algorithm choice
|
||||
// Using Set for O(1) lookup instead of Array.includes() which is O(n)
|
||||
const seen = new Set(ids);
|
||||
|
||||
Python
|
||||
|
||||
# Good: Explains the algorithm choice
|
||||
# Using binary search because data is sorted and we need O(log n) performance
|
||||
index = bisect.bisect_left(sorted_list, target)
|
||||
|
||||
# Bad: Redundant
|
||||
def get_total(items):
|
||||
return sum(items) # Return the sum of items
|
||||
|
||||
# Good: Explains why we're doing this
|
||||
# Extract to separate function for type checking in mypy
|
||||
def validate_user(user):
|
||||
if not user or not user.id:
|
||||
raise ValueError("Invalid user")
|
||||
return user
|
||||
|
||||
TypeScript
|
||||
|
||||
// Good: Explains the type assertion
|
||||
// TypeScript can't infer this is never null after the check
|
||||
const element = document.getElementById('app') as HTMLElement;
|
||||
|
||||
// Bad: Obvious
|
||||
const sum = a + b; // Add a and b
|
||||
|
||||
// Good: Explains non-obvious behavior
|
||||
// spread operator creates shallow copy; use JSON for deep copy
|
||||
const newConfig = { ...config };
|
||||
|
||||
Comment Quality Checklist
|
||||
|
||||
Before committing, ensure your comments:
|
||||
|
||||
Explain WHY, not WHAT
|
||||
Are grammatically correct and clear
|
||||
Will remain accurate as code evolves
|
||||
Add genuine value to code understanding
|
||||
Are placed appropriately (above the code they describe)
|
||||
Use proper spelling and professional language
|
||||
Follow team conventions for annotation keywords
|
||||
Could not be replaced by better naming or structure
|
||||
Are not obvious statements about language features
|
||||
Reference tickets/issues when applicable
|
||||
|
||||
Summary
|
||||
|
||||
Priority order:
|
||||
|
||||
Clear code - Self-explanatory through naming and structure
|
||||
Good comments - Explain WHY when necessary
|
||||
Documentation - API docs, docstrings for public interfaces
|
||||
No comments - Better than bad comments that lie or clutter
|
||||
|
||||
Remember: Comments are a failure to make the code self-explanatory. Use them sparingly and wisely.
|
||||
Key Takeaways
|
||||
Goal Approach
|
||||
Reduce comments Improve naming, extract functions, simplify logic
|
||||
Improve clarity Use self-explanatory code structure, clear variable names
|
||||
Document APIs Use docstrings/JSDoc for public interfaces
|
||||
Explain WHY Comment only business logic, algorithms, workarounds
|
||||
Maintain accuracy Update comments when code changes, or remove them
|
||||
@@ -10,7 +10,7 @@ import "react-sliding-pane/dist/react-sliding-pane.css";
|
||||
import SlidingPane from "react-sliding-pane";
|
||||
import { determineCoverFile } from "../../shared/utils/metadata.utils";
|
||||
import { styled } from "styled-components";
|
||||
import { RawFileDetails as RawFileDetailsType } from "../../graphql/generated";
|
||||
import type { RawFileDetails as RawFileDetailsType, InferredMetadata } from "../../graphql/generated";
|
||||
|
||||
// Extracted modules
|
||||
import { useComicVineMatching } from "./useComicVineMatching";
|
||||
@@ -23,52 +23,47 @@ const StyledSlidingPanel = styled(SlidingPane)`
|
||||
background: #ccc;
|
||||
`;
|
||||
|
||||
type InferredIssue = {
|
||||
interface ComicVineMetadata {
|
||||
name?: string;
|
||||
number?: number;
|
||||
year?: string;
|
||||
subtitle?: string;
|
||||
[key: string]: any;
|
||||
};
|
||||
volumeInformation?: Record<string, unknown>;
|
||||
[key: string]: unknown;
|
||||
}
|
||||
|
||||
type ComicVineMetadata = {
|
||||
name?: string;
|
||||
volumeInformation?: any;
|
||||
[key: string]: any;
|
||||
};
|
||||
|
||||
type Acquisition = {
|
||||
interface Acquisition {
|
||||
directconnect?: {
|
||||
downloads?: any[];
|
||||
downloads?: unknown[];
|
||||
};
|
||||
torrent?: any[];
|
||||
[key: string]: any;
|
||||
};
|
||||
torrent?: unknown[];
|
||||
[key: string]: unknown;
|
||||
}
|
||||
|
||||
type ComicDetailProps = {
|
||||
interface ComicDetailProps {
|
||||
data: {
|
||||
_id: string;
|
||||
rawFileDetails?: RawFileDetailsType;
|
||||
inferredMetadata: {
|
||||
issue?: InferredIssue;
|
||||
};
|
||||
inferredMetadata: InferredMetadata;
|
||||
sourcedMetadata: {
|
||||
comicvine?: ComicVineMetadata;
|
||||
locg?: any;
|
||||
comicInfo?: any;
|
||||
locg?: Record<string, unknown>;
|
||||
comicInfo?: Record<string, unknown>;
|
||||
};
|
||||
acquisition?: Acquisition;
|
||||
createdAt: string;
|
||||
updatedAt: string;
|
||||
};
|
||||
userSettings?: any;
|
||||
queryClient?: any;
|
||||
userSettings?: Record<string, unknown>;
|
||||
queryClient?: unknown;
|
||||
comicObjectId?: string;
|
||||
};
|
||||
}
|
||||
|
||||
/**
|
||||
* Displays full comic detail: cover, file info, action menu, and tabbed panels
|
||||
* for metadata, archive operations, and acquisition.
|
||||
*
|
||||
* @param data.queryClient - react-query client passed through to the CV match
|
||||
* panel so it can invalidate queries after a match is applied.
|
||||
* @param data.comicObjectId - optional override for the comic ID; used when the
|
||||
* component is rendered outside a route that provides the ID via `useParams`.
|
||||
*/
|
||||
export const ComicDetail = (data: ComicDetailProps): ReactElement => {
|
||||
const {
|
||||
@@ -104,7 +99,8 @@ export const ComicDetail = (data: ComicDetailProps): ReactElement => {
|
||||
setVisible(true);
|
||||
}, []);
|
||||
|
||||
// Action menu handler
|
||||
// Hide "match on Comic Vine" when there are no raw file details — matching
|
||||
// requires file metadata to seed the search query.
|
||||
const Placeholder = components.Placeholder;
|
||||
const filteredActionOptions = filter(actionOptions, (item) => {
|
||||
if (isUndefined(rawFileDetails)) {
|
||||
@@ -180,6 +176,7 @@ export const ComicDetail = (data: ComicDetailProps): ReactElement => {
|
||||
rawFileDetails={rawFileDetails}
|
||||
inferredMetadata={inferredMetadata}
|
||||
comicVineMatches={comicVineMatches}
|
||||
// Prefer the route param; fall back to the data ID when rendered outside a route.
|
||||
comicObjectId={comicObjectId || _id}
|
||||
queryClient={queryClient}
|
||||
onMatchApplied={() => {
|
||||
|
||||
@@ -7,7 +7,7 @@ import TextareaAutosize from "react-textarea-autosize";
|
||||
|
||||
interface EditMetadataPanelProps {
|
||||
data: {
|
||||
name?: string;
|
||||
name?: string | null;
|
||||
[key: string]: any;
|
||||
};
|
||||
}
|
||||
|
||||
@@ -2,27 +2,27 @@ import React from "react";
|
||||
import { ComicVineSearchForm } from "./ComicVineSearchForm";
|
||||
import { ComicVineMatchPanel } from "./ComicVineMatchPanel";
|
||||
import { EditMetadataPanel } from "./EditMetadataPanel";
|
||||
import { RawFileDetails } from "../../graphql/generated";
|
||||
import type { RawFileDetails, InferredMetadata } from "../../graphql/generated";
|
||||
|
||||
type InferredIssue = {
|
||||
name?: string;
|
||||
number?: number;
|
||||
year?: string;
|
||||
subtitle?: string;
|
||||
[key: string]: any;
|
||||
};
|
||||
|
||||
type CVMatchesPanelProps = {
|
||||
interface CVMatchesPanelProps {
|
||||
rawFileDetails?: RawFileDetails;
|
||||
inferredMetadata: {
|
||||
issue?: InferredIssue;
|
||||
};
|
||||
inferredMetadata: InferredMetadata;
|
||||
comicVineMatches: any[];
|
||||
comicObjectId: string;
|
||||
queryClient: any;
|
||||
onMatchApplied: () => void;
|
||||
};
|
||||
|
||||
/**
|
||||
* Sliding panel content for ComicVine match search.
|
||||
*
|
||||
* Renders a search form pre-populated from `rawFileDetails`, a preview of the
|
||||
* inferred issue being searched for, and a list of ComicVine match candidates
|
||||
* the user can apply to the comic.
|
||||
*
|
||||
* @param props.onMatchApplied - Called after the user selects and applies a match,
|
||||
* allowing the parent to close the panel and refresh state.
|
||||
*/
|
||||
export const CVMatchesPanel: React.FC<CVMatchesPanelProps> = ({
|
||||
rawFileDetails,
|
||||
inferredMetadata,
|
||||
@@ -62,4 +62,4 @@ type EditMetadataPanelWrapperProps = {
|
||||
|
||||
export const EditMetadataPanelWrapper: React.FC<EditMetadataPanelWrapperProps> = ({
|
||||
rawFileDetails,
|
||||
}) => <EditMetadataPanel data={rawFileDetails} />;
|
||||
}) => <EditMetadataPanel data={rawFileDetails ?? {}} />;
|
||||
|
||||
@@ -30,6 +30,7 @@ interface VolumeInformationProps {
|
||||
onReconcile?: () => void;
|
||||
}
|
||||
|
||||
/** Sources stored under `sourcedMetadata` — excludes `inferredMetadata`, which is checked separately. */
|
||||
const SOURCED_METADATA_KEYS = ["comicvine", "locg", "comicInfo", "metron", "gcd"];
|
||||
|
||||
const SOURCE_LABELS: Record<string, string> = {
|
||||
@@ -72,6 +73,17 @@ const MetadataSourceChips = ({
|
||||
</div>
|
||||
);
|
||||
|
||||
/**
|
||||
* Displays volume metadata for a comic.
|
||||
*
|
||||
* - When multiple sources are present, renders a chip bar listing each source
|
||||
* with a "Reconcile sources" action to merge them.
|
||||
* - When exactly one source is present and it is ComicVine, renders the full
|
||||
* ComicVine detail panel directly.
|
||||
*
|
||||
* @param props.data - Comic data containing sourced and inferred metadata.
|
||||
* @param props.onReconcile - Called when the user triggers source reconciliation.
|
||||
*/
|
||||
export const VolumeInformation = (props: VolumeInformationProps): ReactElement => {
|
||||
const { data, onReconcile } = props;
|
||||
|
||||
@@ -83,7 +95,7 @@ export const VolumeInformation = (props: VolumeInformationProps): ReactElement =
|
||||
if (key === "locg") return Object.values(val as Record<string, unknown>).some((v) => !isNil(v) && v !== "");
|
||||
return true;
|
||||
});
|
||||
if (!isNil(data?.inferredMetadata?.issue) && !isEmpty(data.inferredMetadata.issue)) {
|
||||
if (!isNil(data?.inferredMetadata?.issue) && !isEmpty(data?.inferredMetadata?.issue)) {
|
||||
sources.push("inferredMetadata");
|
||||
}
|
||||
return sources;
|
||||
|
||||
Reference in New Issue
Block a user