AgentSkillsCN

code-review-component

深度审视组件的技术债务、现代化进程及规范合规性。

SKILL.md
--- frontmatter
name: code-review-component
description: Deep review of a component for technical debt, modernization, and convention compliance.
license: Complete terms in LICENSE.txt

When to use this skill

When you need to perform a comprehensive audit of one or more components. This skill is for deep dives to identify technical debt, modernization opportunities, documentation gaps, and convention violations across an entire component.

Use this skill:

  • Before major refactoring work.
  • When onboarding to understand component quality.
  • Periodically to assess technical debt.
  • When preparing a component for production hardening.

How to use this skill

  1. Identify the component(s) to review.
  2. Follow the detailed instructions to audit each category.
  3. Generate findings categorised by severity.
  4. Add findings to the sprint backlog for remediation.

Detailed instructions

Step 1: Identify scope

Determine which component(s) to review. Components are located under projects/ores.COMPONENT.

For each component, identify all parts and facets:

PartContents
include/ores.COMPONENT/Header files organised by facet
src/Implementation files organised by facet
tests/Catch2 unit tests
modeling/PlantUML diagrams

Reference: component-creator skill for expected structure.

Step 2: Component structure audit

Verify the component follows the expected structure.

Folder Structure

CheckDescription
Parts presentComponent has include, src, tests, modeling folders.
Include nestinginclude has single folder named ores.COMPONENT.
Facet consistencySame facets exist in both include and src.

CMake Configuration

CheckDescription
Component CMakeLists.txtPresent at projects/ores.COMPONENT/CMakeLists.txt.
Src CMakeLists.txtPresent at projects/ores.COMPONENT/src/CMakeLists.txt.
Tests CMakeLists.txtPresent at projects/ores.COMPONENT/tests/CMakeLists.txt.
Modeling CMakeLists.txtPresent at projects/ores.COMPONENT/modeling/CMakeLists.txt.
Dependencies correctAll required dependencies are linked.

Documentation

CheckDescription
Namespace docsEach facet folder has namespace documentation for Doxygen.
READMEComponent has README or equivalent documentation.

Step 3: Headers & licensing audit

Review all .hpp and .cpp files in the component.

CheckDescription
Copyright headerEvery file has the standard GPL copyright header.
Mode lineFiles have Emacs mode line: /* -*- mode: c++; ... */
Include guardsHeaders use #ifndef ORES_COMPONENT_FACET_FILE_HPP pattern.

Step 4: Code style audit

Naming Conventions

CheckDescription
snake_case (non-Qt)All classes, methods, variables use snake_case.
CamelCase (Qt)ores.qt uses CamelCase per Qt conventions.
File namingFiles match class names exactly.
Namespace namingNamespaces follow ores::component::facet pattern.

Include Order

CheckDescription
Standard library first<algorithm>, <string>, etc.
Third-party second<boost/...>, <rfl/...>, etc.
Project headers last"ores.component/..."

Comment Quality

CheckDescription
No stale commentsRemove historical, deprecated, or outdated TODO comments.
Useful comments onlyComments explain "why", not "what".
No commented-out codeRemove dead code, use version control instead.
Doxygen completenessAll public classes and methods have @brief documentation.

Step 5: Platform isolation audit

CheckDescription
Platform macros_WIN32, __linux__, __APPLE__ only in ores.platform.
Platform headers<windows.h>, <unistd.h> only in ores.platform.
Abstraction usageOther components use ores.platform abstractions.

Step 6: Code locality audit

CheckDescription
Utility placementGeneric helpers belong in ores.utility or ores.platform.
Duplicate detectionSearch for similar code patterns across components.
Function extractionIdentify repetitive code that should be factored out.
Single responsibilityClasses and functions do one thing well.

Step 7: C++23 modernization audit

Identify opportunities to modernize code to C++23 idioms.

PatternModern Alternative
Raw pointers for ownershipstd::unique_ptr, std::shared_ptr
Exceptions for expected errorsstd::expected
Manual loopsRanges and views
typename template constraintsConcepts
Stream concatenationstd::format
NULLnullptr
typedefusing
C-style castsstatic_cast, dynamic_cast
Manual RAIIStandard RAII wrappers

Step 8: Domain types audit

For each domain type in the component, verify completeness per domain-type-creator skill.

Core Definition

CheckDescription
Struct with public fieldsDomain type is a struct final with public fields.
Doxygen documentationStruct and all fields have @brief comments.
Appropriate typesUUIDs, timestamps, strings use correct C++ types.

Services

CheckDescription
JSON I/O_json_io.hpp/.cpp exists and uses reflect-cpp.
Table I/O_table_io.hpp/.cpp exists and uses libfort.
Generator_generator.hpp/.cpp exists in generators facet.

Temporality Patterns

CheckDescription
Audit fieldsDomain type has recorded_by and recorded_at fields.
Entity mappingDatabase entity has corresponding temporal columns.
Mapper correctnessMapper converts temporal fields correctly.
Repository queriesRepository has read_at_timepoint() for historical queries.
Latest queriesRepository has read_latest() for current state.

Repository

CheckDescription
Entity defined_entity.hpp with sqlgen annotations.
Mapper defined_mapper.hpp/.cpp with to_domain() and from_domain().
Repository defined_repository.hpp/.cpp with CRUD operations.
SQL methodRepository has sql() method for table creation.

Step 9: Protocol & messaging audit

For components with messaging, verify per binary-protocol-developer skill.

Message Types

CheckDescription
Enum registrationAll messages registered in message_types.hpp.
Correct rangeMessages use component's allocated hex range.
Naming conventionsRequests end with _request, responses with _response.

Protocol Structs

CheckDescription
Struct patternRequest/response structs are struct final.
Serializationserialize() documents wire format in Doxygen.
Deserializationdeserialize() returns std::expected.
Stream operatoroperator<< implemented for debugging.

Message Traits

CheckDescription
Traits definedmessage_traits specialization for each request type.
Correct typesrequest_type, response_type, request_message_type defined.

Handler

CheckDescription
Handler methodEach message type has a handle_* method.
Switch caseHandler is registered in message dispatch switch.
AuthorizationServer-side authorization checks in place.

Step 10: PlantUML diagram audit

Verify diagrams are up-to-date per plantuml-class-modeler skill.

CheckDescription
Diagram existsmodeling/ores.COMPONENT.puml exists.
All classes presentDiagram includes all domain types, services, repositories.
Relationships correctInheritance, composition, aggregation arrows are accurate.
Styling correctClasses use correct colours and stereotypes.
CompilesDiagram compiles without PlantUML errors.
Test suitesTest classes are included in the diagram.

Step 11: Unit test audit

Verify test coverage per unit-test-writer skill.

Coverage

CheckDescription
Domain testsEach domain type has domain_*_tests.cpp.
Repository testsEach repository has repository_*_tests.cpp.
Service testsEach service has service_*_tests.cpp.
Messaging testsProtocol structs have serialization tests.

Quality

CheckDescription
Naming conventionTests follow layer_class_tests.cpp pattern.
No SECTIONsTests use separate TEST_CASE instead of SECTION.
LoggingTests use BOOST_LOG_SEV for output.
Generators usedTests use generators for synthetic data.
Database helperRepository tests use database_helper.

Step 12: Error handling audit

CheckDescription
std::expected usageOperations that can fail return std::expected.
Error propagationErrors are propagated, not silently ignored.
Meaningful messagesError messages are descriptive and actionable.
LoggingErrors are logged at appropriate severity.
No exceptions for control flowExceptions reserved for truly exceptional cases.

Step 13: Security audit

CheckDescription
No hardcoded secretsNo passwords, keys, or tokens in code.
Input validationExternal input validated before use.
AuthorizationServer-side checks for all privileged operations.
No client-provided identityIdentity from session, not request fields.
Secure hashingPasswords use proper hashing (bcrypt, argon2).

Step 14: Generate findings

For each issue found, categorise by severity:

Critical (Must Fix)

  • Security vulnerabilities.
  • Data corruption risks.
  • Crashes or undefined behaviour.
  • Missing critical functionality.

Important (Should Fix)

  • Architecture problems.
  • Missing services (JSON I/O, tests, etc.).
  • Convention violations.
  • Technical debt that impedes development.
  • Outdated patterns that should be modernised.

Minor (Nice to Have)

  • Code style preferences.
  • Documentation improvements.
  • Optimization opportunities.
  • Refactoring suggestions.

Step 15: Document findings

For each issue provide:

  • File line - Exact location (or file/folder for structural issues).
  • What's wrong - Clear description of the problem.
  • Why it matters - Impact of not fixing.
  • How to fix - Suggested resolution.
  • Effort estimate - Small/Medium/Large.

Step 16: Add to sprint backlog

Create stories in the sprint backlog per agile-product-owner skill.

Group issues by theme:

  • Structure & CMake - Component structure issues.
  • Headers & Style - Copyright, naming, formatting.
  • Domain Completeness - Missing services for domain types.
  • Test Coverage - Missing or incomplete tests.
  • Modernization - C++23 upgrade opportunities.
  • Documentation - Missing Doxygen, diagrams.
  • Security - Security-related findings.

Example story format:

code
**** STARTED [component] Complete domain type services :code:

Several domain types are missing standard services.

***** Tasks
- [ ] Add JSON I/O for foo domain type
- [ ] Add table I/O for foo domain type
- [ ] Add generator for foo domain type
- [ ] Add temporality fields to bar domain type

***** Notes

Component review performed on YYYY-MM-DD.

Step 17: Summary report

Provide an overall assessment:

Component Health Score

Rate the component on each dimension (1-5):

DimensionScoreNotes
Structure
Code Style
Domain Completeness
Test Coverage
Documentation
Security
Modernization

Priority Recommendations

List top 3-5 items to address first, considering:

  1. Critical issues (must fix immediately).
  2. High-impact improvements (good ROI).
  3. Quick wins (easy fixes with visible benefit).

Technical Debt Summary

Summarize the overall technical debt level:

  • Low - Component is well-maintained, minor issues only.
  • Medium - Some gaps, but fundamentally sound.
  • High - Significant work needed before production-ready.