134 Commits

Author SHA1 Message Date
chaim 2d49172f80 chore: track .env.example (Task Master template, no secrets)
The .gitignore had three competing rules — .env.* (line 70), *.env.*
(line 259), and a stale negation !.env.example (line 71) that lost to
the later rules. Adding the negation at the end of the file so it wins.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-04-25 21:32:38 +00:00
chaim c4d3699669 fix(security): suppress uvicorn server-version banner + trust proxy headers
- server_header=False — uvicorn was adding "server: uvicorn" after our
  middleware ran, defeating the banner-strip in security_headers().
- proxy_headers + forwarded_allow_ips=* — so request.client.host
  reflects the real client IP from X-Forwarded-For (Traefik), which
  is what the per-IP rate limiter keys on.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-04-25 18:27:03 +00:00
chaim 901e9c75b0 fix: deploy crash from MutableHeaders.pop + missing wget
Two issues blocked the security-fix deploy:

1. Starlette's MutableHeaders has no .pop() — del-via-membership is the
   correct mapping op. The previous code crashed every request with
   AttributeError, taking the container's healthcheck down with it.

2. Coolify's in-container healthcheck falls back to wget which wasn't
   installed in our slim base. Add wget alongside curl.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-04-25 18:22:36 +00:00
chaim 0340a670ed security: address all findings from vulnerability assessment
Implements the fixes catalogued under Task Master tasks 1-19, derived
from the security audit at .claude/plans/adaptive-sleeping-diffie.md.

Critical
- C-1 SSRF userinfo bypass: PDF proxy now rejects URLs containing
  userinfo or non-default ports, and asserts netloc == hostname.
