docs: audit and refresh README - #1
Merged
Merged
Conversation
Review Summary by QodoAudit and refresh README to match current multi-project structure
WalkthroughsDescription• Reframe README as umbrella for multi-project repository • Document Florence-2 extractor structure and CLI subcommands • Add ProjectBroadside layout table covering sub-projects • Replace boilerplate sections with status note reflecting reality Diagramflowchart LR
A["Old README<br/>Florence-2 only"] -->|Reframe| B["New README<br/>Umbrella structure"]
B --> C["Florence-2<br/>Extractor section"]
B --> D["ProjectBroadside<br/>Sub-projects table"]
B --> E["Repository layout<br/>table"]
D --> F["BattleshipMaker<br/>BattleshipMaker2<br/>MCPUnityRockstar<br/>Scripts"]
File Changes1. README.md
|
Code Review by Qodo
1. Wrong env var names
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
Comment on lines
90
to
97
| ```bash | ||
| # Model settings | ||
| FLORENCE_MODEL_NAME="microsoft/Florence-2-large" | ||
| FLORENCE_DEVICE="cuda" | ||
|
|
||
| # Processing settings | ||
| PDF_DPI=300 | ||
| DETECTION_CONFIDENCE_THRESHOLD=0.7 | ||
|
|
||
| # Output settings | ||
| OUTPUT_DIR="./extracted_warships" | ||
| SAVE_ANNOTATED_IMAGES=true | ||
| ``` |
There was a problem hiding this comment.
1. Wrong env var names 🐞 Bug ≡ Correctness
README.md documents configuration variables like FLORENCE_MODEL_NAME/OUTPUT_DIR, but Settings only reads variables with the WARSHIP_ prefix and different field names, so the documented exports won’t affect runtime configuration. This will silently fall back to defaults and mislead users trying to configure the extractor.
Agent Prompt
### Issue description
README.md lists environment variables that do not match the extractor’s `Settings` implementation. `Settings` uses `env_prefix = "WARSHIP_"` and field names like `florence_model_name`, `default_dpi`, `output_directory`, and `save_visualizations`, so exporting `FLORENCE_MODEL_NAME`, `PDF_DPI`, `OUTPUT_DIR`, etc. has no effect.
### Issue Context
The extractor configuration is implemented via `pydantic-settings` in `src/warship_extractor/config/settings.py`.
### Fix Focus Areas
- README.md[88-97]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+65
to
67
| # Generate an analysis report | ||
| warship-extract report results/ --format html | ||
| ``` | ||
|
|
There was a problem hiding this comment.
2. Invalid report cli example 🐞 Bug ≡ Correctness
README.md shows warship-extract report results/ --format html, but the CLI requires a JSON file positional argument and accepts --output-dir/--template (no --format). Running the documented command will fail with argparse errors.
Agent Prompt
### Issue description
The README’s `report` command example uses a non-existent `--format` flag and passes a directory-like path, while the CLI expects `report <results_file.json>` and supports `--output-dir` and `--template`.
### Issue Context
The CLI is implemented with `argparse` in `src/warship_extractor/cli.py`.
### Fix Focus Areas
- README.md[58-69]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
75
to
84
| from warship_extractor.pipeline import ExtractionPipeline | ||
| from warship_extractor.config import Settings | ||
|
|
||
| # Initialize pipeline | ||
| settings = Settings() | ||
| pipeline = ExtractionPipeline(settings) | ||
|
|
||
| # Extract warships from PDF | ||
| results = pipeline.extract_from_pdf("janes_1900.pdf") | ||
|
|
||
| # Access results | ||
| for detection in results['detections']: | ||
| for detection in results["detections"]: | ||
| print(f"Found {detection['label']} with confidence {detection['confidence']:.2f}") | ||
| ``` |
There was a problem hiding this comment.
3. Broken python api snippet 🐞 Bug ≡ Correctness
README.md’s Python API example imports ExtractionPipeline/Settings from modules that don’t expose those names and calls extract_from_pdf(), but the implemented API is ExtractionPipeline.process_pdf() and Settings lives in config/settings.py. Copy/pasting the snippet will raise import/method errors.
Agent Prompt
### Issue description
The README’s Python API snippet doesn’t match the actual public surface:
- `Settings` is defined in `warship_extractor.config.settings`.
- `ExtractionPipeline` is defined in `warship_extractor.pipeline.extraction_pipeline`.
- The pipeline method is `process_pdf(...)` (no `extract_from_pdf`).
- Returned results are `ExtractionResult` objects containing `Detection` objects.
### Issue Context
The pipeline class and settings class are defined under `src/warship_extractor/`.
### Fix Focus Areas
- README.md[72-84]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
There was a problem hiding this comment.
Pull request overview
This PR refreshes the top-level README to describe the repository as an umbrella for multiple sub-projects (the Florence-2 extractor plus the ProjectBroadside 3D/Unity pipelines and supporting assets/docs), instead of focusing solely on the extractor.
Changes:
- Reframes the README around the repo’s multi-project structure and adds a clear repository layout + ProjectBroadside subproject listing.
- Updates extractor documentation to point at the
src/warship_extractor/entry points and thewarship-extractCLI. - Replaces generic Contributing/License/Acknowledgments boilerplate with a “Status” section reflecting the exploratory nature of the repo.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| Generate analysis report: | ||
| ```bash | ||
| # Generate an analysis report | ||
| warship-extract report results/ --format html |
Comment on lines
75
to
83
| from warship_extractor.pipeline import ExtractionPipeline | ||
| from warship_extractor.config import Settings | ||
|
|
||
| # Initialize pipeline | ||
| settings = Settings() | ||
| pipeline = ExtractionPipeline(settings) | ||
|
|
||
| # Extract warships from PDF | ||
| results = pipeline.extract_from_pdf("janes_1900.pdf") | ||
|
|
||
| # Access results | ||
| for detection in results['detections']: | ||
| for detection in results["detections"]: | ||
| print(f"Found {detection['label']} with confidence {detection['confidence']:.2f}") |
Comment on lines
91
to
96
| FLORENCE_MODEL_NAME="microsoft/Florence-2-large" | ||
| FLORENCE_DEVICE="cuda" | ||
|
|
||
| # Processing settings | ||
| PDF_DPI=300 | ||
| DETECTION_CONFIDENCE_THRESHOLD=0.7 | ||
|
|
||
| # Output settings | ||
| OUTPUT_DIR="./extracted_warships" | ||
| SAVE_ANNOTATED_IMAGES=true |
| - **Detection Accuracy**: 85-95% for clear illustrations | ||
| - **Memory Usage**: 2-4GB GPU memory for large documents | ||
| - **False Positive Rate**: <10% with proper filtering | ||
| `pyproject.toml` already wires up `pytest --cov` against `src/warship_extractor` by default. |
| - Multi-prompt strategy to catch different illustration styles | ||
| - High-resolution PDF rasterization (300+ DPI by default) | ||
| - Non-Maximum Suppression to drop duplicate detections | ||
| - CUDA-aware with CPU fallback; dynamic batch sizing |
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.
ProjectBroadside/(BattleshipMaker, BattleshipMaker2, HullCutter, MCPUnityRockstar, Unity scripts),Docs/,Scans/, andBAttleships/. Reframe the README as an umbrella for those sub-projects.git clone <repository-url>/cd warship-extractorblock — the repo isn't named that and isn't cloned that way.src/warship_extractor/withpipeline/,detection/,processing/,core/,config/,utils/,cli.py) and thewarship-extractCLI subcommands wired up in pyproject.ProjectBroadside.Scripts/(Unity C#).