Skip to content

fix(dashboard): trend grafigi ay sonunda 7-11 aya dusuyordu - #1

Closed
ersinkoc wants to merge 4 commits into
CodeByPinar:mainfrom
ersinkoc:main
Closed

fix(dashboard): trend grafigi ay sonunda 7-11 aya dusuyordu#1
ersinkoc wants to merge 4 commits into
CodeByPinar:mainfrom
ersinkoc:main

Conversation

@ersinkoc

@ersinkoc ersinkoc commented Sep 11, 2026

Copy link
Copy Markdown

Summary by cubic

Fixes the dashboard trend chart shrinking to 7–11 months on month-end days and stops review assessments from lowering inherent score below the current residual score.

Bug Fixes

  • recent_months() in includes/functions.php anchors month arithmetic to the first of the month; api/dashboard_charts.php now uses it for the 12-month window and SQL :since.
  • assessments/_validate.php rejects review scores below the current residual score, so COALESCE(residual, inherent) can no longer report inflated risk; to lower inherent, the residual must be updated first.
  • Adds DB-free regression tests tools/trend_months_test.php and tools/assessment_validation_test.php.

Written for commit c144ad6. Summary will update on new commits.

Review in cubic

Kok neden: strtotime('-N month') gun tasmasini kirmaz. Ayin 29-31'inde
hedef ayda o gun yoksa sonuc bir SONRAKI aya tasiyor; ayni 'Y-m' anahtari
iki kez uretiliyor ve api/dashboard_charts.php'deki 12 aylik trend
penceresi 7-11 aylik map'e cokuyordu. Kayip aylarin riskleri trend
graginde hic sayilmiyor, SQL penceresinin alt siniri (:since) da bir ay
geriden geliyordu.

Cozum: ay aritmetigi ayin 1'ine sabitlenen recent_months() yardimcisi
(includes/functions.php) eklendi; uc nokta bu yardimciyi kullaniyor.
1. gunden cikarilan ay asla tasamaz.

Kanit: .temp_files altindaki proof-script oncesinde 11 prob gununden
9'unda FAIL (buckets=7, since kayik), sonrasi PASS. Dayanikli regresyon
testi tools/trend_months_test.php olarak eklendi (22 kontrol: ay sonu
gunleri, artik yil, yil siniri, count sinirlari, uc nokta baglantisi;
DB gerektirmez, php tools/trend_months_test.php ile calisir).
fix(dashboard): trend grafigi ay sonunda 7-11 aya dusuyordu

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

1 issue found across 3 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="tools/trend_months_test.php">

<violation number="1" location="tools/trend_months_test.php:90">
P3: The `check('taban null acik gecilirse ayni sonuc', recent_months(12, null) === recent_months(12))` assertion compares two independent `time()` evaluations captured microseconds apart. If the test happens to run across a month boundary, the two calls can land in different months and the assertion fails spuriously. Since both calls are meant to compare the same base, use a single fixed `$now` timestamp for both arguments to make the assertion deterministic.</violation>
</file>

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

$now = time();
check('bugun (varsayilan taban): 12 ay',
recent_months(12) === expected_months($now, 12));
check('taban null acik gecilirse ayni sonuc',

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P3: The check('taban null acik gecilirse ayni sonuc', recent_months(12, null) === recent_months(12)) assertion compares two independent time() evaluations captured microseconds apart. If the test happens to run across a month boundary, the two calls can land in different months and the assertion fails spuriously. Since both calls are meant to compare the same base, use a single fixed $now timestamp for both arguments to make the assertion deterministic.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At tools/trend_months_test.php, line 90:

<comment>The `check('taban null acik gecilirse ayni sonuc', recent_months(12, null) === recent_months(12))` assertion compares two independent `time()` evaluations captured microseconds apart. If the test happens to run across a month boundary, the two calls can land in different months and the assertion fails spuriously. Since both calls are meant to compare the same base, use a single fixed `$now` timestamp for both arguments to make the assertion deterministic.</comment>

<file context>
@@ -0,0 +1,133 @@
+$now = time();
+check('bugun (varsayilan taban): 12 ay',
+    recent_months(12) === expected_months($now, 12));
+check('taban null acik gecilirse ayni sonuc',
+    recent_months(12, null) === recent_months(12));
+check('count=1 yalnizca bulunulan ay',
</file context>

…altina indirebiliyordu

Kok neden: "residual skor inherent skoru asamaz" invarianti risk
olusturma/duzenleme formlarinda (risks/_validate.php) ve 'residual' tipi
degerlendirmede (assessments/_validate.php) dogrulaniyordu, ancak
'review' tipi icin koruma yoktu. Review, likelihood/impact (ve uretilen
inherent_score) kolonlarini degistirdigi icin mevcut residual skorun
ALTINA dusen bir inherent kabul ediliyordu: ornek 5x5=25 inherent,
4x4=16 residual olan riskte 2x3=6 review kabul edilir, tabloda
residual_score(16) > inherent_score(6) kalirdi.

Etki: tum "etkin skor" hesaplari (COALESCE(residual_score,
inherent_score) - risks/index.php filtreleri, reports/_reports.php,
api/dashboard_charts.php, reports/executive_summary.php) riskin
kendisinden buyuk deger gosterir; register ve matris celisir.

Cozum: assessment_collect_input() icinde review tarafi icin tek bir
dogrulama dalı eklendi - riskte residual varsa yeni skor residual
skordan kucuk olamaz; esit/uzerinde/residualsiz review'ler etkilenmez.

Kanit: round-owned proof-script GERCEK dogrulayiciyi stub db() koza
semaiyle calistirdi: oncesinde FAIL (hata donmuyordu), sonrasi PASS
(8/8 kontrol). Dayanikli regresyon testi
tools/assessment_validation_test.php eklendi (11 kontrol, DB
gerektirmez, uretim dogrulayicisini gercekten calistirir).

Not: bu dal main uzerinden acilmistir; PR #1/#2'ye bagimliligi yoktur.
fix(assessments): review degerlendirmesi inherent skorunu residualin altina indirebiliyordu
@CodeByPinar

Copy link
Copy Markdown
Owner

Teşekkürler. Dört bulguyu da doğruladım, dördü de gerçekti.

Özellikle schema.sql'deki eksik users tablosu ciddiydi: belgelenmiş
kurulumu tamamen kırıyordu ve yeni eklediğim Docker kurulumunu da
bozuyordu (docker-compose.yml şemayı initdb'ye bağlıyor). Yakaladığınız
için teşekkürler.

Düzeltmeleri main'e bağımsız olarak uyguladım (2530ae1). PR'ları merge
etmek yerine yeniden yazmamın iki sebebi var:

  1. PR'lar birbirinin üstüne istiflenmişti ve diff'leri çakışıyordu
    (fix(dashboard): trend grafigi ay sonunda 7-11 aya dusuyordu #1 ile fix(dashboard): trend grafigi ay sonunda 7-11 aya dusuyordu  #4 aynı düzeltmeyi içeriyor, fix(reports): yonetici ozeti Son 6 Ay tablosu ay sonunda 3-5 aya dusuyordu  #5 fix(dashboard): trend grafigi ay sonunda 7-11 aya dusuyordu  #4'ün üstünde duruyor).
  2. cubic'in test dosyalarına yaptığı eleştiriler haklıydı: sabit
    kodlanmış veritabanı parolası, sabit isimli veritabanını DROP etme,
    ilgisiz audit_logs satırlarını silen temizlik.

cubic'in son admin korumasına yaptığı P1 eleştirisini de hesaba kattım:
hedefin admin olup olmadığı artık transaction öncesi anlık görüntüden
değil, kilitli okumadan geliyor. Aksi hâlde hedef bu arada admin'e
yükseltilirse koruma tamamen atlanabilirdi.

Eklenen regresyon testleri:

  • tools/trend_months_test.php 35 OK (veritabanı gerektirmez)
  • tools/last_admin_race_test.php 7 OK (iki süreçli gerçek yarış;
    users tablosunu yedekler, finally bloğunda geri yükler)

#7 açık kalıyor - orada karşılığı olmayan bir test var, kapsamı
kaybetmek istemiyorum. İki P1 kapanınca memnuniyetle merge ederim.

Tekrar teşekkürler, başka bulgunuz olursa memnuniyetle bakarım.

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.

2 participants