Skip to content

refactor: remove dead code, split utils into package, expand config output - #575

Merged
d-oit merged 1 commit into
mainfrom
chore/dead-code-cleanup
Aug 11, 2026
Merged

refactor: remove dead code, split utils into package, expand config output#575
d-oit merged 1 commit into
mainfrom
chore/dead-code-cleanup

Conversation

@d-oit

@d-oit d-oit commented Aug 11, 2026

Copy link
Copy Markdown
Owner

Summary

Consolidates an uncommitted cleanup pass into a single reviewed change (51 files, +910/−1968):

  • Python: split 789-line scripts/utils.py into a utils/ package (async_http, cache, content_clean, fetch, html, http, thread_pool, urls); delete dead provider_decorator.py; slim providers_impl.py by 561 lines; consolidate routing_memory/state/resolve; mirror into .agents/skills/do-web-doc-resolver/scripts/
  • CLI: init_logging respects config.log_level; config subcommand now prints routing.prewarm, cache.ttl, provider rate limits, min_free_quality_to_skip_paid; add ResolveMetrics unit tests; drop dead config/resolver code
  • Web: delete dead lib/providers.ts, ToggleChip.tsx, providers.test.ts; thread maxChars through provider functions; consolidate resolvers
  • Docs/plans: mark plan 11 (cache pre-warming) shipped; update plan 03 progress; DEPLOYMENT.md tweak

Validation

  • pytest -m "not live": 872 passed, 5 xfailed
  • cargo test: all binaries green
  • ./scripts/quality_gate.sh: all checks passed (ruff, black, fmt, clippy, eslint, tsc)

Test plan

  • Full offline Python suite
  • Full Rust suite
  • Web lint + typecheck
  • CI green

@do-ops885
refactor: remove dead code, split utils into package, expand config o…
de0fa59 
…utput

Python: split 789-line scripts/utils.py into a utils/ package
(async_http, cache, content_clean, fetch, html, http, thread_pool,
urls), delete dead provider_decorator.py, slim providers_impl.py,
consolidate routing_memory/state/resolve; mirror into skill scripts.

CLI: respect config log_level in init_logging, expand 'config'
subcommand output (routing.prewarm, cache.ttl, provider rate limits,
min_free_quality_to_skip_paid), add ResolveMetrics unit tests, drop
dead config/resolver code.

Web: delete dead lib/providers.ts, ToggleChip.tsx, providers.test.ts;
thread maxChars through provider functions; consolidate resolvers.

Docs: mark plan 11 (cache pre-warming) shipped, update plan 03
progress, DEPLOYMENT.md tweak.

Validated: pytest 872 passed (not live), cargo test green,
quality_gate.sh all checks passed.
@vercel

vercel Bot commented Aug 11, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
do-web-doc-resolover Ready Ready Preview Aug 11, 2026 11:33am

@deepsource-io

deepsource-io Bot commented Aug 11, 2026 •
edited
Loading

Copy link
Copy Markdown

DeepSource Code Review

We reviewed changes in f088a06...de0fa59 on this pull request. Below is the summary for the review, and you can see the individual issues we found as inline review comments.

See full review on DeepSource ↗

Important

Some issues found as part of this review are outside of the diff in this pull request and aren't shown in the inline review comments due to GitHub's API limitations. You can see those issues on the DeepSource dashboard.

PR Report Card

Overall Grade   Security  

Reliability  

Complexity  

Hygiene  

Code Review Summary

Analyzer Status Updated (UTC) Details
JavaScript Aug 11, 2026 11:33a.m. Review ↗
Python Aug 11, 2026 11:33a.m. Review ↗
Rust Aug 11, 2026 11:33a.m. Review ↗
Shell Aug 11, 2026 11:33a.m. Review ↗

Important

AI Review is run only on demand for your team. We're only showing results of static analysis review right now. To trigger AI Review, comment @deepsourcebot review on this thread.

deepsource-io[bot]
deepsource-io Bot reviewed Aug 11, 2026
View reviewed changes
Comment thread cli/src/config/parsing.rs
@@ -4,14 +4,26 @@ use super::Config;

