Repository navigation
Document the 33 public symbols that had no doc comment - #141
Merged
Merged
Conversation
Every public symbol declared in the five library modules now has a doc comment, as the Coverage bullet of docs/development.md says. Comments only: no code, signature or access level changes. - VisionTower.swift (#129): the @ModuleInfo and @ParameterInfo properties and the callAsFunction methods of VisionAttention, VisionMLP, VisionBlock, VisionPatchEmbedder, VisionModel, VisionEncoder and MultimodalEmbedder (29), and the callAsFunction of ClippableLinear and VisionRMSNorm. A property says what it holds and its checkpoint key, as the text model's do; a callAsFunction says what it computes, citing mlx-vlm's __call__, with the file's shapes. - VisionError.description (#121): "The message.", as the other errors' descriptions say. - extension ChatCompletionsConfiguration in OpenJevServer (#136): a /// line on the extension, as BackendProvider.swift's two extensions of core types have.
Contributor
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
All added comments accurately describe the existing implementation and introduce no behavioral changes.
Review effort: Balanced
Findings: None
What changed in this PR
Adds missing API documentation across server, vision-error, and vision-tower public symbols without changing behavior.
Changes:
- Documents 31 vision-tower members and operations.
- Documents
VisionError.description. - Documents the chat-completions configuration extension.
| File | Description |
|---|---|
Sources/OpenJevServer/ChatCompletionsRoute.swift |
Documents generation settings mapping. |
Sources/OpenJevDiffusionGemma/Vision/VisionError.swift |
Documents the error description. |
Sources/OpenJevDiffusionGemma/Model/VisionTower.swift |
Documents public vision-layer properties and calls. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds a
///doc comment to each of the 33 public symbols that the five library modules declared without one, the ones PR #139's body listed at 4194e02. Comments only: every added line is a///line, and no code, signature or access level changes. With it, every public symbol declared inOpenJevCore,OpenJevServer,OpenJevDiffusionGemma,OpenJevEncodersandOpenJevLetterReadouthas a doc comment of its own (table below).The comments
Sources/OpenJevDiffusionGemma/Model/VisionTower.swift, 31 symbols (added by #129):ClippableLinearcallAsFunction(_:)VisionRMSNormcallAsFunction(_:)VisionAttentionqProj,kProj,vProj,oProj,qNorm,kNorm,callAsFunction(_:positions:mask:)VisionMLPgateProj,upProj,downProj,callAsFunction(_:)VisionBlockselfAttention,mlp,inputLayerNorm,postAttentionLayerNorm,preFeedforwardLayerNorm,postFeedforwardLayerNorm,callAsFunction(_:positions:mask:)VisionPatchEmbedderinputProj,positionEmbeddingTable,callAsFunction(_:positions:padding:)VisionModelpatchEmbedder,encoder,stdBias,stdScaleVisionEncoderlayers,callAsFunction(_:positions:mask:stages:)MultimodalEmbedderembeddingProjection,callAsFunction(_:)k_proj." inAttention.swift). For exampleVisionModel.stdBias: "The shift the standardization subtracts from the soft tokens,std_bias, or nil when the configuration does not standardize."callAsFunctionsays what it computes and returns. The four that take positions cite mlx-vlm's__call__invision.py, as the classes' comments citevision.py, and list their parameters and result in the file's shape notation ([B, L, hidden],[B, C, H, W],[B, 1, L, L]), asAttentionandDecoderLayerdo. The four that take one array say it in a line or two, asDenseMLPdoes;VisionMLP's is "down_proj(gelu_approx(gate_proj(x)) * up_proj(x)), inx's shape,[..., hidden]."Sources/OpenJevDiffusionGemma/Vision/VisionError.swift,description(#121): "The message.", the wordingServerSettingsError,JevK5ModelErrorandFixtureError, error structs of the same shape, use for the samedescription { message }.Sources/OpenJevServer/ChatCompletionsRoute.swift,extension ChatCompletionsConfiguration(#136): "The generation settings thatOPENJEV_GEN_*variables set, forPOST /v1/chat/completions.", afterBackendProvider.swift's "The engine settings thatOPENJEV_*variables set, forDecisionBackendProvider." The route type is internal, so the comment names the route in code voice instead of linking it.The vision comments rest on the code, the classes' comments, and mlx-vlm 0.6.15's
models/gemma4/vision.pyandgemma4.py, the version the file ports. Every key a property's comment names is part of the pinned checkpoint's tensor names (mlx-community/diffusiongemma-26B-A4B-it-4bit, itsmodel.safetensors.index.json), undermodel.encoder.vision_tower.ormodel.encoder.embed_vision.. The clipping boundsClippableLinear.callAsFunction(_:)names (input_minand the like) are not, since that checkpoint does not clip (use_clipped_linearsis false). The shapes and ranges were checked in the code:VisionModelbuilds the mask as[B, 1, L, L]and the positions as int32 with x first, the position table's first slice is x's, andpixel_valuesarrive in [0, 1] (each byte times 1/255, no normalization), which_patchifymaps to [-1, 1]. No symbol's purpose was unclear. No new comment contains a``link.Coverage, per module
OpenJevCoreOpenJevServerOpenJevDiffusionGemmaOpenJevEncodersOpenJevLetterReadoutA symbol counts when its
accessLevelispublicoropenand itslocation.uriis underSources/<Module>/. It has a doc comment when itsdocCommenthas a line with text and is its own: a comment copied from another module's protocol requirement (the graph marks it"module": "Swift", as forCustomStringConvertible.description's) does not count. Symbols with no location in the module's sources (synthesized and inherited members such as!=,hashValue, Actor'sassertIsolatedand Hummingbird'sRequestContextdefaults) need nothing and are left out. #137 (generation on the MLX backend) is still open, so its symbols are not counted.OpenJevCorecounts 735 where #139's table says 729. The 6 more are inOpenJevCore@Swift.symbols.json: the publicpythonReprextensions ofDoubleandStringinErrors/PythonRepr.swift(three extension blocks and three members), all documented. The next section says why that file was missed before.How the counts were taken
For each module I ran the check
docs/development.mdnames,swift package --allow-writing-to-directory "$DIR" generate-documentation --target <Module> --experimental-documentation-coverage --coverage-summary-level detailed --output-path "$DIR/<Module>.doccarchive", and read the symbol graphs in.build/out/Products/Debug/arm64/<Module>.symbolgraphs/. With this toolchain (Swift 6.4, SwiftPM's Swift Build backend), a graph file's modification time shows neither whether the run wrote it nor whether it is current:OpenJevCore@Swift.symbols.json(written at 10:02 today) andOpenJevServer@OpenJevCore.symbols.json(10:58) were byte for byte what a fresh extraction of 4194e02 gives. Moved aside, neither was re-created by a run over unchanged sources, and the 4194e02 run's DocC had read the 10:58 file: its warnings cite the comment ofEngineConfiguration.init(_:), which only that file holds, and with the file moved aside they are gone.So I also extracted each module's graphs, into an empty folder, from the module the run had just built:
swift-symbolgraph-extract -minimum-access-level public -skip-inherited-docs -emit-extension-block-symbols, with the build's module and header search paths. On this branch, these graphs and the ones in.buildhold the same symbols declared in the modules' sources, with the same doc comment text. At 4194e02 they matched too, apart from the extension-block files the run had not re-created. The table counts the extracted graphs.DocC's own report agrees. In
OpenJevDiffusionGemma'sdocumentation-coverage.json, 36 entries under the nine classes ofVisionTower.swiftandVisionErrorhad no abstract at 4194e02: the 32 above, plusVisionError's!=,localizedDescriptionand two "Implementations" groups, which no comment can document. On this branch only those 4 remain. InOpenJevServer's, the extension and itsinit(_:)both have an abstract.The Coverage bullet in docs/development.md
#139 merged with a Copilot Autofix commit that turned the bullet into a rule, "Every public symbol needs a doc comment.", in place of the claim "Every public symbol declared in the five modules has a doc comment, and a new one needs one too." This pull request leaves
docs/development.mdalone (open #140 edits the bullet just above). With it merged the claim holds, and the table above is its evidence, if the bullet should say so again.Checks
make lintpasses.generate-documentationruns raise no warning in the three changed files; their warnings are the expected cross-module ones (Can't resolve 'OpenJevCore'). The new comments contain no links, and each- Parameters:list names exactly its function's parameters, so the Documentation workflow (every DocC warning an error,Sources/**in its paths) has nothing new to resolve.make docswas not run locally.swift testwas not run: comments change no behavior, and CI runs the tests.mainat 70d0126 (Say the DocC site has five modules, not four #139).