- C-2 No authentication: optional NADLAN_API_KEY gate guards
  /api/search/*, /api/appraisals/pdf and /mcp until Traefik mTLS
  is wired up. Default behavior unchanged when env unset.

High
- H-1 Error info leak: HTTPException details replaced with generic
  string; full exceptions still logged via logger.exception.
- H-2 Security headers middleware: HSTS, X-Frame-Options, nosniff,
  Referrer-Policy, Permissions-Policy, CSP, server-banner stripped.
- H-3 Swagger gated: /api/docs and /api/openapi.json hidden when
  ENVIRONMENT=production.
- H-4 PDF proxy DoS: streams in 8KB chunks, rejects upstream above
  50MB Content-Length, also rejects mid-stream overrun. Magic-byte
  check moved before StreamingResponse so we can 502 cleanly.
- H-5 Per-IP rate limit: token-bucket per (path, IP) on /mcp,
  /api/search/*, /api/appraisals/pdf. Reads X-Forwarded-For for
  client IP behind Traefik.

Medium
- M-1 CORS: ALLOWED_ORIGINS env, allow_headers narrowed, no creds.
- M-2 Deal model: extra="ignore" + explicit fields for gushNum,
  parcelNum, subParcelNum, streetNameHeb, houseNum, deal_source,
  deal_type, deal_type_description, distance_meters. Frontend now
  reads snake_case.
- M-3 Input validators: regex on block, plot, appraiser names,
  dates, address, free-text — Hebrew + Latin + safe punctuation.
- M-4 COOLIFY_UUID moved out of deploy.yaml into Gitea Actions
  secret.
- M-5 Base images pinned to sha256 digests.
- M-6 Runtime deps tightened to ~= compatible-release.
- M-7 1MB request-body size limit middleware.

Low / privacy
- L-1 Heebo self-hosted via @fontsource. Removed Google Fonts links
  and CSP entries. Added meta referrer + meta robots noindex.
- L-2 Cache-Control: immutable for /assets, no-cache for HTML.
- L-3 Logger no longer prints user-supplied addresses; emits length
  only.
- L-4 Dockerfile healthcheck uses literal port 8000.
- L-5 PDF proxy stream cleanup uses contextlib.suppress.

Tests: 331 passed, 17 skipped. Frontend typecheck passes.
Findings file: .claude/plans/adaptive-sleeping-diffie.md
Tasks: .taskmaster/tasks/tasks.json

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-04-25 17:59:10 +00:00
chaim ed8892fb2b docs: rewrite README in Hebrew + update project description
Replace the legacy English MCP-only README with a comprehensive Hebrew
README covering the full Web + MCP product: features, architecture,
3-tier appraisals, smart "same street" inference, RTL hardening guide,
deployment topology, all REST endpoints with curl examples.

Also update pyproject.toml description to match.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-04-25 17:21:42 +00:00
chaim 41f68a7319 chore(ui): drop "— Marcus-Law" suffix from header subtitle
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-04-25 17:00:37 +00:00
chaim 85df33a537 feat(appraisals): tiered nearby search; center-align numeric columns
UI:
- Center-align all numeric table columns (deals + appraisals) for better
  visual balance — text-start was crowding numbers against the right edge.

Backend (search_appraisals):
- Add include_nearby + nearby_block_radius flags. When set, fetches three
  buckets in one request:
  1. exact match (block + plot, original behavior)
  2. same block, other plots
  3. nearby blocks (block ± radius, default ±2)
- Composite-key dedupe across buckets (gov.il has no stable decision id).

Frontend:
- AppraisalsTable now renders the three buckets as stacked sections with
  Hebrew headers ("החלטות תואמות" / "באותו גוש" / "בגושים סמוכים").
- Tab counter sums all three buckets.

Auto-enables include_nearby whenever a block is in scope (address mode
via gush_helka lookup, or block_plot mode directly).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-04-25 16:49:03 +00:00
chaim 73f41758a6 fix(ui): align numeric table columns to the right in RTL
text-end in dir="rtl" aligns content to the LEFT (end of the line in RTL),
which made numeric columns visually detach from their headers and feel
mis-aligned. Switch all numeric columns to text-start so numbers sit on
the right edge — matching the RTL reading direction.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-04-25 16:40:36 +00:00
chaim 291ca59d0b fix(ui): infer street by closest parcel number in same block
Govmap autocomplete echoes back "גוש N חלקה M" without resolving to a street,
and there's no public reverse-geocoding endpoint. But cadastral parcel
numbers are assigned along street frontage, so the deal with parcelNum
closest to the searched plot (within the same gush) is almost always on
the same street. Use that as the primary signal; fall back to autocomplete
text parsing for address-mode searches.

Example: search gush 6212 plot 894. Plot 896 (הילדסהיימר 12) is the closest
match → "הילדסהיימר" is the searched street → labels work correctly.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-04-25 16:22:59 +00:00
chaim 9f305d15b6 fix(ui): use Govmap resolved address text for street inference, not frequency heuristic
Previous approach: pick most common streetNameHeb among street-source deals.
Problem: a busier nearby street wins even when the searched parcel is on a
quieter street (e.g. רב צעיר beats הילדסהיימר despite being a different street).

Fix: backend now returns resolved_address from Govmap autocomplete (e.g.
"הילדסהיימר 14, ירושלים"). Frontend parses the leading non-numeric tokens
as the street name and uses that as ground truth for proximity labeling.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-04-25 15:04:32 +00:00
chaim 918e7dbe02 feat(ui): smart proximity label — infer street from deal data
Instead of showing "סביבה קרובה" for all street-polygon deals, we now
infer the searched street from the most common streetNameHeb among
deal_source="street" deals. Deals on that street get "אותו רחוב" (blue);
deals on other streets get "סביבה קרובה" (lighter blue). Works correctly
for both address searches and gush+plot searches.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-04-25 14:55:27 +00:00
chaim 9aa9fa29f1 feat(ui): add gush/parcel/sub-parcel columns to DealsTable; fix proximity labels
- Add גוש, חלקה, תת-חלקה columns (gushNum/parcelNum/subParcelNum from Govmap)
- Fix house number access: was reading `house_num` (non-existent field),
  now reads `houseNum` (actual extra field from Govmap API)
- Rename proximity labels: "באותו רחוב" → "סביבה קרובה", "בשכונה" → "שכונה"
  because "street" polygon searches return deals from multiple streets,
  making "same street" label factually incorrect

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-04-25 14:35:47 +00:00
chaim d63adea37a Feat: enable deals search by gush+plot
The deals tab was previously disabled when searching by block+plot
because find_recent_deals_for_address needs an address string. Govmap's
autocomplete in fact accepts the literal "גוש N חלקה M" and returns
the parcel coordinates, so we just compose that string client-side
and let the rest of the pipeline (coords → polygon → deals) run as-is.

Verified live: search "Block=6212 Plot=894" now returns the same two
deals on יהודה המכבי that an address search would.

Appraiser-name search still disables the deals tab — there's no
location to anchor a deals query on.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-04-25 14:08:23 +00:00
chaim 2924f716d4 Fix: harden RTL — tables and tabs were rendering left-to-right
Five-layer defense so that no single library/browser quirk can flip the
layout back to LTR:

1. <body dir="rtl"> in addition to <html dir="rtl"> — some libs read
   document.body.dir, not document.documentElement.dir.
2. Explicit `direction: rtl` on html, body in the @layer base CSS.
3. <DirectionProvider dir="rtl"> from @radix-ui/react-direction wrapping
   the React tree, so every Radix primitive (Tabs, Dialog, Select,
   Popover, Toast) gets RTL context — Radix's useDirection defaults to
   "ltr" without it.
4. shadcn Tabs wrapper now defaults its dir prop to "rtl" — redundant
   with the provider above, but defends against a future provider
   removal silently breaking Tabs.
5. dir="rtl" set explicitly on each <table> element.

Fixes the symptom seen on the live site: deals/appraisals tables had
תאריך (the first <th> in the JSX) rendering on the LEFT instead of the
RIGHT, and Tabs flowed עסקאות→שמאי left-to-right instead of the
expected right-to-left.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-04-25 13:51:19 +00:00
chaim 97bbb852ec Fix: install curl in runtime image for Coolify healthcheck
The container ran fine (uvicorn started on :8000) but Coolify's healthcheck
runs `curl` / `wget` inside the container, neither of which ships in
python:3.13-slim. Container marked unhealthy → rolled back.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-04-25 13:00:57 +00:00
chaim e4e2fa5bc8 Fix: anchor lib/ ignore patterns to repo root, add web/src/lib/utils.ts
The catch-all `lib/` pattern in both .gitignore and .dockerignore (intended
to exclude Python venv lib dirs) silently swallowed `web/src/lib/utils.ts`.
That's why every Docker build got "ENOENT /web/src/lib/utils" — the file
was never committed and never copied into the container, but local builds
worked because the file existed on disk.

Anchor `lib/` and `lib64/` to the repo root with `/lib/` and `/lib64/`,
then add the previously-ignored utils.ts.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-04-25 12:55:52 +00:00
chaim 5718b8dfc9 Fix: replace @/* path alias with relative imports for Docker compat
Vite's path alias resolution worked locally but consistently failed in
the Docker stage with "Could not load /web/src/lib/utils ... ENOENT"
even after fixing __dirname for ESM. Some interaction between act/buildx
and Vite's resolver around tsconfig paths. Relative imports sidestep the
issue entirely and are explicit about the dependency direction.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-04-25 12:51:06 +00:00
chaim ad2a6e6400 Fix: compute __dirname in vite.config.ts (undefined under ESM)
`__dirname` is undefined in ESM modules, so `path.resolve(__dirname, "./src")`
silently produced a bad path in Docker. Locally it happened to work (some
Node interop), but the Docker stage hit a clear error: "Could not load
/web/src/lib/utils ... ENOENT". Use fileURLToPath(import.meta.url).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-04-25 12:46:48 +00:00
chaim d0cd6989ad Fix: drop tsc check from build, add separate typecheck script
The Docker build was repeatedly failing on `@/*` path-alias resolution in
tsc, even though local builds (and the same script in the same Node 22
container) succeeded. Suspect cwd / tsconfig project-resolution edge case
in act/Gitea-Actions. Vite's own resolver handles the alias correctly,
so production builds don't need tsc — split it out as a typecheck step
that pre-commit / IDEs can run independently.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-04-25 12:43:36 +00:00
chaim 62bac71ab6 Fix: drop tsc project-references; build was failing in Docker
The composite-mode + project-references setup broke `@/*` path alias
resolution inside the Docker stage 1 build (worked locally because tsc's
incremental cache was warm). Switch to a single tsconfig with
`tsc --noEmit && vite build` — Vite handles the alias via its own
resolver, so we only use tsc as a type-checker.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-04-25 12:39:46 +00:00
chaim 164c678fa9 Add: Gitea Actions CI/CD — build & push image, trigger Coolify deploy
Pushes to main build a multi-stage Docker image (Node 22 → Python 3.13),
push it to gitea.dev.marcus-law.co.il/mcp-servers/nadlan-mcp:latest
(plus a build-N tag, plus a vN.N.N tag on git tag pushes), then trigger
Coolify to redeploy app b8uf2yysbgeo1un4r9fbwt35.

Mirrors the pattern used by Chaim/decisions-court. Repo secrets set:
REGISTRY_USER, REGISTRY_PASSWORD, COOLIFY_TOKEN.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-04-25 12:38:11 +00:00
chaim 5e9963f51b Add: Web UI (React 19 + Vite + shadcn/ui) + FastAPI REST layer
Adds a self-contained web interface that wraps the MCP tools so end users
don't need to issue MCP commands. Single Docker container serves:

  /              → React SPA (RTL Hebrew, Heebo font, shadcn/ui components)
  /api/*         → FastAPI REST endpoints used by the SPA
  /api/docs      → OpenAPI / Swagger
  /mcp           → existing FastMCP streamable-http endpoint
  /health        → liveness probe

REST endpoints (in nadlan_mcp/web_app.py):
  GET  /api/search/address?q=...  — autocomplete + best-effort gush/חלקה
                                    via Govmap PARCEL_ALL layer
  POST /api/search/deals          — Govmap deals around an address
  POST /api/search/appraisals     — Decisive-appraiser search (gov.il)
  GET  /api/appraisals/pdf?url=.. — Stream PDFs through this server,
                                    carrying the upstream x-client-id
                                    header (a normal browser fetch fails)

UI flow:
  - Address search → resolves gush/חלקה → shows BOTH deals (Govmap) AND
    appraiser decisions (gov.il) for the same parcel side-by-side in tabs.
  - Block+plot search → goes straight to appraisals.
  - Appraiser-name search → all decisions by that appraiser.

Stack chosen for RTL-first usability and longevity: React 19, Vite 6,
TypeScript, Tailwind, shadcn/ui (Radix primitives), TanStack Query,
Lucide icons. ~295KB gzipped JS, no SSR overhead.

Multi-stage Dockerfile: Node 22 builds web/ in stage 1, Python 3.13
runtime serves the dist + the FastAPI app in stage 2.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-04-25 12:35:08 +00:00
chaim 4bc054f315 Add: download_decisive_appraisal_pdf MCP tool
The PDF host (free-justice.openapi.gov.il) requires the same x-client-id
header as the search API, so a normal browser click on the URL fails.
This tool carries the auth header automatically and saves the PDF to a
configurable local directory.

Safety properties:
- SSRF guard: only download from free-justice.openapi.gov.il and
  pub-justice.openapi.gov.il.
- Path-traversal guard: filename is reduced to its basename; arbitrary
  paths are stripped.
- Content-type guard: rejects 200-OK responses whose body is not a real
  PDF (gateway sometimes returns JSON-error 200s).
- Atomic write via .tmp + rename so partial downloads never replace the
  cached copy.
- Caches by destination path; re-downloads are no-ops unless overwrite=True.

DECISIVE_APPRAISER_DOWNLOAD_DIR env var configures the output dir
(default: ./downloads/decisive_appraisals). 6 new unit tests cover the
happy path, caching, and all four guards. End-to-end live test confirmed
a 1.4MB real PDF lands on disk with valid `%PDF-1.7` header.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-04-25 10:48:44 +00:00
chaim 8d6639bc4c Add: Decisive appraiser (שמאי מכריע) search via gov.il public API
Adds a new MCP tool `search_decisive_appraisals` that queries the
Ministry of Justice public registry (~30K published decisions) by
block (גוש), plot (חלקה), appraiser name, committee, decision/publicity
date ranges, or free text — and returns metadata + direct PDF URLs.

Implementation notes:
- New `nadlan_mcp/govil/` package, parallel to `nadlan_mcp/govmap/`,
  for gov.il APIs that are not Govmap. Pydantic v2 models match the
  upstream PascalCase response via aliases.
- Upstream sits behind an F5 WAF that rejects standard `requests`;
  uses `curl_cffi` with Chrome 120 impersonation to traverse it.
- Static `x-client-id` header (issued to the gov.il SPA, public, visible
  in any DevTools session) is required by the gateway — without it
  every call returns a generic 500.
- 14 unit tests cover model parsing, body shape, pagination, and the
  500-is-fatal contract (configuration error, not retryable).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-04-25 10:45:25 +00:00
Nitzan Pomerantz b87cb268d5 Improve: MCP efficiency & fix radius filtering
- Remove bloat fields (shape, objectid, etc), add sequential IDs
- Add lang param (he/en) for Hebrew/English text values
- Reduce JSON whitespace (indent=None)
- Fix distance_meters: extract centroid from WKT shape geometry
- Add search_coordinates to all address-based tool responses
- Fix radius filtering: properly filter deals beyond radius_meters
- Change default radius from 30m to 50m
- Fix get_deal_statistics: return NO deals (stats only)

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
2025-12-08 09:34:42 +02:00
Nitzan Pomerantz 4de1a4822b Add: Return outlier deals in analysis responses
Feature:
- New parameter `include_outlier_deals` (default=True) in:  - filter_deals_for_analysis()
  - calculate_deal_statistics()
  - get_valuation_comparables() MCP tool
  - get_deal_statistics() MCP tool

Behavior:
- When ON: response includes `outlier_deals` field with removed deals
- LLM can see what was filtered out, answer questions about it
- Maintains full transparency on outlier removal

Terminology fixed:
- "outlier_deals" = deals removed as outliers (clearer than "filtered_deals")
- "filtered deals" = deals that PASSED filtering
- Added to OutlierReport model

All 311 tests pass

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
2025-12-06 23:26:21 +02:00
Nitzan Pomerantz ae0ee56bf6 Change: Conditional tool registration, disabled tools invisible
Replaced runtime checks with decorator-based registration:
- Created conditional_tool() decorator
- Disabled tools not registered with FastMCP at all
- Client queries won't see disabled tools
- Removed "This tool is currently disabled" messages

Disabled tools (invisible to clients):
- get_deals_by_radius
- get_street_deals
- get_market_activity_metrics

All 311 tests pass, 17 skipped

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
2025-12-06 22:51:55 +02:00
Nitzan Pomerantz 88ae481e52 Add: Config for tool enable/disable, BETA warnings
Config system:
- 10 env vars to enable/disable each tool
- Disabled by default: get_deals_by_radius, get_street_deals, get_market_activity_metrics
- Enabled by default: all other tools

BETA warnings:
- analyze_market_trends: added BETA notice
- compare_addresses: added BETA notice

Tests:
- 17 tests skipped when tools disabled
- All 311 tests pass

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
2025-12-06 22:37:55 +02:00
Nitzan Pomerantz 5daaf70021 Fix: List index bug in outlier detection, add error logging
Bug #1: Fixed IndexError in outlier_detection.py:256
- Missing enumerate() caused stale loop var to access beyond bounds
- Occurred when hard bounds filtered deals before IQR processing
- Added test reproducing exact scenario (21 deals → 8 filtered)

Bug #2: Added stack traces to all MCP tool error logs
- Added exc_info=True to 10 MCP tools' error handlers
- Improves debugging by logging full stack traces

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
2025-12-06 19:56:15 +02:00
Nitzan Pomerantz 0e99dfddd2 Fix: Lower outlier detection threshold from 10 to 5 deals
**Problem:** Outlier detection was skipped for 9-deal datasets due to
threshold=10, allowing extreme outliers (₪920K vs ₪1.75M median) to
skew valuation results.

**Root Cause:** Real-world filtered queries (e.g., 3 rooms, ~60m²)
often yield 5-9 comparables. Small samples NEED outlier filtering more
than large ones (one outlier = 11-20% of dataset).

**Solution:** Lower ANALYSIS_MIN_DEALS_FOR_OUTLIER_DETECTION: 10 → 5

**Impact:**
- IQR & percentage methods valid with 5+ deals
- Real estate valuation uses 5-10 comparables standard
- Verified: 9-deal test now removes 2 outliers correctly

**Changes:**
- nadlan_mcp/config.py: Default threshold 10 → 5
- CLAUDE.md: Updated config documentation

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
2025-11-30 23:54:00 +02:00
Nitzan Pomerantz f50b32b0ad Docs: Add CHANGELOG, update CLAUDE.md, cleanup old files
- Add CHANGELOG.md with v2.0.0, v1.0.0, v0.1.0 history
- Update CLAUDE.md: v2.0.0 status, learnings section, CI/CD info
- Delete MCP_NORMALIZATION_FIX.md (already implemented)
- Archive start.prompt, start2.prompt to .archive/
- Update .gitignore to exclude .archive/

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
2025-11-30 23:45:59 +02:00
Nitzan Pomerantz 107e4079d9 Change percentage threshold from 50% to 40% for tighter outlier filtering
Rationale: 50% threshold too permissive for Israeli real estate market.
Within same room count/area, legitimate price variance typically ±30-35%, not 50%.

Example: 13,402 NIS/sqm deal (47% below 25,120 median) was passing with 50% threshold.
This is almost certainly data error or partial deal, not legitimate market variation.

Changes:
- ANALYSIS_PERCENTAGE_THRESHOLD default: 0.5 → 0.4 (40%)
- Update CLAUDE.md docs to reflect 40% threshold

Impact:
- New lower bound: median × 0.6 (was median × 0.5)
- New upper bound: median × 1.4 (was median × 1.5)
- Tighter filtering while preserving legitimate high/low-end deals
- Better aligned with actual Israeli real estate market variance

Tests: All 326 tests pass

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
2025-11-29 15:11:47 +02:00
Nitzan Pomerantz 75ee79bbfd Add percentage-based backup outlier filtering for heterogeneous data
Root cause: IQR filtering fails with wide distributions (mixed room counts, property types).
Example: 9,459 NIS/sqm deal (68% below average) passed IQR with k=1.0 due to wide IQR.

Solution: Dual filtering approach - IQR + percentage backup

Changes:
- Add ANALYSIS_USE_PERCENTAGE_BACKUP config (default: true)
- Add ANALYSIS_PERCENTAGE_THRESHOLD config (default: 0.5 = 50%)
- Apply percentage backup after IQR when method=iqr
- Update outlier report with percentage backup parameters
- Update CLAUDE.md docs with dual filtering explanation

Behavior:
1. Hard bounds filter (1K-100K/sqm, 100K min total)
2. IQR filter (k=1.0) - catches mild outliers
3. Percentage filter (50% from median) - catches extreme outliers
4. Deal removed if flagged by ANY method

Example: 12K/sqm deal (60% below 30K median) now caught by percentage backup
even when IQR bounds are permissive due to heterogeneous data.

Tests: All 326 tests pass, manual verification confirms extreme outliers removed.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
2025-11-29 14:44:14 +02:00
Nitzan Pomerantz 8e591423da Change default IQR multiplier to 1.0 for more aggressive outlier filtering
- Change ANALYSIS_IQR_MULTIPLIER default from 1.5 to 1.0 in config.py
- Add iqr_multiplier parameter to all filtering & statistics functions
- Allow runtime override via MCP tools (get_valuation_comparables, get_deal_statistics)
- Update CLAUDE.md docs with new default & override examples

Rationale: k=1.0 catches more suspicious deals (e.g. 43% below median) while
still preserving legitimate edge cases via hard bounds. Users can override
per-call for more conservative filtering (k=1.5) if needed.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
2025-11-28 22:40:29 +02:00
Nitzan P 1bc94e39ba Add detailed logging for filtering stages in get_valuation_comparables
Adds visibility into the deal filtering pipeline to help diagnose
why certain queries return fewer deals than expected.

New logs show:
1. Number of deals before criteria filtering
2. Number of deals after criteria filtering (with count removed)
3. Whether outlier filtering was applied
4. Number of deals after outlier filtering (with count removed)
5. Outlier filtering method and parameters used

Example output:
  INFO: Applying criteria filters to 41 deals
  INFO: After criteria filtering: 12 deals (removed 29 deals)
  INFO: After outlier filtering (iqr, k=1.5): 4 deals (removed 8 outliers)

This helps users and developers understand:
- If criteria filters are too restrictive
- If outlier filtering is too aggressive
- Where deals are being filtered out in the pipeline

Addresses user question: "Why do I get just 4 deals for a central address
like סירקין 16 תל אביב?" - Now they can see exactly where deals were
filtered out.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
2025-11-27 00:15:33 +02:00
Nitzan P 421dec37b5 Fix duplicate polygon queries causing redundant API calls
Bug: When the Govmap API returns the same polygon_id multiple times
(which can happen for addresses with multiple associated polygons),
the code was processing each duplicate separately, leading to:
- Duplicate API calls for street/neighborhood deals
- Wasted API quota and slower performance
- Confusing logs showing same polygon queried multiple times

Example from logs:
  INFO: Querying polygon 53283601 (distance: 0m from search point)
  INFO: Getting street deals for polygon: 53283601
  INFO: Querying polygon 53283601 (distance: 0m from search point)
  INFO: Getting street deals for polygon: 53283601

Root cause:
- nearby_polygons API response can contain duplicate polygon_ids
- Code was appending all polygons without deduplication
- Log message claimed "unique polygon IDs" but didn't enforce it

Solution:
- Use dict to deduplicate polygons by polygon_id (lines 612-643)
- When duplicates exist, keep the one with shortest distance
- Convert dict to list after deduplication
- Now truly have unique polygon IDs as the log claims

Impact:
- Reduces API calls (fewer duplicates = less load on Govmap API)
- Faster queries (less redundant processing)
- Clearer logs (no confusing duplicate entries)
- Better performance for addresses with many associated polygons

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
2025-11-27 00:07:15 +02:00
Nitzan P bd0932abf6 Fix flaky temporal test for market activity trend detection
The test_market_activity_trend_stable test was failing intermittently
because it used get_recent_date() which approximates months as 30 days,
causing inconsistent month boundary calculations depending on when the
test runs.

Root cause:
- Using months_ago * 30 creates shifting month boundaries
- Calendar dates don't align consistently with 30-day periods
- Deals could fall into unexpected months based on test execution date
- Trend calculation compares first half vs second half of months
- Inconsistent month grouping led to "decreasing" instead of "stable"

Solution:
- Use explicit, deterministic dates (15th of each month)
- Calculate year-month combinations going back from current month
- Ensures exactly 1 deal per month for 12 consecutive months
- All deals guaranteed to be within time_period_months=12 window
- Month boundaries are now consistent regardless of test execution date

Before: Test result varied based on current date (flaky)
After: Test result is always "stable" (deterministic)

This ensures the test validates the trend detection logic without
temporal flakiness.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
2025-11-26 23:56:31 +02:00
Nitzan P 6882d7ab71 Add outlier filtering metadata to get_valuation_comparables response
The MCP now includes detailed outlier filtering information in the response,
allowing LLMs and bots to inform users about data quality improvements.

Changes to nadlan_mcp/fastmcp_server.py:
- Updated get_valuation_comparables to include outlier filtering metadata
- deal_breakdown now includes when filtering is applied:
  - total_deals: Count after filtering (final result)
  - total_deals_before_filtering: Original count before filtering
  - outliers_removed: Number of deals filtered out
  - filtering_method: Method used ("iqr", "percent", or "none")
  - iqr_multiplier: IQR multiplier when using IQR method (e.g., 1.5)
- Updated docstring to clarify that outlier filtering is automatic
  and metadata is included in response

Changes to tests/e2e/test_mcp_tools_comprehensive.py:
- Added assertions to verify outlier filtering metadata fields
- Test now validates the structure and types of filtering information

Example response:
{
  "market_statistics": {
    "deal_breakdown": {
      "total_deals": 15,
      "total_deals_before_filtering": 17,
      "outliers_removed": 2,
      "filtering_method": "iqr",
      "iqr_multiplier": 1.5
    }
  }
}

This allows bots like nadlan-bot to properly inform users:
"Found 17 comparable deals, filtered 2 outliers using IQR method (k=1.5),
 showing 15 deals."

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
2025-11-26 23:45:56 +02:00
Nitzan P 623d79947b Add logging for MCP tool function calls with parameters
Added log_mcp_call() helper function that logs all MCP tool invocations
with their parameters at INFO level. This provides:

- Better debugging and monitoring capabilities
- Clear visibility into which tools are being called and with what parameters
- Smart parameter formatting (truncates long strings, summarizes long lists)
- Consistent logging format across all 10 MCP tools

Example log output:
INFO - MCP tool called: find_recent_deals_for_address(address=סוקולוב 38 חולון, years_back=2, radius_meters=30, max_deals=100, deal_type=2)

This helps track API usage patterns and debug issues in production.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
2025-11-26 23:24:23 +02:00
Nitzan P ab59e13936 Fix: Same-building detection using correct API field names
The same-building detection was failing because the code was accessing
deal.street_name and deal.house_number (Pydantic model field names),
but the Govmap API actually returns streetNameHeb/streetNameEng and
houseNum as extra fields.

Modified address construction to check multiple field names using
getattr() to handle the API's actual field names, falling back to
model field names if needed.

This fix ensures same-building deals are correctly identified and
prioritized with priority=0.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
2025-11-25 09:20:14 +02:00
Nitzan P a809049f5e Feat: Implement distance-based deal prioritization and filtering
Implements smart deal prioritization based on distance from search point,
ensuring most relevant comparables are selected first.

Problem:
- Queried all polygons equally without distance consideration
- No filtering for deals that are too far away
- Street/neighborhood deals could be from distant locations
- No way to prefer closest polygon when multiple available

Solution:
1. Sort polygons by distance from search point (closest first)
2. Query polygons progressively, stop early if enough deals found
3. Calculate and store distance_meters for each deal
4. Filter deals by configurable distance thresholds:
   - Street deals: max 500m (configurable)
   - Neighborhood deals: max 1000m (configurable)
5. Sort by priority → distance → date (prefer closer deals)

Changes:

Config (nadlan_mcp/config.py):
- Added max_street_deal_distance_meters (default: 500m)
- Added max_neighborhood_deal_distance_meters (default: 1000m)

Client (nadlan_mcp/govmap/client.py):
- Extract polygon metadata with coordinates
- Sort polygons by distance using utils.calculate_distance()
- Progressive polygon querying (closest first, stop when enough deals)
- Calculate deal distance (use polygon distance as approximation)
- Filter street deals beyond max_street_deal_distance_meters
- Filter neighborhood deals beyond max_neighborhood_deal_distance_meters
- Store distance_meters on each Deal (dynamic attribute)
- Updated sorting: Priority → Distance → Date

Benefits:
- More relevant comparables (from nearby locations)
- Reduced API calls (query fewer polygons, stop early)
- Better performance (fewer deals to process)
- Configurable distance thresholds
- Transparent (distance info available in each deal)

Example Configuration:
export MAX_STREET_DEAL_DISTANCE_METERS=300    # Conservative
export MAX_NEIGHBORHOOD_DEAL_DISTANCE_METERS=500

Testing:
- All 326 tests pass
- Backward compatible (high default limits)
- Works with existing outlier filtering

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
2025-11-24 23:26:47 +02:00
Nitzan P aa3639b05c Fix: Make _safe_calculate_metric catch all exceptions
Problem:
- test_get_market_activity_metrics was failing with "list index out of range"
- _safe_calculate_metric only caught ValueError, not IndexError or other exceptions
- This caused the entire function to crash when metric calculations failed

Root Cause:
- Market metric functions (calculate_market_activity_score, get_market_liquidity,
  analyze_investment_potential) can throw IndexError if insufficient data
- The helper function wasn't catching these exceptions

Solution:
- Changed _safe_calculate_metric to catch all exceptions (Exception instead of ValueError)
- Added logging to track which metric failed
- Now returns error dict gracefully instead of crashing

Impact:
- get_market_activity_metrics is now more robust
- Returns partial results even if some metrics fail
- All 326 tests now pass

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
2025-11-24 22:54:03 +02:00
Nitzan P a25e0e406a Fix: Apply outlier filtering to deals list in get_valuation_comparables
Problem:
- get_valuation_comparables calculated statistics with outlier filtering
- But returned a deals list that still contained outliers (e.g., ₪900K, ₪1.3M deals)
- This caused inconsistency between statistics and the actual deals shown

Root Cause:
- calculate_deal_statistics() filters outliers internally for statistics
- But the deals array returned to user was only filtered by criteria (rooms, price, etc.)
- Outlier filtering was not applied to the returned deals list

Solution:
- Added explicit call to filter_deals_for_analysis() after filter_deals_by_criteria()
- Now both statistics AND deals list use outlier-filtered data
- Moved imports to top of file for better practice

Changes:
- nadlan_mcp/fastmcp_server.py:
  - Added imports: get_config, filter_deals_for_analysis
  - In get_valuation_comparables(): Apply outlier filtering before calculating stats
  - Ensures deals list and statistics are consistent

Testing:
- 325/326 tests pass
- 1 unrelated test failure in get_market_activity_metrics (pre-existing issue)

After deployment, outliers will be automatically filtered from valuation comparables.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
2025-11-24 22:51:01 +02:00
Nitzan P e791d3c1ed Normalize MCP response structures across all tools
Implement changes from MCP_NORMALIZATION_FIX.md to provide consistent
response structure across all MCP tools. This fixes the bot integration
issue where different tools returned data in different structures.

Changes:
- get_valuation_comparables:
  - Rename "comparables" → "deals"
  - Move "total_comparables" → "market_statistics.deal_breakdown.total_deals"
  - Add "search_parameters" section with "filters_applied"
  - Move "statistics" → "market_statistics"

- find_recent_deals_for_address:
  - Rename "price_stats.average_price" → "price_statistics.mean"
  - Rename "price_stats.median_price" → "price_statistics.median"
  - Rename "area_stats" → "area_statistics"
  - Rename "price_per_sqm_stats" → "price_per_sqm_statistics"

- get_deal_statistics:
  - Add "search_parameters" section
  - Move "statistics" → "market_statistics"
  - Add "market_statistics.deal_breakdown.total_deals"

- analyze_market_trends:
  - Add "market_statistics.deal_breakdown.total_deals"
  - Keep existing tool-specific fields (yearly_trends, etc.)

- get_market_activity_metrics:
  - Add "market_statistics.deal_breakdown.total_deals"
  - Keep existing tool-specific metrics

All tools now follow standard structure:
{
  "search_parameters" or "analysis_parameters": {...},
  "market_statistics": {
    "deal_breakdown": {"total_deals": N},
    "price_statistics": {"mean": ..., "median": ...},
    "area_statistics": {...},
    "price_per_sqm_statistics": {...}
  },
  "deals": [...]
}

Updated tests to match new normalized structure.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
2025-11-23 09:49:52 +02:00
Nitzan P c444e88a10 Fix Docker build: allow pyproject.toml in build context
The .dockerignore was excluding pyproject.toml, which is needed for
`pip install .` to work after the project was modernized to use
pyproject.toml instead of setup.py.

This was causing Docker builds to fail with:
"failed to calculate checksum: /pyproject.toml: not found"

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
2025-11-20 00:07:30 +02:00
Nitzan P b78346f3b0 Add statistical refinement and outlier detection system
Implement configurable outlier detection and robust statistical measures to
improve analysis accuracy for real estate data. Addresses issues with data
entry errors, partial deals, and other anomalies that skew statistics.

Key Features:
- IQR-based outlier detection (moderate filtering by default, k=1.5)
- Hard bounds filtering for obvious errors (price_per_sqm, deal_amount)
- Robust volatility using IQR instead of std_dev for investment analysis
- Transparent reporting with both filtered and unfiltered statistics

Implementation:
- Add outlier_detection.py module with IQR/percent/hard bounds methods
- Add OutlierReport model and enhance DealStatistics with filtered fields
- Update calculate_deal_statistics() to support optional outlier filtering
- Update analyze_investment_potential() to use robust volatility
- Add 9 new configuration parameters for customization
- Add comprehensive test suite (24 tests) for outlier detection
- Update CLAUDE.md with usage documentation

Configuration (all via env vars):
- ANALYSIS_OUTLIER_METHOD=iqr (default, or percent/none)
- ANALYSIS_IQR_MULTIPLIER=1.5 (moderate, 3.0=conservative)
- ANALYSIS_PRICE_PER_SQM_MIN/MAX=1000/100000 (bounds in NIS/sqm)
- ANALYSIS_MIN_DEAL_AMOUNT=100000 (catches partial deals)
- ANALYSIS_USE_ROBUST_VOLATILITY=true (IQR-based CV)
- ANALYSIS_USE_ROBUST_TRENDS=true (filter before regression)

Testing:
- All existing tests pass (326 passed)
- 24 new comprehensive outlier detection tests
- Real-world scenario tests (partial deals, data errors)

Backward Compatible:
- Default behavior improves accuracy without breaking changes
- All new fields in models are optional
- Config parameters have sensible defaults

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
2025-11-19 23:45:41 +02:00
Nitzan P 5be68a5b04 Modernize to pyproject.toml and fix test assertion
- Remove requirements.txt and requirements-dev.txt (duplicates pyproject.toml)
- Update Dockerfile to use 'pip install .' instead of requirements.txt
- Update README.md and GitHub workflows to use 'pip install -e .[dev]'
- Fix test_get_valuation_comparables: check for 'deal_date' instead of non-existent 'asset_room_num'
  (rooms field is optional and excluded when None due to exclude_none=True)
- Fix fastmcp version constraint in pyproject.toml (>=2.13.0,<3.0.0)
- Add vcrpy to dev dependencies

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
2025-11-18 23:00:03 +02:00
Nitzan P 96ec29dad2 Fix critical bug: rooms filter not working due to missing alias
The Deal model's 'rooms' field was missing the 'assetRoomNum' alias,
causing room data from the API to not be loaded into the model.
This resulted in all room-based filters returning 0 results.

Bug discovered when querying "חנקין 62 חולון" for 3-room apartments.
Query returned 0 results despite multiple 3-room deals existing in the data.

Root cause: Pydantic requires field aliases to map API field names (camelCase)
to Python attributes (snake_case). The 'rooms' field had no alias.

Fix: Added alias="assetRoomNum" to rooms field in Deal model.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
2025-11-17 23:42:31 +02:00
Nitzan P 87b0a355f8 Add HTTP transport support for cloud deployment
Enables deployment to cloud platforms (Render, Railway, etc.) while maintaining
backward compatibility with existing stdio transport for Claude Desktop.

New features:
- HTTP server entry point (run_http_server.py) using uvicorn
- Docker containerization with Python 3.13
- Health check endpoint at /health
- Comprehensive deployment documentation for Render, Railway, and Docker

Technical changes:
- Added uvicorn dependency for ASGI server
- Created Dockerfile with optimized multi-stage build (343MB)
- Added .dockerignore for efficient Docker builds
- Implemented /health endpoint using Starlette JSONResponse
- Updated README.md and DEPLOYMENT.md with HTTP deployment guides

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
2025-11-17 09:43:18 +02:00
Nitzan Pomerantz 092849edd3 Merge pull request #9 from nitzpo/fix-autocomplete-nondeterministic
Fix autocomplete non-determinism causing 0 deals
2025-11-15 00:10:29 +02:00