pub fn apply_env_overrides(config: &mut Config) {

@deepsource-io deepsource-io Bot Aug 11, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

`fn apply_env_overrides` has a cyclomatic complexity of 59 with "critical" risk


A function with high cyclomatic complexity can be hard to understand and
maintain. Cyclomatic complexity is a software metric that measures the number of
independent paths through a function. A higher cyclomatic complexity indicates
that the function has more decision points and is more complex.

Comment thread cli/src/metrics.rs

#[test]
fn test_new_is_empty() {
let m = ResolveMetrics::new();

@deepsource-io deepsource-io Bot Aug 11, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Empty call to `new()`


The new() function is used to initialise an object with specific data.
If no arguments are passed, the behaviour is identical to default().

Comment thread cli/src/metrics.rs

#[test]
fn test_record_cache_hit() {
let mut m = ResolveMetrics::new();

@deepsource-io deepsource-io Bot Aug 11, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Empty call to `new()`


The new() function is used to initialise an object with specific data.
If no arguments are passed, the behaviour is identical to default().

Comment thread cli/src/metrics.rs
assert!(m.cache_hit);
assert!(!m.synthesis_cache_hit);

let mut m2 = ResolveMetrics::new();

@deepsource-io deepsource-io Bot Aug 11, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Empty call to `new()`


The new() function is used to initialise an object with specific data.
If no arguments are passed, the behaviour is identical to default().

Comment thread cli/src/metrics.rs

#[test]
fn test_record_gate_sets_score() {
let mut m = ResolveMetrics::new();

@deepsource-io deepsource-io Bot Aug 11, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Empty call to `new()`


The new() function is used to initialise an object with specific data.
If no arguments are passed, the behaviour is identical to default().

Comment thread cli/src/metrics.rs
assert!(!low.quality_gate_passed);
assert!(low.quality_gate_score.is_none());

let mut high = ResolveMetrics::new();

@deepsource-io deepsource-io Bot Aug 11, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Empty call to `new()`


The new() function is used to initialise an object with specific data.
If no arguments are passed, the behaviour is identical to default().

Comment thread cli/src/metrics.rs

#[test]
fn test_record_provider_success_marks_paid_usage() {
let mut m = ResolveMetrics::new();

@deepsource-io deepsource-io Bot Aug 11, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Empty call to `new()`


The new() function is used to initialise an object with specific data.
If no arguments are passed, the behaviour is identical to default().

Comment thread cli/src/metrics.rs
assert!(!m.provider_metrics[0].paid);
assert_eq!(m.provider_metrics[0].attempt_index, 0);

let mut m2 = ResolveMetrics::new();

@deepsource-io deepsource-io Bot Aug 11, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Empty call to `new()`


The new() function is used to initialise an object with specific data.
If no arguments are passed, the behaviour is identical to default().

Comment thread cli/src/metrics.rs

#[test]
fn test_record_provider_detailed_preserves_fields() {
let mut m = ResolveMetrics::new();

@deepsource-io deepsource-io Bot Aug 11, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Empty call to `new()`


The new() function is used to initialise an object with specific data.
If no arguments are passed, the behaviour is identical to default().

Comment thread cli/src/metrics.rs

#[test]
fn test_metrics_round_trip_serialization() {
let mut m = ResolveMetrics::new();

@deepsource-io deepsource-io Bot Aug 11, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Empty call to `new()`


The new() function is used to initialise an object with specific data.
If no arguments are passed, the behaviour is identical to default().

@codacy-production

codacy-production Bot commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

Not up to standards ⛔

🔴 Issues 3 medium

Alerts:
⚠ 3 issues (≤ 0 issues of at least minor severity)

Results:
3 new issues

Category Results
Security 3 medium

View in Codacy

🟢 Metrics 143 complexity · -2 duplication

Metric Results
Complexity 143
Duplication -2

View in Codacy

NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.

deepsource-io[bot]
deepsource-io Bot reviewed Aug 11, 2026
View reviewed changes
Comment thread .agents/skills/do-web-doc-resolver/scripts/constants.py

logger = logging.getLogger(__name__)

if typing.TYPE_CHECKING:

@deepsource-io deepsource-io Bot Aug 11, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Body doesn't contain any code


In most cases, an empty body of for, while or if implies some piece of code is missing.
Such empty block must be either filled or removed.

Comment thread .agents/skills/do-web-doc-resolver/scripts/resolve.py
resolve_url_stream = scripts._url_resolve.resolve_url_stream
resolve_query = scripts._query_resolve.resolve_query
resolve_query_stream = scripts._query_resolve.resolve_query_stream
resolve_url_stream_async = scripts._url_resolve_async.resolve_url_stream_async

@deepsource-io deepsource-io Bot Aug 11, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Access to a protected member _url_resolve_async of a client class


Accessing a protected member (a member prefixed with _) of a class from outside that class is not recommended, since the creator of that class did not intend this member to be exposed. If accesing this attribute outside of the class is absolutely needed, refactor it such that it becomes part of the public interface of the class.

Comment thread .agents/skills/do-web-doc-resolver/scripts/resolve.py
"""Async version of resolve_url."""
if isinstance(profile, str):
profile = Profile(profile.lower())
return await scripts._url_resolve_async.resolve_url_async(

@deepsource-io deepsource-io Bot Aug 11, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Access to a protected member _url_resolve_async of a client class


Accessing a protected member (a member prefixed with _) of a class from outside that class is not recommended, since the creator of that class did not intend this member to be exposed. If accesing this attribute outside of the class is absolutely needed, refactor it such that it becomes part of the public interface of the class.

Comment thread .agents/skills/do-web-doc-resolver/scripts/utils/__init__.py

def get_config_data() -> dict[str, Any]:
"""Load configuration from config.toml if available."""
global _CONFIG_DATA

@deepsource-io deepsource-io Bot Aug 11, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Using the global statement


It is recommended not to use global statement unless it is really necessary. Global variables are dangerous because they can be simultaneously accessed from multiple sections of a program. This frequently results in bugs. This also make code difficult to read, because they force you to search through multiple functions or even modules just to understand all the different locations where the global variable is used and modified. Read more about why it should be avoided here.

Comment thread .agents/skills/do-web-doc-resolver/scripts/utils/__init__.py
with open(config_path, "rb") as f:
_CONFIG_DATA = typing.cast(dict[str, Any], tomllib.load(f))
except Exception as e:
logger.debug(f"Failed to load config.toml: {e}")

@deepsource-io deepsource-io Bot Aug 11, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Use lazy % formatting in logging functions


Formatting the message manually before passing it to a logging call does unnecessary work if logging is disabled. Consider using the logging module's built-in formatting features to avoid that.

Comment thread .agents/skills/do-web-doc-resolver/scripts/utils/http.py
return h


def is_safe_url(url: str) -> bool:

@deepsource-io deepsource-io Bot Aug 11, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

`is_safe_url` has a cyclomatic complexity of 18 with "high" risk


A function with high cyclomatic complexity can be hard to understand and
maintain. Cyclomatic complexity is a software metric that measures the number of
independent paths through a function. A higher cyclomatic complexity indicates
that the function has more decision points and is more complex.

Comment thread .agents/skills/do-web-doc-resolver/scripts/utils/http.py
Comment on lines +125 to +126
"0.0.0.0",
):

@deepsource-io deepsource-io Bot Aug 11, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Possible binding to all interfaces.


Binding to all network interfaces can potentially open up a service to traffic on unintended interfaces, that may not be properly documented or secured. This can be prevented by changing the code so it explicitly only allows access from localhost.

Comment thread .agents/skills/do-web-doc-resolver/scripts/utils/thread_pool.py

def get_shared_pool(max_workers: int = 10) -> concurrent.futures.ThreadPoolExecutor:
"""Get or create the shared ThreadPoolExecutor."""
global _shared_pool

@deepsource-io deepsource-io Bot Aug 11, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Using the global statement


It is recommended not to use global statement unless it is really necessary. Global variables are dangerous because they can be simultaneously accessed from multiple sections of a program. This frequently results in bugs. This also make code difficult to read, because they force you to search through multiple functions or even modules just to understand all the different locations where the global variable is used and modified. Read more about why it should be avoided here.

Comment thread .agents/skills/do-web-doc-resolver/scripts/utils/thread_pool.py
"""Get or create the shared ThreadPoolExecutor."""
global _shared_pool
with _pool_lock:
if _shared_pool is None or _shared_pool._shutdown:

@deepsource-io deepsource-io Bot Aug 11, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Access to a protected member _shutdown of a client class


Accessing a protected member (a member prefixed with _) of a class from outside that class is not recommended, since the creator of that class did not intend this member to be exposed. If accesing this attribute outside of the class is absolutely needed, refactor it such that it becomes part of the public interface of the class.

Comment thread .agents/skills/do-web-doc-resolver/scripts/utils/thread_pool.py

def shutdown_shared_pool() -> None:
"""Shutdown the shared ThreadPoolExecutor."""
global _shared_pool

@deepsource-io deepsource-io Bot Aug 11, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Using the global statement


It is recommended not to use global statement unless it is really necessary. Global variables are dangerous because they can be simultaneously accessed from multiple sections of a program. This frequently results in bugs. This also make code difficult to read, because they force you to search through multiple functions or even modules just to understand all the different locations where the global variable is used and modified. Read more about why it should be avoided here.

@d-oit
d-oit merged commit 5fa032a into main Aug 11, 2026
85 of 89 checks passed
@d-oit
d-oit deleted the chore/dead-code-cleanup branch August 11, 2026 12:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@deepsource-io deepsource-io[bot] deepsource-io[bot] left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@d-oit @do-ops885