Skip to content

Fixes2#2

Open
RHUDHRESH wants to merge 2 commits intomainfrom
fixes2
Open

Fixes2#2
RHUDHRESH wants to merge 2 commits intomainfrom
fixes2

Conversation

@RHUDHRESH
Copy link
Copy Markdown
Owner

@RHUDHRESH RHUDHRESH commented Oct 5, 2025

Summary by CodeRabbit

  • Documentation
    • Streamlined README with simplified sections and cleaner formatting.
    • Updated Switch 6 instructions: --refresh-every now specifies seconds (not minutes).
    • Consolidated test behavior notes and clarified Position Validator Engine scoring list.
  • Bug Fixes
    • Live data fetching now correctly honors the configured timeout, improving reliability and consistency.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Oct 5, 2025

Walkthrough

Documentation in README.md is significantly reduced and reformatted. In code, RequestsFetcher now uses the instance timeout (self._timeout) when creating httpx.AsyncClient instead of an undefined local variable. No public APIs change.

Changes

Cohort / File(s) Summary
Documentation pruning and edits
README.md
Removed multiple sections and condensed content; adjusted CLI note for refresh interval from minutes to seconds; reformatted list items and minor whitespace.
Fetcher timeout correction
market_research/fetchers.py
Replaced undefined local timeout with self._timeout in httpx.AsyncClient; minor formatting cleanup.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

I nibbled docs down to a tidy trail,
Swapped minutes for seconds—swift as a tail.
In fetcher burrows, I fixed the time,
From stray to self, now requests align.
Thump-thump—clean hops, lean lines prevail.

Pre-merge checks and finishing touches

❌ Failed checks (1 warning, 1 inconclusive)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
Title Check ❓ Inconclusive The title “Fixes2” is overly generic and does not convey any meaningful information about the main changes in this pull request, such as the timeout refactor in RequestsFetcher or the updates to README content. As a result, it fails to help reviewers or future readers understand the scope or purpose of the changes. Please choose a concise, descriptive title that highlights the primary change, for example: “Use instance timeout in RequestsFetcher and update README refresh interval.”
✅ Passed checks (1 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
✨ 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 fixes2

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

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between face15a and a237509.

📒 Files selected for processing (2)
  • README.md (1 hunks)
  • market_research/fetchers.py (1 hunks)
🔇 Additional comments (3)
market_research/fetchers.py (1)

58-58: Critical bug fix: correct use of instance timeout.

This change fixes a critical bug by using the instance variable self._timeout (initialized in __init__ at line 35) instead of an undefined local variable. This prevents a NameError at runtime and ensures the configured timeout is properly applied to the HTTP client.

README.md (2)

54-54: LGTM!

The documentation clarification about test module behavior is clear and helpful for users who may not have the optional scientific dependencies installed.


69-69: LGTM!

The reformatted description improves readability by clearly listing the pluggable scoring modules and their configuration options.

Comment thread README.md
```

- `scripts/switch6_refresh.py` accepts `--refresh-every <minutes>` to loop continuously�ideal for cron/Task Scheduler.
- `scripts/switch6_refresh.py` accepts `--refresh-every <seconds>` to loop continuously—ideal for cron/Task Scheduler.
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

Verify the time unit change for the refresh script.

The documentation now indicates that --refresh-every accepts seconds instead of minutes. Please confirm that the actual implementation in scripts/switch6_refresh.py has been updated to match this documentation, as this is a user-facing parameter change.

Run the following script to verify the parameter implementation:


🏁 Script executed:

#!/bin/bash
# Description: Verify the refresh-every parameter unit in the script.
# Expected: Documentation should confirm whether the parameter uses seconds or minutes.

# Search for the refresh-every parameter definition and usage
rg -n -A5 -B5 'refresh.?every' --type=py

Length of output: 1651


Fix time-unit mismatch for --refresh-every.

The README entry at line 47 should reflect that --refresh-every is in minutes (per the script’s help text), or adjust the parser and loop logic if you intend it to be seconds.

🤖 Prompt for AI Agents
In README.md around line 47, the README states that scripts/switch6_refresh.py
accepts --refresh-every <seconds> but the script's help text uses minutes;
either update the README to say "<minutes>" (or "minutes") to match the script,
or change the script's argument parser and loop logic to treat the flag as
seconds. Fix by aligning the unit: if keeping minutes, update README text to
mention minutes and ensure any examples use minutes; if preferring seconds,
update the argparse help string, convert the parsed value to seconds where the
loop sleeps, and adjust tests/docs accordingly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant